sveltejs / svelte-atom

Syntax, diagnostics, and other smarts for Svelte in Atom
MIT License
30 stars 11 forks source link

Syntax highlighting not working for `<script>` and `<style>` tags on GitHub (web and mobile) #20

Open kindoflew opened 2 years ago

kindoflew commented 2 years ago

The syntax highlighting is not working on GitHub, both web and mobile, for the <script> and <style> sections of .svelte files. The template section seems to be unaffected and is still working (including svelte-specific tags, like {#each}) as long as you aren't using <template> tags (I didn't know this until opening this Issue and making the examples below).

Opening this Issue here because this repo is what linguist is pointing to for svelte syntax highlighting in GitHub. A handful of Issues have been opened in the related repos (GitHub's community and linguist) and I even sent a bug ticket to GitHub mobile through the app, but they all seem to suggest it might be a bug in the actual grammar. This lines up with my observations, as I only noticed this behavior after this https://github.com/github/linguist/pull/5963 was merged.

This is the most recent reply from linguist: https://github.com/github/linguist/issues/5830#issuecomment-1192348176

I don't know anything about syntax highlighting grammars, so I can't suggest any ideas. And I'm not even sure if you were aware that linguist is now using this repo for GitHub -- in which case I'd be fine with closing the issue as wontfix and maybe trying to revert the PR in linguist.


EXAMPLES:

(First two are just using markdown codeblocks with ```svelte, the rest are images -- first two are web on Chrome, third is mobile on iOS)

Without <template> tag:

<script>
  const notWorking = 'I am not highlighted';
</script>

{#each something as s}
  <p on:click={doSomething}>
    Our tags are highlighted correctly
  </p>
{/each}

<style>
  .not-highlighted {
    color: 'black'; // because it isn't highlighted
  }
</style>

With <template> tag:

<script>
  const notWorking = 'I am not highlighted';
</script>

<template>
  {#each something as s}
    <p on:click={doSomething}>
      Now we aren't working.
    </p>
  {/each}
</template>

<style>
  .not-highlighted {
    color: 'black'; // because it isn't highlighted
  }
</style>

Screen Shot 2022-07-26 at 10 19 51 AM

Screen Shot 2022-07-26 at 10 20 10 AM

Image from iOS (6)


Related issues:

kindoflew commented 2 years ago

Update: According to this comment, https://github.com/github/linguist/issues/5830#issuecomment-1195653540, it might have to do with a bug in the injections section.

kindoflew commented 1 year ago

Another Update: Unfortunately, my fix didn't work. However, the maintainer of linguist did some digging and discovered what is likely to be the actual cause of the bug: https://github.com/github/linguist/discussions/6164#discussioncomment-4178987

TL;DR -- an old bug in the linguist highlighter's TextMate grammar parser does not expand scope match regex (things like $1). so, it takes the symbol literally, instead of expanding to something like script|style|template. I don't have any explanation for why it works for some of them, but not the top-level tags -- but I don't really understand most of this stuff beyond what little I've learned over the last few months investigating this 😅.

Unfortunately, they are no longer adding fixes or improvements to their old parser (they are working on migrating everything to TreeSitter). So, if we want to solve it here, we'd need to replace all of those with the explicit tag it is referencing (which I think means we'd need to duplicate the rules for each tag type?). or, we can close this and eventually point linguist to a Svelte TreeSitter grammar (they linked to one in the answer, but I don't know if we'd rather have an official one under the sveltejs org or not).

Kind of a bummer, but at least we now know what the issue is and have possible ways to fix it eventually.