Help with figuring out what's wrong with Svelte grammar

[kindoflew]

I opened a PR in the upstream Svelte grammar to attempt to fix broken syntax highlighting (sveltejs/svelte-atom#21), but without any solid way to test it, we were left waiting for the new release of linguist to see if it worked or not.

Another dev opened a PR here (#6144) before the new release, but closed it, because we determined that the fix in svelte-atom should have worked.

That dev made a fork of the community fork of Lightshow (https://umanghome.github.io/NovaLightshow/) that allowed us to test the grammar. Using that, I found that NovaLightshow was getting tripped up by svelte-atom's .cson grammar -- it was throwing a YAMLError. If I converted it manually to JSON (I used this site: https://www.convertsimple.com/convert-cson-to-json/) and pasted it in the Grammar box, syntax highlighting worked.

It was also my understanding that linguist converts .cson files to .json when building its grammars: https://github.com/github/linguist/blob/32ec19c013a7f81ffaeead25e6e8f9668c7ed574/tools/grammars/compiler/proto.go#L65-L69 So I assumed that meant the "fixed" upstream grammar would work once converted to JSON.

However, after the release of 7.24.0, Svelte syntax highlighting is still broken on GitHub. Is there anyone who knows more about this stuff who could possibly help figure out what is going wrong here? Thanks!

[lildude]

I've done some digging and the issue is going to be down to GitHub's old highlighting engine which still parses these TextMate-based grammars.

The problem is it doesn't expand scope matches and instead treats the $1 in places like this literally:

'tags-lang':
    begin: '<(script|style|template)'
    end: '</\\1\\s*>|/>'
    beginCaptures:
      '0':
        patterns: [include: '#tags-start-node']
    endCaptures:
      '0':
        patterns: [include: '#tags-end-node']
    name: 'meta.$1.svelte'.  #<---------------- HERE
    patterns: [
      begin: '\\G(?=\\s*[^>]*?(type|lang)\\s*=\\s*([\'"]|)(?:text/)?(\\w+)\\2)'
      end: '(?=</|/>)'
      name: 'meta.lang.$3.svelte'
      patterns: [include: '#tags-lang-start-attributes']
    ,
      include: '#tags-lang-start-attributes'
    ]

This means it will not match any of your injections which are expecting the $1 to be replaced with script|style|template.

This is news to me. It's also unfortunately been like this for well over 8 years and unfortunately isn't going to change as this old parser is no longer getting internal funding for fixes or improvements in favour of the newer tree-sitter based highlighting.

This means the only solution is to be explicit with the scope names and not rely on the regex matches, or wait for the Svelte tree-sitter grammar (I think this is it) to be pulled in by the team that maintains the highlighting engines.

If the tree-sitter grammar is at least as functionally complete as the TextMate grammar (lemme know, as I don't know), I can have a word with the team and put it on their radar for consideration.

[lildude]

So it turns out my local dev env was not actually pulling in the treesitter grammar when testing. It was actually pulling in the TextMate grammar from Linguist v7.20.0 which means the TextMate grammar was working properly then and started failing after that.

Really (promise this time) testing the tree-sitter grammar from https://github.com/Himujjal/tree-sitter-svelte showed it is questionably "better" (ie it's not much) than what we have now:

So the tree-sitter grammar is definitely not ready for prime time, so I will not be continuing down that avenue.

Back to the TextMate grammar...

The first time we see a change in the Svelte grammar after Linguist v7.20 was the change in #5963 which changed the source of the grammar. This first shipped in Linguist v7.22.0 and is likely when the breakage occurred.

Looking at the two grammars, I can see they're quite different.

It does mean we have a quick fix: revert #5963 and go back to the four-year old grammar from https://github.com/umanghome/svelte-atom whilst we wait for a prime-time tree-sitter grammar or for the https://github.com/sveltejs/svelte-atom grammar to update to take into account the limitations of the ancient highlighter.

Given how impactful this is, I'm leaning towards reverting back to the old grammar.

[lildude]

It's interesting to note that https://github.com/umanghome/svelte-atom was forked from https://github.com/sveltejs/svelte-atom

Looking through the history of differences, this would be where things broke: sveltejs/svelte-atom@d01994a This commit suggests this was with the adoption of the VSCode grammar and can see things switch from explicit scopes to using expanded regex matched names.

[lildude]

Given how impactful this is, I'm leaning towards reverting back to the old grammar.

I've just been reminded that we're close to the feature freeze date for the next GHES release and I'd rather not ship with known broken syntax highlighting so I'm going to go for this option and make a patch release early next week.

If the https://github.com/sveltejs/svelte-atom grammar has been updated to address the limitations, I'll use that instead.

[lildude]

#6184

[kindoflew]

Sorry for the delay, Thanksgiving holiday in the US.

I see that you've already merged the old grammar, but I agree -- there's a few bugs in it still, but it's better than the completely broken highlighting we had.

I haven't gotten to open a PR in svelte-atom yet, but since we have syntax highlighting mostly working again, it doesn't seem like a time sensitive issue anymore. I'll either open one there eventually or reach out to the svelte team to see if they have any other ideas (like eventually maintaining an official TreeSitter grammar under the sveltejs org). Either way, probably expect a PR from me sometime before the next, next GHES release I guess 😅.

Thanks so much again for getting involved in this!