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

Svelte highlighting seems to be not working #6180

Closed
stevepopovich opened this issue Nov 21, 2022 · 7 comments · Fixed by #6184
Closed

Svelte highlighting seems to be not working #6180

stevepopovich opened this issue Nov 21, 2022 · 7 comments · Fixed by #6184

Comments

@stevepopovich
Copy link

Describe the bug

Hi folks, hubber here from the mobile team. Svelte highlighting seems to be broken and has been for months. I am trying to get to the bottom of what is going on and need some direction, hoping y'all can help.

Highlighting in this https://github.com/Lleweraf/supachat/blob/latest/src/components/ChatInput.svelte is broken, both on dotcom and on mobile.

Expected behaviour

We should see svelte code highlighted properly

Related discussion

This is the original issue we are tracking on mobile.

@lildude
Copy link
Member

lildude commented Nov 22, 2022

@stevepopovich I recently got to the bottom of this in #6164

In short, the grammar is using regex matches in the scope names but GitHub's very old syntax highlighter (prettylights) doesn't expand these so subsequent rules aren't found.

The old highlighter isn't going to be fixed so the options are:

  • update the grammar to take this limitation into account, or
  • confirm the tree-sitter grammar is at least as complete as the Textmate-based grammar and renders correctly and then I can lobby to have that added to the new highlighter (treelights) instead (Linguist has no direct influence on the treelights grammars).

I'm waiting to hear back on that discussion, but in short this isn't a Linguist issue but rather a grammar/highlighter issue.

@lildude
Copy link
Member

lildude commented Nov 22, 2022

I've done some quick testing in dev using the tree-sitter grammar and it looks promising and much better than the Textmate grammar:

CleanShot 2022-11-22 at 09 45 37

I'm going to submit a PR proposing to add the Svelte tree-sitter grammar. We'll see how it goes.

@stevepopovich
Copy link
Author

Amazing! Thank you. Sorry I was in the wrong spot, things have gotten confusing for me trying to figure out who owns the problem.

@lildude
Copy link
Member

lildude commented Nov 23, 2022

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.

More deets in my response at #6164 (reply in thread)

@stevepopovich
Copy link
Author

So once https://github.com/github/github/pull/248502 is deployed, this is expected to be fixed?

@lildude
Copy link
Member

lildude commented Nov 28, 2022

Yup.

@lildude
Copy link
Member

lildude commented Nov 28, 2022

And we're back to working Svelte syntax highlighting (below should look like the screenshot above):

<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 bar
  }
</style>

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

@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants