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

• 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 is 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.