Use Vetur for Vue grammar #4014

octref · 2018-01-29T19:42:54Z

Using https://github.com/vuejs/vetur instead of https://github.com/vuejs/vue-syntax-highlight for Vue syntax highlighting. Since:

vue-syntax-highlight is moving to Sublime specific grammar.
Vetur is the de-facto extension for Vue development on VS Code with 1.9M download, all 5 star reviews, recommended by Vue and VS Code.

Description

The ./script/add-grammar --replace didn't work, so I looked at previous PR and did it manually.

There are two tests not passing but don't seem to be related to my change.

Changes

The lightshow app doesn't seem to work for me.

Others

I did not find these two errors from #3924 when running test:

- Missing include in grammar: text.html.vue (in vue.YAML-tmLanguage) attempts to include text.slm but the scope cannot be found
- Missing include in grammar: text.html.vue (in vue.YAML-tmLanguage) attempts to include text.pug but the scope cannot be found

/cc @lildude @yyx990803

pchaigno · 2018-01-29T19:57:15Z

The lightshow app doesn't seem to work for me.

Could you elaborate?

Did you write the grammar from scratch?

octref · 2018-01-29T20:03:00Z

@pchaigno OK, got it working...

https://github-lightshow.herokuapp.com/?utf8=%E2%9C%93&scope=from-url&grammar_format=auto&grammar_url=https%3A%2F%2Fgithub.com%2Fvuejs%2Fvetur%2Fblob%2Fmaster%2Fsyntaxes%2Fvue.json&grammar_text=&code_source=from-url&code_url=https%3A%2F%2Fgithub.com%2Fvuejs%2Fvue-syntax-highlight%2Fblob%2F5c2b5afbb3e71c87aca1eda626edbb4506571072%2Fsamples%2Fbasic.vue&code=

Notice the <template> is not being highlighted. I'm addressing it...

Two questions:

In https://github.com/vuejs/vetur/tree/master/syntaxes, I have vue.json and vue-html.json. It seems lightshow only allows one grammar. How do I make it load two? (Or should I merge both json into one?)
When I added vuejs/vetur as a submodule, linguist figured out the grammar "automagically". However I did not know which grammar was loaded, as there are many here...https://github.com/vuejs/vetur/tree/master/syntaxes. Is there any way to specify which grammar to load?

pchaigno · 2018-01-29T20:16:44Z

In https://github.com/vuejs/vetur/tree/master/syntaxes, I have vue.json and vue-html.json. It seems lightshow only allows one grammar. How do I make it load two? (Or should I merge both json into one?)

I don't think it's possible to load several grammars unfortunately. @lildude may know more.

Is there any way to specify which grammar to load?

Linguist knows which grammar to load from the scope field in languages.yml.

octref · 2018-01-29T20:42:14Z

@pchaigno

Did you write the grammar from scratch?

I forked from vue-syntax-highlight but rewrote a lot of sections.

Linguist knows which grammar to load from the scope field in languages.yml.

I have multiple grammars that have scope source.vue.
The use case is we generate a grammar according to user setting to support custom blocks in Vue, so we do have multiple grammars in the repo.

I don't think it's possible to load several grammars unfortunately.

Merging the two json doesn't seem possible. Possible solutions:
- I make a special version of Vetur's grammar, where I mark the content between `<template>` as `html` instead of `vue-html`. Then I only need one grammar.
- I introduce two grammars, `vue` and `vue-html`. `vue-html` is modified from HTML grammar to better support Vue's directive attributes and interpolations.

yyx990803 · 2018-01-29T20:48:17Z

Just for the record, this PR is officially endorsed by the Vue team so it's ok to replace vue-syntax-highlight.

octref · 2018-01-29T20:58:03Z

vue-html vs html.

The major difference is the highlighting of the JS interpolation.

pchaigno · 2018-01-29T21:44:11Z

I forked from vue-syntax-highlight but rewrote a lot of sections.

I am not a lawyer, but as far as I understand the MIT license, you'll need to keep the original copyright notice in addition to your own.

I have multiple grammars that have scope source.vue.

I see. @lildude Do you know which grammar is selected in that case on github.com?

Merging the two json doesn't seem possible. Possible solutions:

AFAIK, it's only Lightshow that can't load multiple grammars at once; it shouldn't be an issue on github.com.

lildude · 2018-01-30T10:53:31Z

The ./script/add-grammar --replace didn't work,

I've looked into this and it's because the compiler can't find your grammar files for some reason:

$ script/add-grammar --replace vue-syntax-highlight https://github.com/vuejs/vetur
Deregistering: vendor/grammars/vue-syntax-highlight
$ git submodule deinit vendor/grammars/vue-syntax-highlight
$ git rm -rf vendor/grammars/vue-syntax-highlight
$ script/grammar-compiler update -f
Registering new submodule: vendor/grammars/vetur
$ git submodule add -f https://github.com/vuejs/vetur vendor/grammars/vetur
$ script/grammar-compiler add vendor/grammars/vetur
  > latest: Pulling from linguist/grammar-compiler
  > Digest: sha256:baa701b92206292e8e2b61afc0c02a521c1122dc513c20fd96781ad134e2c20c
  > Status: Image is up to date for linguist/grammar-compiler:latest
  > source 'vendor/grammars/vetur' contains no grammar files
$

Your grammar has *.json grammar files:

vendor/grammars/vetur/syntaxes/postcss.json
vendor/grammars/vetur/syntaxes/vue-generated.json
vendor/grammars/vetur/syntaxes/vue-html.json
vendor/grammars/vetur/syntaxes/vue.json

... so these should have been found by the compiler.

@vmg, do you know why these files weren't picked up?

The fact they're not being found also explains why you didn't see the two errors.

As for the other questions:

In https://github.com/vuejs/vetur/tree/master/syntaxes, I have vue.json and vue-html.json. It seems lightshow only allows one grammar. How do I make it load two? (Or should I merge both json into one?)

Yup, Lightshow currently only allows selecting one grammar at a time, and you'll understand why in mo.

I have multiple grammars that have scope source.vue.
The use case is we generate a grammar according to user setting to support custom blocks in Vue, so we do have multiple grammars in the repo.
... and ...

I have multiple grammars that have scope source.vue.

I see. @lildude Do you know which grammar is selected in that case on github.com?

GitHub doesn't have support for custom or dynamically generated grammars, nor multiple scopes for a single grammar. GitHub will use the scope for the language Linguist identifies as per the languages.yml

If you want to use multiple scopes, you'll need to uniquely identify them and add them to Linguist as separate uniquely identifiable languages, like CSound has done https://github.com/github/linguist/blob/f7835f71191af350730becc12d5f062698c8e7af/lib/linguist/languages.yml#L875-L902. I'm not sure this is something you can do with Vue though.

On a side note, I need to get an update of Linguist out for GItHub.com today, so I'll be doing so using the current Vue grammar.

vmg · 2018-01-30T11:24:46Z

@lildude: The grammar is in a non-standard location. I guess because it's a VSCode project and not an Atom project. Fixed in #4015

octref · 2018-02-05T22:15:36Z

@lildude Giving it a try again. The only hurdle for me now is in VS Code side, slm is marked as text.jade.slm, whereas in Linguist it's text.slim (since slm seems to be a port of slim with exact syntax: slim-template/slim#526

Since it seems impossible to give scope aliases, I'm planning to split the grammar from Vetur to vuejs/vue-textmate, where I have

/src: yamls and scripts that build/modify grammars
/syntaxes: built json grammars to be used by Linguist
/vetur: built & transformed json grammars to be used by Vetur

I'll then update the PR to:

Introduce a new language, Vue Template, with scope text.html.vue-html (similar to CSound Document)
Replace vue-syntax-highlight with vue-textmate, and modify Vue's scope from text.html.vue to source.vue

Does this sound reasonable to you?

octref · 2018-02-05T22:23:02Z

@lildude One sec, so CSound has only 304 results in GitHub search and you considered adding it. Can we add slm language which has 934 files?

@vmg Thanks for the fix. syntaxes is indeed a VS Code convention.

@pchaigno

I am not a lawyer, but as far as I understand the MIT license, you'll need to keep the original copyright notice in addition to your own.

Sure, I'll do that for vuejs/vue-textmate.

lildude · 2018-02-06T12:11:42Z

Does this sound reasonable to you?

Your explanation seems reasonable to me on the face of it, though I may be missing something that may come up later, but that's why we have multiple reviewers on PRs 😁.

One sec, so CSound has only 304 results in GitHub search and you considered adding it. Can we add slm language which has 934 files?

From a quick look, probably not, for three reasons:

.csound is not an extension associated with any language in languages.yml, probably due to lack of use
The usage of .slm is a little on the low side, even assuming all the .slm files found are Slm - several appear not to be
If appears to be an almost complete and compatible port of Slim as opposed to being a completely new language. This is going to make differentiating Slm files using the .slm extension from Slim files using the .slm extension almost impossible.

Reasoning for the last point: .slm isn't currently associated with any language within Linguist, but anyone using it now for Slim will be getting this matched (as HTML due to the group) through the classifier, assuming it gets it right. Adding this extension for a new language called "Slm" will mean all of those files will be identified as Slm by extension alone which will cause complaints. This is normally solved by adding the extension to both languages and using a heuristic to differentiate the two, and that's where I think this proposal is going to fall down.

octref · 2018-02-06T18:27:44Z

I've updated PR. Here are the test errors from bundle exec rake test

1) Error:
TestRepository#test_incremental_stats:
ArgumentError: wrong number of arguments (given 6, expected 0)
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:142:in `each_delta'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:142:in `each'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:142:in `any?'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:142:in `compute_stats'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:116:in `cache'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:68:in `languages'
    /Users/octref/Code/fiddle/linguist/test/test_repository.rb:43:in `test_incremental_stats'

  2) Error:
TestRepository#test_commit_with_git_attributes_data:
NoMethodError: undefined method `new_file' for #<Rugged::Patch:70108708233620>
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:142:in `block in compute_stats'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:142:in `each_patch'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:142:in `each'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:142:in `any?'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:142:in `compute_stats'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:116:in `cache'
    /Users/octref/Code/fiddle/linguist/lib/linguist/repository.rb:98:in `breakdown_by_file'
    /Users/octref/Code/fiddle/linguist/test/test_repository.rb:79:in `test_commit_with_git_attributes_data'

  3) Failure:
TestLanguage#test_codemirror_modes_present [/Users/octref/Code/fiddle/linguist/test/test_language.rb:424]:
#<Linguist::Language name=Vue> missing CodeMirror MIME mode

1145 runs, 21687 assertions, 1 failures, 2 errors, 1 skips

You have skipped tests. Run with --verbose for details.

1 and 2 I don't quite understand. 3 I'm wondering why it's failing as vue seems to be a valid codemirror mode: https://github.com/codemirror/CodeMirror/tree/master/mode/vue

octref · 2018-02-06T18:29:05Z

@pchaigno I also added third party licenses notice to Vetur repo: https://github.com/vuejs/vetur/blob/master/ThirdPartyNotices.txt

Is it sufficient or do I need to have a copy of vue-syntax-highlight's license in this repo too?

lildude · 2018-02-07T12:12:05Z

lib/linguist/languages.yml

  ace_mode: html
+  codemirror_mode: vue


1) Failure: TestLanguage#test_codemirror_modes_present [/home/travis/build/github/linguist/test/test_language.rb:424]: #<Linguist::Language name=Vue> missing CodeMirror MIME mode

... is happening because you're missing a codemirror_mime_type: <blah> entry right about here.

lildude · 2018-02-07T12:15:26Z

lib/linguist/languages.yml

+  type: markup
+  tm_scope: text.html.vue-html
+  ace_mode: html
+  codemirror_mode: vue


You'll need a codemirror_mime_type: <blah> here too.

lildude · 2018-02-07T12:16:44Z

lib/linguist/languages.yml

  language_id: 391
+Vue Template:
+  type: markup


As this is essentially HTML, it's probably a good idea to group it with HTML(add group: HTML to this section) like the many other HTML templating languages do.

octref · 2018-02-08T17:59:08Z

Thanks for the feedback! Think I addressed them.

However I think codemirror_mime_type should be described on top of languages.yml.
Also, the contributing part did not mention the docker requirement, but it's needed for running some commands.

pchaigno · 2018-02-14T08:54:04Z

lib/linguist/languages.yml

  ace_mode: html
+  codemirror_mode: vue
+  codemirror_mime_type: text/html


The Travis build is failing because this reference cannot be found in CodeMirror. I think you want text/x-vue.

Thanks! Was busy with other things and didn't notice Travis was failing.

pchaigno · 2018-02-14T08:54:26Z

@pchaigno I also added third party licenses notice to Vetur repo: https://github.com/vuejs/vetur/blob/master/ThirdPartyNotices.txt
Is it sufficient or do I need to have a copy of vue-syntax-highlight's license in this repo too?

I don't know; I'm not a lawyer. In any case, it's for you to decide and it is not blocking for the PR to be merged.

yyx990803 · 2018-02-14T09:26:04Z

I can clear the license problems as the author of vue-syntax-highlight.

pchaigno · 2018-02-14T21:57:38Z

The new Travis-CI error is unrelated to this pull request.

octref · 2018-03-01T05:58:10Z

@lildude I think I fixed all your requested changes. Anything still blocking?

lildude · 2018-03-01T10:50:11Z

Anything still blocking?

Only a review from one of the other collaborators that is needed before I will put my stamp in it 😄

pchaigno · 2018-03-01T11:06:12Z

I'm not sure I followed here (which is the reason why I restrained from approving directly); do we have a working grammar in the end? Is there a fully-working Lightshow example?

lildude · 2018-06-21T14:22:37Z

Nudge. Looks like we're still looking for a response to:

I'm not sure I followed here (which is the reason why I restrained from approving directly); do we have a working grammar in the end? Is there a fully-working Lightshow example?

You'll also need to address the conflicts before this PR can be merged.

octref · 2018-06-21T14:56:39Z

@pchaigno Currently Lightshow is offline, but even when it's online, it cannot handle showing 2 grammars. I have links in this comment: #4014 (comment)

What the vue grammar does: splitting each region into embedded languages, like:

<template> -> vue-html
<style> -> css
<style lang="sass"> -> sass

What the vue-html grammar does: it handles vue-specific interpolations and differ from html grammar. That's why it has to be another grammar.

Lightshow can load either grammar directly: https://github.com/vuejs/vetur/blob/master/syntaxes/vue.json and https://github.com/vuejs/vetur/blob/master/syntaxes/vue-html.json. The expected glitch is when you load vue grammar, since vue-html grammar cannot be loaded due to Lightshow limitation, everything inside the <template></template> region will not be tokenized and highlighted.

Let me know any more questions you have. I'll solve the merge conflicts soon.

lildude · 2018-06-21T16:17:31Z

Currently Lightshow is offline

Ooops. I meant to post an issue for this this morning. Just done it now - #4168

lildude · 2018-10-17T14:28:58Z

Lightshow is back (forgot to come back and nudge this one when it came back). Please update your fork.

stale · 2018-11-16T19:36:37Z

This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

stale · 2018-11-30T19:59:09Z

This pull request has been automatically closed because it has not had activity in a long time. Please feel free to reopen it or create a new issue.

octref · 2018-11-30T20:54:46Z

I'll open another one later. Hope lightshow works this time...

Use Vetur for Vue grammar. Fix github-linguist#4011

610b3ef

octref force-pushed the master branch from 78663aa to 610b3ef Compare February 6, 2018 18:18

minor tweak for vue/vue-html language definition

3fd4087

lildude requested changes Feb 7, 2018

View reviewed changes

codemirror mime type and group info

1e9eeee

pchaigno reviewed Feb 14, 2018

View reviewed changes

update codemirror_mime_type

4ee5e13

Merge branch 'master' into master

ae3c3a6

pchaigno added the Awaiting Lightshow label Jul 21, 2018

lildude removed the Awaiting Lightshow label Oct 17, 2018

stale bot added the Stale label Nov 16, 2018

stale bot closed this Nov 30, 2018

github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Vetur for Vue grammar #4014

Use Vetur for Vue grammar #4014

octref commented Jan 29, 2018

pchaigno commented Jan 29, 2018

octref commented Jan 29, 2018

pchaigno commented Jan 29, 2018

octref commented Jan 29, 2018 •

edited

Loading

yyx990803 commented Jan 29, 2018

octref commented Jan 29, 2018

pchaigno commented Jan 29, 2018

lildude commented Jan 30, 2018 •

edited

Loading

vmg commented Jan 30, 2018

octref commented Feb 5, 2018

octref commented Feb 5, 2018

lildude commented Feb 6, 2018 •

edited

Loading

octref commented Feb 6, 2018

octref commented Feb 6, 2018

lildude Feb 7, 2018

lildude Feb 7, 2018

lildude Feb 7, 2018

octref commented Feb 8, 2018

pchaigno Feb 14, 2018

octref Feb 14, 2018

pchaigno commented Feb 14, 2018

yyx990803 commented Feb 14, 2018

pchaigno commented Feb 14, 2018

octref commented Mar 1, 2018

lildude commented Mar 1, 2018

pchaigno commented Mar 1, 2018

lildude commented Jun 21, 2018

octref commented Jun 21, 2018

lildude commented Jun 21, 2018

lildude commented Oct 17, 2018

stale bot commented Nov 16, 2018

stale bot commented Nov 30, 2018

octref commented Nov 30, 2018

Use Vetur for Vue grammar #4014

Use Vetur for Vue grammar #4014

Conversation

octref commented Jan 29, 2018

Description

Changes

Others

pchaigno commented Jan 29, 2018

octref commented Jan 29, 2018

pchaigno commented Jan 29, 2018

octref commented Jan 29, 2018 • edited Loading

yyx990803 commented Jan 29, 2018

octref commented Jan 29, 2018

pchaigno commented Jan 29, 2018

lildude commented Jan 30, 2018 • edited Loading

vmg commented Jan 30, 2018

octref commented Feb 5, 2018

octref commented Feb 5, 2018

lildude commented Feb 6, 2018 • edited Loading

octref commented Feb 6, 2018

octref commented Feb 6, 2018

lildude Feb 7, 2018

Choose a reason for hiding this comment

lildude Feb 7, 2018

Choose a reason for hiding this comment

lildude Feb 7, 2018

Choose a reason for hiding this comment

octref commented Feb 8, 2018

pchaigno Feb 14, 2018

Choose a reason for hiding this comment

octref Feb 14, 2018

Choose a reason for hiding this comment

pchaigno commented Feb 14, 2018

yyx990803 commented Feb 14, 2018

pchaigno commented Feb 14, 2018

octref commented Mar 1, 2018

lildude commented Mar 1, 2018

pchaigno commented Mar 1, 2018

lildude commented Jun 21, 2018

octref commented Jun 21, 2018

lildude commented Jun 21, 2018

lildude commented Oct 17, 2018

stale bot commented Nov 16, 2018

stale bot commented Nov 30, 2018

octref commented Nov 30, 2018

octref commented Jan 29, 2018 •

edited

Loading

lildude commented Jan 30, 2018 •

edited

Loading

lildude commented Feb 6, 2018 •

edited

Loading