language-tools icon indicating copy to clipboard operation
language-tools copied to clipboard

VSCode plugin: Syntax highlighting not correctly applied to embedded svelte component in markdown

Open wlach opened this issue 4 years ago • 16 comments

Describe the bug Syntax highlighting not correctly applied to embedded svelte component in markdown when using "Svelte for VSCode"

To Reproduce Steps to reproduce the behavior:

Put the following inside a markdown document:

```svelte
<script>
  import { onMount } from "svelte";

  export let data = undefined;
  let dom_node;

  onMount(() => {
    Plotly.newPlot(dom_node, data, {barmode: 'stack'});
  });
</script>

<div id="plotDiv" bind:this={dom_node}></div>
```

For example a code snippet that is treated in a way you don't expect.

Expected behavior

Expected similar syntax highlighting to what I see when viewing a svelte file with that content.

Actual behaviour

Get somewhat incorrect / inconsistent formatting (see screenshots)

Screenshots If applicable, add screenshots to help explain your problem.

View for a svelte component (correct):

image

View for a svelte component embedded in markdown (incorrect):

image

System (please complete the following information):

  • OS: Mac and Windows 10
  • IDE: VSCode
  • Plugin/Package: "Svelte for VSCode"

Additional context Add any other context about the problem here.

wlach avatar Jul 13 '21 19:07 wlach

Seem like the injection rules here doesn't work in the markdown code block for some reason. We'll see whether that's possible to fix.

jasonlyu123 avatar Jul 14 '21 01:07 jasonlyu123

I spent a bit of time today trying to debug this. I used this toy example to test:

```svelte
<script>
  var x = 5;
</script>
```

After giving things slightly more unique names, I found out that vscode's textmate parser seems to be "stuck" in this directive when in markdown:

https://github.com/sveltejs/language-tools/blob/6e0396ca18ea5e7da801468eab35cdef43b3c979/packages/svelte-vscode/syntaxes/svelte.tmLanguage.src.yaml#L467

That is to say, when I'm at var inside a script block, it still thinks I'm in meta.tag.start.svelte (renamed to meta.tag.start3.svelte in my local checkout). i.e. the scopes are:

entity.other.attribute-name.svelte
meta.attribute.var.svelte
meta.tag.start3.svelte
meta.script.svelte
meta.embedded.block.svelte
markup.fenced_code.block.markdown
text.html.markdown

What's odd is when I put the cursor at the very end of the script block above (i.e. just after the >), the textmate scopes are:

punctuation.definition.tag.end.svelte
meta.tag.start3.svelte
meta.script.svelte
source.svelte

So it's noticing the end of the block, but somehow not transitioning into the state where it's actually looking at the JavaScript. I assume that's:

https://github.com/sveltejs/language-tools/blob/6e0396ca18ea5e7da801468eab35cdef43b3c979/packages/svelte-vscode/syntaxes/svelte.tmLanguage.src.yaml#L18

Why would it be triggering endCaptures but not actually ending the block? It's very strange.

wlach avatar Jul 15 '21 19:07 wlach

Had the bright idea of using git bisect, which revealed that this used to work (when it was added in July 2020 with #301) but seems to have been broken by #657

wlach avatar Jul 15 '21 21:07 wlach

@Monkatraz any ideas on ^^^ ?

wlach avatar Jul 15 '21 22:07 wlach

Apologies for my flailing around. Dug into things more and realized that @jasonlyu123 was correct all along: the injection rules are not being applied in an embedded context. The weird behaviour I'm describing above is only due to the fact that the injection rules are not there, and it's falling back to whatever else is defined in the file.

I'm now pretty sure this is an issue upstream so filed an issue there: https://github.com/microsoft/vscode-textmate/issues/152 -- that has some more details as well as confirmation of the problem described.

It's likely possible to make the grammar not depend on injections (e.g. I got it working for bare script tags without them), though I think it would be less elegant.

wlach avatar Jul 16 '21 13:07 wlach

Thank you for digging into this!

dummdidumm avatar Jul 16 '21 13:07 dummdidumm

Based on the response in https://github.com/microsoft/vscode-textmate/issues/152, it looks like we might have to rewrite this not to use injection grammars (at least if we care about fixing this bug).

wlach avatar Oct 22 '21 20:10 wlach

Doing that will be horribly messy... How frustrating.

Monkatraz avatar Oct 23 '21 15:10 Monkatraz

At least this comment gave me another possibility on how to better debug grammar matches

dummdidumm avatar Oct 23 '21 15:10 dummdidumm

Okay I guess I need to do three things sooner or later:

  • find a decent way of adding embedded languages that doesn't involve injections
  • Make a separate PR for the --css-var attribute highlighting
  • Clean up my miniature CSS grammar PR

I've been working with CodeMirror 6 a lot recently and the highlighting there is so much better it's jarring going back to VSCode lol (I should make a Svelte grammar for it)

Monkatraz avatar Oct 23 '21 15:10 Monkatraz

Although I should add that one alternative is to use a different grammar for markdown, I think one that always uses TypeScript would be a decent compromise. Obviously that has edge cases (what if you're using something nutty like CoffeeScript or Rescript)

Monkatraz avatar Oct 23 '21 15:10 Monkatraz

This sounds like a decent short-time solution. It would solve the issue for the majority of users, for those drinking coffee or otherwise it's still as buggy as before inside the script tag.

dummdidumm avatar Oct 23 '21 15:10 dummdidumm

I'm having a similar issue, but with strings inside html tags. It's pretty annoying. Is there some sort of workaround that can be applied locally? image Something's up with stuff inside the brackets too. It as if the brackets inherit the string color or something.

If it's of any help here are some token inspector captures:

image

image

karmaral avatar Dec 11 '21 16:12 karmaral

I just wanted to update that doing a clean vscode install seems to have fixed my issue. I couldn't backtrack it to something specific though. It wasn't my settings.json nor my extensions, but something in the programs settings UI. It probably too buried to bother looking. image

karmaral avatar Dec 23 '21 18:12 karmaral

Although I should add that one alternative is to use a different grammar for markdown, I think one that always uses TypeScript would be a decent compromise. Obviously that has edge cases (what if you're using something nutty like CoffeeScript or Rescript)

Not sure if this is what you had in mind, but I tried something like this in https://github.com/sveltejs/language-tools/pull/1537. Basically reinjecting enough rules to make script/style tags do something sensible if they're embedded in markdown.

wlach avatar Jun 22 '22 21:06 wlach

Mostly fixed by #1537 through adding additional injection rules for markdown files. I'll keep this open, maybe we find a way someday to solve this in a way that these addition injections are not needed.

dummdidumm avatar Jun 26 '22 07:06 dummdidumm