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

Update TextMate grammar to properly represent Razor content. #14287

Closed
5 tasks done
NTaylorMullen opened this issue Aug 22, 2018 · 8 comments
Closed
5 tasks done

Update TextMate grammar to properly represent Razor content. #14287

NTaylorMullen opened this issue Aug 22, 2018 · 8 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates cost: L Will take from 5 - 10 days to complete Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one Razor Tooling Big Rock

Comments

@NTaylorMullen
Copy link
Contributor

NTaylorMullen commented Aug 22, 2018

Right now we're re-using the built-in Razor TextMate grammar but it has its own flaws. One of which is the following end </p> tag is not colored correctly: <p>@DateTime.Now</p>


Updates after initial release

Have done a lot of work to get the Razor grammar into a usable state for most scenarios. That being said there are several known things I still haven't done:

  • Markin in functions
    image

  • Razor templates
    image

  • await foreach
    image

  • Email addresses
    image

  • Nested HTML in non-transitioned C# constructs
    image

  • (Optional) Known TagHelpers. This can technically be done at the semantic colorization level as well
    image

  • (Optional) TagHelper C# attribute content.This can technically be done at the semantic colorization level as well
    image


Areas for improvement

  • Mark embedded languages in the grammar and extension configuration. This will enable things such as contextual comments (C# comment = // or /* ... */, or HTML = <!-- ... -->) and specific embedded language brace matching/indentation.

  • Customize default VSCode themes to make the Razor colorizations more familiar. For instance, in VS the @ transitions are yellow and the C# bits that follow are distinct from HTML.

@NTaylorMullen NTaylorMullen changed the title Update TextMateGrammer to properly represent Razor content. Update TextMate grammar to properly represent Razor content. Aug 27, 2018
@NTaylorMullen
Copy link
Contributor Author

This new grammar should be in its own repo a lot like https://github.com/Microsoft/TypeScript-TmLanguage

That way VSCode itself will ship our grammar.

@NTaylorMullen
Copy link
Contributor Author

https://github.com/aspnet/Razor.VSCode/issues/383#issue-473664717 has some very specific examples on when this falters. @danroth27 this is by far our number 1 area for improvement in Razor VSCode, issues piling up as dups 😄. I also haven't linked every dup issue so there's more than just mentioned here, i'll get better at that!.

@Eilon Eilon transferred this issue from aspnet/Razor.VSCode Sep 23, 2019
@Eilon Eilon assigned NTaylorMullen and unassigned NTaylorMullen Sep 23, 2019
@Eilon Eilon added this to the 1.0.0-alpha4 milestone Sep 23, 2019
@Eilon Eilon added 1 - Ready area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates cost: L Will take from 5 - 10 days to complete enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Sep 23, 2019
NTaylorMullen added a commit to dotnet/razor that referenced this issue Nov 23, 2019
- Used the suggested yml format to write our grammar given it will definitely grow in complexity. See [VSCode's guidance](https://code.visualstudio.com/api/language-extensions/syntax-highlight-guide#using-yaml-to-write-a-grammar) for more info. One thing to note, the utility to go from YAML to JSON was already a dependency of our extension.
- Added a compile TextMate task to VSCode to enable easy Run Extension -> Edit grammar -> Re-compile text mate -> reload Experiemental instance dev flows.
- Replaced the existing Razor grammar (yes this means we will lack coloring for a period of time) to ensure we can catch bugs with the in-progress grammar quicker.
- Updated the VSCode's yml settings to have the proper tab size.
- Added a simple grammar to start (escaped Razor transitions).
- Eventually this grammar will need to live in its own repo but until we get it into a reasonable state we're going to group the grammar along side the extension to ease development.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Nov 23, 2019
- Used the suggested yml format to write our grammar given it will definitely grow in complexity. See [VSCode's guidance](https://code.visualstudio.com/api/language-extensions/syntax-highlight-guide#using-yaml-to-write-a-grammar) for more info. One thing to note, the utility to go from YAML to JSON was already a dependency of our extension.
- Added a compile TextMate task to VSCode to enable easy Run Extension -> Edit grammar -> Re-compile text mate -> reload Experiemental instance dev flows.
- Replaced the existing Razor grammar (yes this means we will lack coloring for a period of time) to ensure we can catch bugs with the in-progress grammar quicker.
- Updated the VSCode's yml settings to have the proper tab size.
- Added a simple grammar to start (escaped Razor transitions).
- Eventually this grammar will need to live in its own repo but until we get it into a reasonable state we're going to group the grammar along side the extension to ease development.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Nov 23, 2019
- In order to programatically parse anything with the Razor grammar we need to reconstruct an environment that's similar to VSCode where grammar's for C#, HTML, JavaScript and CSS exist. To do this I grabbed all of Razor's embedded language grammars to ensure we can construct a valid Razor TextMate grammar parser.
- Copied existing unit test boiler plate (jest.config.js etc.) to a new `Microsoft.AspNetCore.Razor.VSCode.Grammar.Test` project.
- Added VSCode utilities to enable running and debugging of grammar tests directly in Visual Studio code.
- Built test utilities to:
  1. Tokenize a content with Razor's grammar.
  2. Generate snapshot contents (a serialized form of tokenized content).
- Used Jest's [built-in snapshot testing](https://jestjs.io/docs/en/snapshot-testing) to build a simple testing suite that Tokenizes Razor content -> Serializes it -> Compares it to a baseline (or updates).
- Ensured that command line testing works as expected as well via `yarn jest` to run tests and `yarn jest -u` to update snapshots for tests.
- Added an escaped transitions grammar test as an example to show how all future tests will be constructed.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Nov 24, 2019
- In order to programatically parse anything with the Razor grammar we need to reconstruct an environment that's similar to VSCode where grammar's for C#, HTML, JavaScript and CSS exist. To do this I grabbed all of Razor's embedded language grammars to ensure we can construct a valid Razor TextMate grammar parser.
- Copied existing unit test boiler plate (jest.config.js etc.) to a new `Microsoft.AspNetCore.Razor.VSCode.Grammar.Test` project.
- Added VSCode utilities to enable running and debugging of grammar tests directly in Visual Studio code.
- Built test utilities to:
  1. Tokenize a content with Razor's grammar.
  2. Generate snapshot contents (a serialized form of tokenized content).
- Used Jest's [built-in snapshot testing](https://jestjs.io/docs/en/snapshot-testing) to build a simple testing suite that Tokenizes Razor content -> Serializes it -> Compares it to a baseline (or updates).
- Ensured that command line testing works as expected as well via `yarn jest` to run tests and `yarn jest -u` to update snapshots for tests.
- Added an escaped transitions grammar test as an example to show how all future tests will be constructed.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Nov 24, 2019
- Added tests to cover simple and complex explicit expression cases.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Dec 14, 2019
- Prior to this `[email protected]` would treat `@contoso.com` as an email address.
- Added tests to validate implicit expressions properly ignore email addresses but also continue to work in situations where there are clear word boundaries.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Dec 26, 2019
- Added tests to validate the new await foreach syntax.
- Updated snapshots for tests.

dotnet/aspnetcore#14287
@NTaylorMullen NTaylorMullen added the blocked The work on this issue is blocked due to some dependency label Dec 26, 2019
@NTaylorMullen
Copy link
Contributor Author

NTaylorMullen commented Dec 26, 2019

The only required pieces of the grammar that are yet to be completed are intermingling HTML into deep C# structures. Blocking issues for this are tracked by microsoft/vscode-textmate#122, microsoft/vscode#87496

Now for why, when we hand off the grammar to the C# grammar it doesn't always give us the opportunity to enable HTML constructs. For top level constructs like @if (...) { we understand everything in the declaration portion, the keyword, condition and opening brace. This means we can allow for HTML inside of the body (the opening brace) because we declare what the body allows; however, in the case of:

@code {
    public void RenerHTML()
    {
        // Content
    }
}

we don't understand method declarations (Razor hasn't had a need to understand them). This means when we let the C# grammar parse the body of @code it will understand method declarations in their entirety and never unweave to give us the opportunity to "allow" HTML.

The two issues I have filed on VSCode take advantage of a TextMate grammar concept called injection grammars. Basically I had the idea that we could tie into specific C# constructs with an injection grammar to give us an opportunity to allow HTML. Turns out they don't currently support injections into child grammars, aka when a top-level grammar (aspnetcorerazor) calls into a child grammar (source.cs).

If VSCode isn't willing to fix the issue we'll need to copy the C# grammar wholesale and edit each piece individually. Or if VS' TextMate grammar support doesn't support injection grammars either we'll have to do this as well.

Leaving this issue as blocked until we hear back from the VSCode and VS folks (i've started those conversations).

NTaylorMullen added a commit to dotnet/razor that referenced this issue Dec 26, 2019
- Added tests to validate the new await foreach syntax.
- Updated snapshots for tests.

dotnet/aspnetcore#14287
@NTaylorMullen
Copy link
Contributor Author

Hmm, VSCode's PHP grammar does similar things and they've been able to get this to work. When I have time i'll have to look into this more.

@NTaylorMullen
Copy link
Contributor Author

Heard back from VSCode folks and if we change how we inject we can get this to work. Unblocking

@NTaylorMullen NTaylorMullen removed the blocked The work on this issue is blocked due to some dependency label Jan 21, 2020
NTaylorMullen added a commit to dotnet/razor that referenced this issue Jan 28, 2020
- Added tests to validate the new Razor template syntax.
- Updated snapshots for tests.
- Arcade must have made a change restricting the VSCode grammar tests project from installing/building successfully on my box and I found that the way the SDK was being imported was breaking the way the Oniguruma library would compile itself. This didn't seem to happen on the CI but in an effort to ensure it can build everywhere I added a Directory.Build.props file that purposefully suppresses SDK imports (since it's an npmproj anyways) to avoid breaking the C++ build that takes place on install.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Jan 28, 2020
- Utilized VSCode's grammar injection feature to enable Razor templates. Basically what's going on in this changeset is we describe an "injection" into the aspnetcorerazor grammar that states (`R:text.aspnetcorerazor`) if a previous Razor rule can't fulfill the current set of tokens to then allow our template rules to take precedence. This ends up also injecting itself in nested C# statements where we'd expect Razor templates.
- Arcade must have made a change restricting the VSCode grammar tests project from installing/building successfully on my box and I found that the way the SDK was being imported was breaking the way the Oniguruma library would compile itself. This didn't seem to happen on the CI but in an effort to ensure it can build everywhere I added a Directory.Build.props file that purposefully suppresses SDK imports (since it's an npmproj anyways) to avoid breaking the C++ build that takes place on install.
- Added tests to validate the new Razor template syntax.
- Updated snapshots for tests.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Jan 30, 2020
- Utilized VSCode's grammar injection feature to enable Razor templates. Basically what's going on in this changeset is we describe an "injection" into the aspnetcorerazor grammar that states (`R:text.aspnetcorerazor`) if a previous Razor rule can't fulfill the current set of tokens to then allow our template rules to take precedence. This ends up also injecting itself in nested C# statements where we'd expect Razor templates.
- Arcade must have made a change restricting the VSCode grammar tests project from installing/building successfully on my box and I found that the way the SDK was being imported was breaking the way the Oniguruma library would compile itself. This didn't seem to happen on the CI but in an effort to ensure it can build everywhere I added a Directory.Build.props file that purposefully suppresses SDK imports (since it's an npmproj anyways) to avoid breaking the C++ build that takes place on install.
- Added tests to validate the new Razor template syntax.
- Updated snapshots for tests.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Jan 30, 2020
- Utilized VSCode's grammar injection feature to enable Razor markup in functions. Basically what's going on in this changeset is we describe an "injection" into the code/functions directive and Razor code blocks grammar that states if we encounter a { inside of a Razor C# block that's not a string or comment, then tokenize the contents of the {} as typical C#. This ends up also injecting itself in nested C# statements where we'd allow nested markup.
- Updated existing tests and added a lot of new ones to cover several edge cases.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Jan 30, 2020
- Utilized VSCode's grammar injection feature to enable Razor markup in functions. Basically what's going on in this changeset is we describe an "injection" into the code/functions directive and Razor code blocks grammar that states if we encounter a { inside of a Razor C# block that's not a string or comment, then tokenize the contents of the {} as typical C#. This ends up also injecting itself in nested C# statements where we'd allow nested markup.
- Updated existing tests and added a lot of new ones to cover several edge cases.

dotnet/aspnetcore#14287
NTaylorMullen added a commit to dotnet/razor that referenced this issue Jan 30, 2020
- Utilized VSCode's grammar injection feature to enable Razor markup in functions. Basically what's going on in this changeset is we describe an "injection" into the code/functions directive and Razor code blocks grammar that states if we encounter a { inside of a Razor C# block that's not a string or comment, then tokenize the contents of the {} as typical C#. This ends up also injecting itself in nested C# statements where we'd allow nested markup.
- Updated existing tests and added a lot of new ones to cover several edge cases.

dotnet/aspnetcore#14287
@NTaylorMullen
Copy link
Contributor Author

Syntactic colorization work complete!

@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 2020
@NTaylorMullen NTaylorMullen added the Done This issue has been fixed label Apr 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates cost: L Will take from 5 - 10 days to complete Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one Razor Tooling Big Rock
Projects
None yet
Development

No branches or pull requests

5 participants