reimplement button-to-open-in-new-tab + custom webview #133 (monorepo)

[kevinramharak]

Reimplemented #133 but with the monorepo setup. Used a new branch to avoid having to solve the merge errors that would come from trying to merge main back into that branch.

Whats left:

• Use of https://github.com/yoavbls/vscode-shiki-bridge instead of custom CSS (if applicable)
• Injecting the vscode-codeicon icon font (example)
• Removing the Open in new Tab link in the Markdown preview (its there, just not shown because the icons don't show up)
• Maybe take another stab at injecting the type grammar this is not needed if we use vscode-shiki-bridge

• I'm not sure why, but if a panel is opened and I click on another one, it stays on the previous one.
  Is there something to do about it? (copied from here)

We can also extend the markdown-it as @MichaelDimmitt also mentioned. Some references are:

• vscode docs
• vscode-markdown-extended
• markdown-it

Some of the bugs/limitations can be discovered in how the markdown preview is implemented at markdown-language-features

This does indicate that a custom webview could be a better option, as it would be a lot less limiting. As for placement, a custom webview can be rendered pretty much everywhere, as an editor tab or part of the activity bar, so we could add configuration to let the user choose where to render it, with a sensible default.

[kevinramharak]

@yoavbls I ported the changes from web-preview-poc to this branch and included use of vscode-shiki-bridge.
It's messy and buggy, some weird build behaviour causing jsonc-parser to sometimes break at runtime when it is internally requireing some file.
A bunch of bugs / missing features of vscode-shiki-bridge that I added comments for (will convert them to proper issues later).

But it does work:

I think this is enough of poc to confirm this works and is a better approach. Maybe we start with implementing it with the default dark/light highlighters dark-plus and light-plus as they are bundled with shiki. vscode-shiki-bridge just needs some work to properly resolve and fetch language grammars and theme files, and when that works properly the rest of the feature will have the correct highlighting and theme out of the box.

[yoavbls]

wowwww I'm so excited to see that this POC is working!
I understand that shiki-bridge is quite broken, I'll try to fix things there over the weekend and update this PR.
If it seems like the web view is working, can we remove the markdown parts and checks?

[kevinramharak]

@yoavbls I think we still need these parts:

• The formatter still needs to embed the codeicon button that triggers the command
• A part that registers the TextDocumentContentProvider API for our custom URI scheme and returns the formatted message as markdown contents.
• A part that registers the command to open in a new tab, which will call vscode.window.createWebviewPanel, fetch the contents through our custom text document content provider and embed it in the webview HTML.

The main change is that instead of executing the markdown.showPreview command, we will execute the tsPrettyErrors.openMarkdownPreview command.

We could embed it all in the command that creates the webview, but having the custom text document content provider forces a separation of concerns. Which I think is a good thing here.

During prototyping of this and the l10n draft I ran into the issue that passing the diagnostic as a string and performing string replacements makes it quite difficult to reason about the state of the string and what is and isn't part of it. For example removing the Open in new Tab link in the formatted message, is going to require another regex to find and replace it.

I also suspect that the heavy use of regexes is part of the performance issues some people experience (although pretty sure it's also caused by huge projects with lots of files).

So if there is a way to implement a more abstract representation of the (formatted) diagnostic, it could help, maybe not so much in this issue, but for future features and changes.

Something more like:

type FormattedDiagnosticPartType = 'text' | 'codeblock' | 'icon' | 'link';

type FormattedDiagnosticPart = { type: FormattedDiagnosticPartType, text: string }

interface FormattedDiagnostic {
  parts: FormattedDiagnosticPart[];
  code: number;
  location: Uri;
  range: Range;
}

I don't think we need it in this implementation, but keeping it in mind while implementing could really help to figure out if we need it and how a consuming API would use a data structure instead of a string.

[kevinramharak]

@yoavbls @MichaelDimmitt

We are getting close:

The old implementation was pretty easy to stitch to the new one, the hardest parts were figuring out CSP and the way we link to a symbol had to change. It's a bit smarter now as well, so it wont open links in the preview in the same active panel.

Code needs a bit of cleanup, but the feature is very close to complete.

[yoavbls]

I didn't review everything yet but I tested it out and it's amazing! I could not wait to merge it 😀
The main things should be done before:

1. Upgrade vscode-shiki-bridge
2. Use the user's theme for the code blocks
3. Get rid of the previous logic meant for the markdown-based solution instead of the webview.
4. Make the panel update instead of opening a new one
5. Make the persistent panel on the explorer section update when clicking on a new error. I really liked this one

Thank you for all of this work
@kevinramharak @MichaelDimmitt

[kevinramharak]

@yoavbls I think this is almost done now. Just needs to use the vscode-shiki-bridge update to use the user theme and languages to highlight the code blocks.

The custom markdown rendering was a bit buggy and generating incorrect HTML which messed up the preview. I ended up trying to integrate markdown-exit as it seemed to integrate with shiki nicely. Besides 1 annoying issue of the markdown renderer wrapping fenced code blocks, it worked out pretty well:

I think having this setup works well, as when I use my laptop with a smaller screen, the preview panel is often not large enough, so being able to open any error in a new tab is very helpful.

The view in the explorer panel now follows the cursor/selection, and feels pretty good. I initially cleared the panel when moving the cursor to a selection without an error, but that can be annoying, so now it just shows the last error for the code you selected.

This solves a lot of issues with the hover not being copiable (#155), as both the new tab preview and the explorer panel allow for selecting and copying text. Additionally the new copy button #156 allows for copying the whole error message, and each code block now has a copy button. I added a notification message to give some user feedback that the click on the button actually did something.

If you can take a look at yoavbls/vscode-shiki-bridge#2 and create a release for it, then all that remains is update the highlighting code to use the user theme, and this should be ready to go.

[kevinramharak]

@yoavbls You happened to push the update, so I stitched it together. I tried it with my own theme and another theme. I think this is now complete.

There could be some QOL things like loading new themes when the theme changes, but it's nothing a restart or reload of VSCode won't fix.

[yoavbls]

Great! yes It's fine if a restart is required in case there is a new extension or theme.
So i'm checking it and let's merge 💃🏼💃🏼

[MichaelDimmitt]

Ill check on my side as well!

[MichaelDimmitt]

From testing ran into the following issues:

1. The copy to clipboard function, when you hover over the preview only copies:
   ZodDiscriminatedUnion<

2. Also the inputs are only shown in a single line.

But these things can probably be fixed with some slight ui adjustments.

When I run it using this example file:

import { z } from "zod";

const fooo = z.object({
  randomVariable: z.literal('new'),
  bar: z.string(),
  baz: z.string(),
  biq: z.object({
    info: z.object({
      example: z.number(),
      crazyType: z.number()
    })
  })
})

const bazz = z.object({
  randomVariable: z.literal('notNew'),
  bar: z.string(),
  baz: z.string(),
  biq: z.object({
    info: z.object({
      example: z.number(),
      crazyType: z.number()
    })
  })
})
interface baz {
  bar: {}
  baz: {}
  biq: {
    info: {
      example: {}
      crazyTypwe: {}
    }
  }
};
const badExample = z.discriminatedUnion('randomVariable', [
  fooo,
  bazz,
])
type argh = typeof badExample

export function funExample(a: argh): argh[] {
  return a;
}

[kevinramharak]

@MichaelDimmitt I will take another look tomorrow and see what is missing.
I haven't tested the .vsix yet, I use the debug feature from VS Code (F5 as default). It will run the build task in the background and start a new VS Code with the extension development host.

build and dev should be similar, dev should not loop infinitely but start a file watcher. The loop might be caused by a generated file being watched.

[kevinramharak]

There is indeed an issue for packaging/publishing the .vsix:

ERROR  SVGs are restricted in README.md; please use other file image formats, such as PNG: https://custom-icon-badges.demolab.com/badge/Visual Studio Code-0078d7.svg?logo=vsc&logoColor=white

I fixed it with 6700a52. Unfortunatly the whitelisted badge providers require to embed the vscode extension icon as a base64 embedded png. But using a markdown reference, its not to ugly. I had to use the flat white icon for cursor instead of the original one, as its the only one imgshields provides.

This allows for building the .vsix properly. I will be using the custom .vsix for a bit to see if it works correctly. @yoavbls We could also release 0.7.0 as a prerelease just for a bit to allow people to use it and see if there any major bugs. And release it normally at 0.7.x afterwards.

---

I think the infinite loop on dev is related to mac OS specifics, I cannot reproduce it. Maybe check the git status to see if uncommited files are generated.

---

I fixed the copy type button, the implementation was a bit hacky dumping the content in an HTML attrivute, now it just copies the innerText from the <pre><code> tag, which should give the exact same result as selecting the text in a code block and copying it manually.

---

As for the type codeblocks not wrapping that is strange, I see the following:

Does it wrap in the hover prettified error? Can you try opening the developer tools from VS Code and see what the HTML is of those codeblocks? Anything in a pre should have the CSS property white-space: pre;, which should render any \n as an actual new line.

If needed we can use Shiki's output to add some CSS, as each line will be wrapped in a <span class="line".

[MichaelDimmitt]

Awesome, I will grab latest and test it out

[MichaelDimmitt]

Tested it out and looking good now!

[MichaelDimmitt]

Tip: tsconfig, "noErrorTruncation": true,

If you are getting output in one line but need to see it in what is depicted in the screenshots change your tsconfig to have a compilerOption "noErrorTruncation": true,

[MichaelDimmitt]

TLDR: Works, thanks!

[MichaelDimmitt]

Nice use of css variables here to get the vscode themes!
I think that is what is going on here alongside TmLanguage-Syntax-Highlighter
and these libraries: markdown-exit, shiki, and vscode-shiki-bridge

[MichaelDimmitt]

Read through the pr, functionality works on my end.

Local development setup was a little difficult but I got it figured out.

It looks like the only outstanding task is writing a script to move the codicon.css file out of nodemodules instead of keeping your own version? But probably out of scope of this pr.

Thanks for adding this! @kevinramharak and @yoavbls

[MichaelDimmitt]

Not sure if you want your contribution.md file to have developer setup instructions for getting debugging environment up and running. The dev setup has changed a bit since the mono repo update. But also this change might be out of scope of this pr.

[kevinramharak]

I thought there was an issue for this, but I cant find it. This is indeed on the todo list after the monorepo setup. I will do this next, as it is intended to easily setup and contribute to this repo

[MichaelDimmitt]

It looks like this file is the main change and uses markdown-exit, shiki, and vscode-shiki-bridge.

[kevinramharak]

@MichaelDimmitt That is very helpful. If you want, you can create a separate issue for it, but I will probably have some time this week to create and fix the CONTRIBUTING.md stuff.

[MichaelDimmitt]

Documented error and docs request in:
#158 and
#159

I'll remove those comments from this pr now.