Implement HLSL syntax highlighting, and use HLSL grammar to highlight Cg blocks in ShaderLab files #20129

Merged
merged 2 commits into from Feb 20, 2017

Conversation

Projects
None yet
5 participants
@tgjones
Contributor

tgjones commented Feb 7, 2017

Implements #20128.

In contrast to the existing ShaderLab grammar, this new ShaderLab grammar delegates highlighting of Cg blocks to the separate HLSL grammar (which has the advantage that standalone HLSL files can now be highlighted).

I created both the HLSL and ShaderLab grammars by reading the relevant language documentation, so they should both be up-to-date with available types / keywords / etc.

I also looked at the latest csharp.tmLanguage so that my choice of scope names roughly matches what has been done there.

Because .cginc files are basically HLSL files, and not ShaderLab files, I've removed that file extension from the ShaderLab extension and included it in the HLSL extension.

For comparison, here's what ShaderLab syntax highlighting looks like in VSCode 1.9:

shaderlab-before

And here's what it looks like using this new ShaderLab grammar:

shaderlab-after

This is what standalone HLSL syntax highlighting looks like using the new HLSL grammar:

hlsl

This is my first contribution to VSCode - I'm very happy to make changes, just let me know what I got wrong :)

Implement HLSL syntax highlighting
And use HLSL grammar to highlight CG blocks in ShaderLab shaders.
@msftclas

This comment has been minimized.

Show comment
Hide comment
@msftclas

msftclas Feb 7, 2017

Hi @tgjones, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

msftclas commented Feb 7, 2017

Hi @tgjones, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@msftgits

This comment has been minimized.

Show comment
Hide comment
@msftgits

msftgits Feb 7, 2017

Hi, I am closing and re-opening this PR to bump the CLA bot. Sorry for the inconvenience!

msftgits commented Feb 7, 2017

Hi, I am closing and re-opening this PR to bump the CLA bot. Sorry for the inconvenience!

@msftgits msftgits closed this Feb 7, 2017

@msftgits msftgits reopened this Feb 7, 2017

@msftclas

This comment has been minimized.

Show comment
Hide comment
@msftclas

msftclas Feb 7, 2017

Hi @tgjones, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

msftclas commented Feb 7, 2017

Hi @tgjones, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

@msftclas msftclas added the cla-signed label Feb 7, 2017

@msftgits msftgits removed the cla-required label Feb 7, 2017

@mjbvz

This comment has been minimized.

Show comment
Hide comment
@mjbvz

mjbvz Feb 16, 2017

Contributor

@aeschli Can you please take a look at this change. It looks good to me but you know much more about the general language support

Contributor

mjbvz commented Feb 16, 2017

@aeschli Can you please take a look at this change. It looks good to me but you know much more about the general language support

@aeschli

This comment has been minimized.

Show comment
Hide comment
@aeschli

aeschli Feb 17, 2017

Contributor

@tgjones Thanks a lot Tim for the great work. Separating HLSL and ShaderLab sounds like a good idea.
Why I'm hesitating and I wanted to talk to @egamma about it, is that this would be the first grammar that we would 'own', where the original version lives in the VSCode repository. For all other languages we import the grammars from other repositories. Having the grammar in a separate repository has the advantage that the grammar can be maintained independently by the original authors or other committers. There's no need for the VSCode team to review and merge every change. Another benefit is that the grammar can be made available to other editors as well.
https://github.com/Microsoft/TypeScript-TmLanguage is an example for such a repository.
We forward issues to that repository and, from time to time, or on request, import the latest grammar to VSCode.
I'd suggest we follow the same model here.
@tgjones What do you think?
That would mean you create your own github repository that just contains the grammar.
In order for us to use it all we need is a friendly open source licence such as MIT.

Contributor

aeschli commented Feb 17, 2017

@tgjones Thanks a lot Tim for the great work. Separating HLSL and ShaderLab sounds like a good idea.
Why I'm hesitating and I wanted to talk to @egamma about it, is that this would be the first grammar that we would 'own', where the original version lives in the VSCode repository. For all other languages we import the grammars from other repositories. Having the grammar in a separate repository has the advantage that the grammar can be maintained independently by the original authors or other committers. There's no need for the VSCode team to review and merge every change. Another benefit is that the grammar can be made available to other editors as well.
https://github.com/Microsoft/TypeScript-TmLanguage is an example for such a repository.
We forward issues to that repository and, from time to time, or on request, import the latest grammar to VSCode.
I'd suggest we follow the same model here.
@tgjones What do you think?
That would mean you create your own github repository that just contains the grammar.
In order for us to use it all we need is a friendly open source licence such as MIT.

@tgjones

This comment has been minimized.

Show comment
Hide comment
@tgjones

tgjones Feb 17, 2017

Contributor

@aeschli that sounds very sensible to me. In fact I've already created such a repository, because GitHub's Linguist (used for syntax highlighting on github.com) requires grammars to be in a submodule, and I've got an active PR over there for adding HLSL syntax highlighting. I'll modify this PR once I get back to my computer.

Contributor

tgjones commented Feb 17, 2017

@aeschli that sounds very sensible to me. In fact I've already created such a repository, because GitHub's Linguist (used for syntax highlighting on github.com) requires grammars to be in a submodule, and I've got an active PR over there for adding HLSL syntax highlighting. I'll modify this PR once I get back to my computer.

@aeschli

This comment has been minimized.

Show comment
Hide comment
@aeschli

aeschli Feb 20, 2017

Contributor

Cool. What's missing to the PR is just a grammar update script plus an updated OSSREADME.
If you point me to your repository I can make that change.

Contributor

aeschli commented Feb 20, 2017

Cool. What's missing to the PR is just a grammar update script plus an updated OSSREADME.
If you point me to your repository I can make that change.

@aeschli

This comment has been minimized.

Show comment
Hide comment
@aeschli

aeschli Feb 20, 2017

Contributor

Ah, I see https://github.com/tgjones/shaders-tmLanguage. I'll make the update.

Contributor

aeschli commented Feb 20, 2017

Ah, I see https://github.com/tgjones/shaders-tmLanguage. I'll make the update.

@aeschli aeschli merged commit 7128e35 into Microsoft:master Feb 20, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@aeschli aeschli added this to the February 2017 milestone Feb 20, 2017

aeschli added a commit that referenced this pull request Feb 20, 2017

@tgjones

This comment has been minimized.

Show comment
Hide comment
@tgjones

tgjones Feb 20, 2017

Contributor

Great, thanks for merging!

Contributor

tgjones commented Feb 20, 2017

Great, thanks for merging!

@tgjones tgjones deleted the tgjones:colorize-shaders branch Feb 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment