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

Broken TypeScript syntax highlighting with newest update #4004

Closed
4 tasks done
MartynasZilinskas opened this issue Jan 26, 2018 · 13 comments
Closed
4 tasks done

Broken TypeScript syntax highlighting with newest update #4004

MartynasZilinskas opened this issue Jan 26, 2018 · 13 comments

Comments

@MartynasZilinskas
Copy link

Preliminary Steps

Please confirm you have...

Problem Description

I suspect the update #4002 broke TypeScript syntax highlighting.

There's a problem with the syntax highlighting of a file

I already opened issue at microsoft/TypeScript-TmLanguage#563 here and they confirmed that is not their problem and everything is working as intended.

Screenshots:
image
https://github.com/SimplrJS/ts-extractor/blob/master/README.md
image
https://github.com/SimplrJS/ts-extractor/blob/master/src/api-registry.ts

@lildude
Copy link
Member

lildude commented Jan 27, 2018

Oh 💩.I think this may have something to do with recent changes to the grammar compiler we use as I can see the language is being correctly detected (Linguist's primary role) but the grammar isn't being applied.

Investigating.

/cc @vmg cos grammar compiler is my early "off the top of my head" suspicion like the Vue issue previously mentioned.

@lildude
Copy link
Member

lildude commented Jan 27, 2018

From microsoft/TypeScript-TmLanguage#563:

This is not the issue in the grammar. Its issue with linguist picking Yaml file where as it should be picking .tmlanguage file.. The yaml files are for helping with the edit and not supposed to be consumed anywhere outside this repository.

Yup, this appears to be more or less what's happened, though I think we've always used the YAML file.

This is comparing one of the test gems I build in the run up to the v6.0.0 release with the v6.0.0 release I can see there is indeed a change:

diff --git a/github-linguist-5.3.3.45.g8cf3bf0a/grammars/source.ts.json b/github-linguist-6.0.0/grammars/source.ts.json
index e58ffd2c..258b363a 100644
--- a/github-linguist-5.3.3.45.g8cf3bf0a/grammars/source.ts.json
+++ b/github-linguist-6.0.0/grammars/source.ts.json
@@ -25,7 +25,7 @@
     },
     "after-operator-block-as-object-literal": {
       "name": "meta.objectliteral.ts",
-      "begin": "(?\u003c=[=(,\\[?+!]|await|return|yield|throw|in|of|typeof|\u0026\u0026|\\|\\||\\*)\\s*(\\{)",
+      "begin": "(?\u003c=[=(,\\[?+!]|{{lookBehindAwait}}|{{lookBehindReturn}}|{{lookBehindYield}}|{{lookBehindThrow}}|{{lookBehindIn}}|{{lookBehindOf}}|{{lookBehindTypeof}}|\u0026\u0026|\\|\\||\\*)\\s*(\\{)",
       "end": "\\}",
       "patterns": [
         {
@@ -233,7 +233,7 @@
       "patterns": [
[... truncated for brevity ...]

This example maps to https://github.com/Microsoft/TypeScript-TmLanguage/blob/master/TypeScript.YAML-tmLanguage#L977-L986

It looks like the variables at https://github.com/Microsoft/TypeScript-TmLanguage/blob/0247d1444a66e683bb4005df38218a0fd9576d03/TypeScript.YAML-tmLanguage#L8-L37 weren't interpolated at all.

The same thing has happened with the tsx grammar:

 diff github-linguist-5.3.3.45.g8cf3bf0a/grammars/source.tsx.json github-linguist-6.0.0/grammars/source.tsx.json    [email protected]
diff --git a/github-linguist-5.3.3.45.g8cf3bf0a/grammars/source.tsx.json b/github-linguist-6.0.0/grammars/source.tsx.json
index 72d73725..3b4950b1 100644
--- a/github-linguist-5.3.3.45.g8cf3bf0a/grammars/source.tsx.json
+++ b/github-linguist-6.0.0/grammars/source.tsx.json
@@ -236,7 +236,7 @@
       "match": "\\S+"
     },
     "jsx-tag-in-expression": {
-      "begin": "(?x)\n  (?\u003c=[({\\[,?=\u003e:*]|\u0026\u0026|\\|\\||\\?|\\Wreturn|^return|\\Wdefault|^)\\s*\n  (?!\u003c\\s*[_$[:alpha:]][_$[:alnum:]]*((\\s+extends\\s+[^=\u003e])|,)) # look ahead is not type parameter of arrow\n  (?=(\u003c)\\s*\n  ([_$a-zA-Z][-$\\w.]*(?\u003c!\\.|-))\n  (?=\\s+(?!\\?)|/?\u003e))",
+      "begin": "(?x)\n  (?\u003c=[({\\[,?=\u003e:*]|\u0026\u0026|\\|\\||\\?|{{lookBehindReturn}}|{{lookBehindDefault}}|^)\\s*\n  (?!\u003c\\s*[_$[:alpha:]][_$[:alnum:]]*((\\s+extends\\s+[^=\u003e])|,)) # look ahead is not type parameter of arrow\n  (?=(\u003c)\\s*\n  ([_$a-zA-Z][-$\\w.]*(?\u003c!\\.|-))\n  (?=\\s+(?!\\?)|/?\u003e))",
       "end": "(/\u003e)|(?:(\u003c/)\\s*((?:[a-z][a-z0-9]*|([_$a-zA-Z][-$\\w.]*))(?\u003c!\\.|-))\\s*(\u003e))",
       "patterns": [
         {
@@ -304,7 +304,7 @@
       }
     },
     "jsx-tag-without-attributes-in-expression": {
-      "begin": "(?x)\n  (?\u003c=[({\\[,?=\u003e:*]|\u0026\u0026|\\|\\||\\?|\\Wreturn|^return|\\Wdefault|^)\\s*\n  (?=(\u003c)\\s*((?:[a-z][a-z0-9]*|([_$a-zA-Z][-$\\w.]*))(?\u003c!\\.|-))?\\s*(\u003e))",
+      "begin": "(?x)\n  (?\u003c=[({\\[,?=\u003e:*]|\u0026\u0026|\\|\\||\\?|{{lookBehindReturn}}|{{lookBehindDefault}}|^)\\s*\n  (?=(\u003c)\\s*((?:[a-z][a-z0-9]*|([_$a-zA-Z][-$\\w.]*))(?\u003c!\\.|-))?\\s*(\u003e))",
       "end": "(?!\\s*(\u003c)\\s*((?:[a-z][a-z0-9]*|([_$a-zA-Z][-$\\w.]*))(?\u003c!\\.|-))?\\s*(\u003e))",
       "patterns": [
         {

I can't see anything that changed in Linguist during this time that could explain this.

@vmg Did you change something in the compiler or docker image that could explain this? If it helps, I built the test github-linguist-5.3.3.45.g8cf3bf0a.gem on 23 Jan 13:24 GMT.

@lildude
Copy link
Member

lildude commented Jan 27, 2018

Hmmm, looks like microsoft/TypeScript-TmLanguage@53d555b happened within the grammar during that window so I don't think it's a compiler change.

Compiling the grammar only produces only a single error/warning:

- [ ] repository `vendor/grammars/TypeScript-TmLanguage` (from https://github.com/Microsoft/TypeScript-TmLanguage) (1 errors)
    - [ ] Unknown keys in grammar: `source.ts` (in `TypeScript.YAML-tmLanguage`) contains invalid keys (`variables`)

... though this isn't going to be relevant as that key was there before those changes and already recorded in #3924.

So it appears the compiler isn't playing nice with the changes introduced in the commit above.

@lildude
Copy link
Member

lildude commented Jan 27, 2018

And just to confirm, I switched back to the old compiler and it handles this change (superfluous output removed):

$ git co v5.3.3
$ rm -rf grammars
$ cd vendor/grammars/TypeScript-TmLanguage
$ git pull
$ git co master
$ $ grep -A5 after-operator-block-as-object-literal: TypeScript.YAML-tmLanguage
  after-operator-block-as-object-literal:
    name: meta.objectliteral.ts
    begin: (?<=[=(,\[?+!]|{{lookBehindAwait}}|{{lookBehindReturn}}|{{lookBehindYield}}|{{lookBehindThrow}}|{{lookBehindIn}}|{{lookBehindOf}}|{{lookBehindTypeof}}|&&|\|\||\*)\s*(\{)
    beginCaptures:
      '1': { name: punctuation.definition.block.ts }
    end: \}
$ cd ~/github/linguist
$ script/convert-grammars
$ grep -A5 after-operator-block-as-object-literal\": grammars/source.ts.json
    "after-operator-block-as-object-literal": {
      "name": "meta.objectliteral.ts",
      "begin": "(?<=[=(,\\[?+!]|^await|[^\\._$[:alnum:]]await|^return|[^\\._$[:alnum:]]return|^yield|[^\\._$[:alnum:]]yield|^throw|[^\\._$[:alnum:]]throw|^in|[^\\._$[:alnum:]]in|^of|[^\\._$[:alnum:]]of|^typeof|[^\\._$[:alnum:]]typeof|&&|\\|\\||\\*)\\s*(\\{)",
      "beginCaptures": {
        "1": {
          "name": "punctuation.definition.block.ts"
$

So this is def a compiler issue. Over to you @vmg 😉

@octref
Copy link

octref commented Jan 28, 2018

@lildude

like the Vue issue

Can you point me to that issue? Couldn't find it here.
I'm using TS / Vue all day and the recent update broke them all.

@lildude
Copy link
Member

lildude commented Jan 28, 2018

@octref there isn't a public issue for it. It was brought to my attention by our support team late on Friday.

The closest is a PR I started #4008 but didn't complete because closer investigation revealed the Vue grammar issue was probably my fault 😊

@lildude
Copy link
Member

lildude commented Jan 28, 2018

@octref there isn't a public issue for it.

There is now. I've opened #4011 to publicly document this.

@octref
Copy link

octref commented Jan 29, 2018

@lildude I've forked the Vue grammar based on TM and made a lot of improvements (even rewrote many sections): https://github.com/vuejs/vetur/commits/master/syntaxes

My Vue plugin for VS Code (https://marketplace.visualstudio.com/items?itemName=octref.vetur) has 1.8M downloads and is used by many Vue developers. If https://github.com/vuejs/vue-syntax-highlight is moving forward with a Sublime specific grammar, I suggest you looking into Vetur's grammar as an alternative: https://github.com/vuejs/vetur/tree/master/syntaxes.

BTW I can also fix the two outstanding issues for Vue in #3924.

@lildude
Copy link
Member

lildude commented Jan 29, 2018

@octref if your grammar is better and more likely to be actively maintained going forward, please review https://github.com/github/linguist/blob/master/CONTRIBUTING.md#changing-the-source-of-a-syntax-highlighting-grammar and send us a PR to switch the sources.

If you could do that ASAP we can use your grammar in the next release that I need to make in the next day or so.

@octref
Copy link

octref commented Jan 29, 2018

OK, if I don't have this ready tonight I'll have it by tomorrow noon.

@vmg
Copy link
Contributor

vmg commented Jan 29, 2018

I think #4012 fixes this. At least from local testing. @lildude can you confirm? :)

@lildude
Copy link
Member

lildude commented Jan 29, 2018

#4012 does indeed fix this. I'm gonna keep this issue open until GitHub.com has been updated, hopefully sometime tomorrow at the latest.

@lildude
Copy link
Member

lildude commented Jan 30, 2018

Linguist v6.0.1 has now been deployed to GitHub.com and TypeScript syntax highlighting is working again, eg https://github.com/github/linguist/blob/master/samples/TypeScript/classes.ts

@lildude lildude closed this as completed Jan 30, 2018
@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.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants