Jison/Jisonlex files no longer highlighted correctly on GitHub

[nwhetsell]

Preliminary Steps

Please confirm you have...

• reviewed How Linguist Works,
• reviewed the Troubleshooting docs,
• considered implementing an override,
• verified an issue has not already been logged for your issue (linguist issues).

Problem Description

Jison/Jisonlex highlighting seems to be incorrect on GitHub. Here’s an example on GitHub:

https://github.com/cdibbs/language-jison/blob/master/spec/sample.jison

And here’s the same file on Lightshow:

https://github-lightshow.herokuapp.com/?utf8=✓&scope=source.jison&grammar_format=auto&code_source=from-url&code_url=https%3A%2F%2Fraw.githubusercontent.com%2Fcdibbs%2Flanguage-jison%2Fmaster%2Fspec%2Fsample.jison

Lightshow appears to be running Linguist v7.3.1. If GitHub is running Linguist v7.6.0 or later, this may be related to PR #4595.

[Alhadis]

#4595 didn't touch Jison highlighting, so the problem likely stems from the author's embedding of the source.lex scope somewhere.

In either case, this is an upstream issue that should be filed at the appropriate repository, not here.

[nwhetsell]

Thanks for the swift reply!

The repository is https://github.com/cdibbs/language-jison, but a pretty big hunk of it is from me. So, while I can file an issue there, I’m almost certainly going to be the one to have to fix it anyway. I’ll also understand if this issue must be closed if it’s determined that this simply can’t be an issue with Linguist.

I’m happy to be wrong about this, but as far as I can tell, source.lex doesn’t appear in any of the Jison grammars. There are inclusions of source.js and source.jisonlex, however. Could the inclusion of source.jisonlex be the issue?

[Alhadis]

Could the inclusion of source.jisonlex be the issue?

I doubt it. I noticed the grammar is using injections; keep in mind they're not handled 100% consistently by GitHub's highlighter (the injectionSelector field in particular is unsupported, according to #3924).

Also, could you be a bit more specific about which parts in particular are incorrectly highlighted?

[nwhetsell]

I was under the impression that the injectionSelector field had been whitelisted: cdibbs/language-jison#17 (comment). At any rate, that part of the grammar seems to be working. The highlighting of yytext in line 9 of language-jison/spec/sample.jison is coming from that injectionSelector:

The part of a Jison file that’s incorrectly highlighted is everything after the first section break denoted by %% (including the section break). For example, here’s lines 37–68 of language-jison/spec/sample.jison on GitHub:

And here’s the same thing on Lightshow:

[Alhadis]

Okay, so this is certainly an issue on GitHub's end. There's no reason that should be working in Lightshow and not on GitHub's side.

I also noticed ordinary Lex files aren't receiving syntax highlighting at all, indicating that something very wrong is happening with Lightshow...

/cc @lildude

[lildude]

Whooops. I've clearly forgotten to update Lightshow 😊. I'll add it to my TODO list.

[Alhadis]

There's still the matter of the highlighting discrepancies; in particular, the missing Lex highlighting added in the last release...

[lildude]

There's still the matter of the highlighting discrepancies; in particular, the missing Lex highlighting added in the last release...

🤔 the language is getting selected correctly so something's gone awry with picking up the grammar itself.

[lildude]

I've taken a look into this as this appears to only affect languages or grammars added in v7.6.0. Work is underway to improved the syntax highlighting service used by GitHub and I suspect it has missed the v7.6.0 update. I've opened an issue to bring this to the attention of the team responsible for the improvements.

I'll update when I know more or when things are working as expected again.

[lildude]

Re:

I also noticed ordinary Lex files aren't receiving syntax highlighting at all, indicating that something very wrong is happening with Lightshow...

... this ...

Work is underway to improved the syntax highlighting service used by GitHub and I suspect it has missed the v7.6.0 update.

... was indeed the case.

The original issue is however unrelated, that's still a grammar issue. I'll try update Lightshow in the next few days.

[lildude]

I've hit a road block with updating Lightshow that is going to require some dedicated time to resolve. As this isn't related to this issue, I'm going to close this issue more.

[nwhetsell]

I understand that closing this issue is entirely up to Linguist, but I’m not sure that this is a grammar issue, and if it is a grammar issue I’m not sure how to determine what the issue is.

I first noticed this issue after GitHub started using Linguist v7.6.0, around the end of Aug 2019. Jison files were definitely highlighted correctly on GitHub before then. Some of my pull requests to Jison grammars were about improving highlighting on GitHub, for example cdibbs/language-jison#8 (merged in May 2017), and it wouldn’t have made sense to make improvements to syntax highlighting on GitHub if that wasn’t working at all.

One possibility mentioned in a comment on this issue is the presence of a source.lex scope in a Jison grammar. The problem there is that source.jison, source.jisonlex, and source.js are used in Jison grammar includes, but not source.lex. Running

pattern="include:\\s*[\"']source\\.\\w+"
base_URL=https://raw.githubusercontent.com/cdibbs/language-jison/v2.12.0/grammars

file=jison.cson
curl --remote-name --show-error --silent $base_URL/$file
egrep $pattern $file

shows 12 includes:

        # Simply setting the patterns object to {include: "source.jisonlex"}
                    patterns: [include: "source.jisonlex#user_code_section"]
                patterns: [include: "source.jisonlex#rules_section"]
            patterns: [include: "source.jisonlex#definitions_section"]
      {include: "source.js"}
        patterns: [include: "source.js"]
        patterns: [include: "source.js"]
        patterns: [include: "source.js"]
        patterns: [include: "source.js#string_escapes"]
        patterns: [include: "source.js#string_escapes"]
        patterns: [include: "source.js#string_escapes"]
        patterns: [include: "source.js"]

Then running:

file=jisonlex.cson
curl --remote-name --show-error --silent https://raw.githubusercontent.com/cdibbs/language-jison/v2.12.0/grammars/$file
egrep $pattern $file

shows 15 includes:

      {include: "source.jison#comments"}
      {include: "source.jison#include_declarations"}
          {include: "source.jison#comments"}
          {include: "source.jison#comments"}
      {include: "source.jison#options_declarations"}
      {include: "source.jison#user_code_blocks"}
      {include: "source.jison#comments"}
        patterns: [include: "source.jison#user_code_blocks"]
          {include: "source.jison#include_declarations"}
          {include: "source.js"}
      {include: "source.jison#user_code_include_declarations"}
      {include: "source.js"}
      {include: "source.jison#comments"}
      {include: "source.jison#quoted_strings"}
      {include: "source.js#string_escapes"}

There are no includes in the one remaining grammar file. The upshot is that it doesn’t seem like the issue is source.lex scopes in Jison grammars.

It’s probably also worth noting that syntax highlighting seems to work fine in Atom when both language-grammars and language-jison are installed:

Again, I completely understand that it’s up to Linguist to determine whether this is a Linguist issue, but I’d be grateful for any guidance on where to look for what’s breaking syntax highlighting of Jison grammars on GitHub.

[lildude]

🤔 I thought this was resolved already.

You raise some interesting points and I'm starting to wonder if this is due to a change around the new syntax highlighting service (it provides its own Javascript grammar) or a change to the source.js pulled in by other grammars. The former started just before v7.6.0 was released and v7.6.0 included an update to the Javascript grammar.

I'll reopen this and see if I can find some time to dig into this.

[Alhadis]

I knew Tree Sitter was going to cause trouble sooner or later... 😜

[lildude]

Edit: Whoops. I was using the wrong grammar. 🤦 .

🔥 my previous update.

Sidenote: I'm closer to updating Lightshow too as fresh eyes have cleared the road block I initially encountered.

[lildude]

Lightshow has now been updated to Linguist v7.9.0 and it shows proper syntax highlighting for Jison. I'll look into where this is getting messed up some time this week or early next.

[nwhetsell]

Awesome, thank you for looking into this!

[maxbrunsfeld]

This looks like a bug in GitHub's TextMate grammar engine. Lightshow uses an older version than GitHub.com, but there haven't been too many changes.

[maxbrunsfeld]

Ok, after a bit more bisecting, I have determined that the highlighting change was caused by this code change: textmate/textmate#1437. GitHub maintains a private fork of the TextMate highlighting engine, but that ☝️ PR backports a fix that we made into TextMate upstream.

The problem is explained in more detail in that PR, but the TLDR is that the jison or jisonlex grammar contains one or more invalid rules that have a patterns property but no end property.

Previously, a rule like that would work, but it would cause the syntax highlighter to go into a performance death spiral. Now, the patterns property is ignored, in order to maintain acceptable performance.

Here are two examples of the problem in language-jison. There may be others. Sorry for the disruption that this caused. When making this change, we tried to search for examples of the problematic rules in all of the grammars, but we must have missed jison.

I opened this issue on the upstream Jison grammar repo: cdibbs/language-jison#21.

[lildude]

I've now updated Lightshow to use the same version as GitHub.com and confirmed it shows the issue with the original grammar too now.

The update coming in cdibbs/language-jison#22 now renders correctly too: new grammar