Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wlach opened this issue Jul 13, 2021 · 16 comments
Labels
bug Something isn't working

Comments

@wlach
Copy link
Contributor

wlach commented Jul 13, 2021

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 wlach added the bug Something isn't working label Jul 13, 2021
wlach added a commit to wlach/myst-vs-code that referenced this issue Jul 13, 2021
Currently the syntax highlighting isn't quite right, probably
related to a bug in the underlying Svelte highlighter for vscode:

sveltejs/language-tools#1094

I'd like to fix the above issue before asking this to be merged, but
I thought I'd file this initial PR for feedback.
@jasonlyu123
Copy link
Member

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.

@wlach
Copy link
Contributor Author

wlach commented Jul 15, 2021

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:

endCaptures: { 0: { name: punctuation.definition.tag.end.svelte } }

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:

'L:meta.script.svelte meta.lang.javascript - (meta source)':

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

@wlach
Copy link
Contributor Author

wlach commented Jul 15, 2021

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
Copy link
Contributor Author

wlach commented Jul 15, 2021

@Monkatraz any ideas on ^^^ ?

@wlach
Copy link
Contributor Author

wlach commented Jul 16, 2021

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: microsoft/vscode-textmate#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.

@dummdidumm
Copy link
Member

Thank you for digging into this!

@wlach
Copy link
Contributor Author

wlach commented Oct 22, 2021

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

@Monkatraz
Copy link
Contributor

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

@dummdidumm
Copy link
Member

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

@Monkatraz
Copy link
Contributor

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
Copy link
Contributor

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)

@dummdidumm
Copy link
Member

dummdidumm commented Oct 23, 2021

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.

@karmaral
Copy link

karmaral commented Dec 11, 2021

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
Copy link

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

wlach added a commit to wlach/language-tools that referenced this issue Jun 22, 2022
Mostly fixes sveltejs#1094. Do this by injecting rules to parse typescript and
css for svelte script/style blocks inside markdown only. This
should cover *most* cases of embedded svelte blocks inside
markdown.

It would not be *too* difficult to extend this further to handle other
types of embedded script/style tags (as is done in the original grammar),
though it would be somewhat tedious.
@wlach
Copy link
Contributor Author

wlach commented Jun 22, 2022

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 #1537. Basically reinjecting enough rules to make script/style tags do something sensible if they're embedded in markdown.

dummdidumm pushed a commit that referenced this issue Jun 26, 2022
Mostly fixes_ #1094. Do this by injecting rules to parse typescript and
css for svelte script/style blocks inside markdown only. This
should cover *most* cases of embedded svelte blocks inside
markdown.

It would not be *too* difficult to extend this further to handle other
types of embedded script/style tags (as is done in the original grammar),
though it would be somewhat tedious.
@dummdidumm
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants