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

[Razor] Textmate colorization fixes #6514

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

allisonchou
Copy link
Contributor

This is less of a fix and more of a hack. I spent a significant amount of time the past few days trying to fix this the proper way - i.e., making major changes to Razor's Textmate grammar - but it was incredibly frustrating and there were edge cases that kept popping up. This solution fixes the problem without a huge investment but makes some compromises.

Problem:

  • The Razor Textmate grammar is very basic and marks a non-trivial amount of proper Razor syntax as invalid.
  • VS Code (via language-configuration.json) marks unbalanced brackets in red. Even setting a semantic token type won't override the red color.
  • One solution would be to change the grammar to account for certain cases, e.g. recognizing => and " as possible valid syntax in component attributes. But because of how buggy our Razor Textmate grammar already is, this turned out to be a much more difficult task than expected, hence the simplified solution in this PR.
     

Solution:

  • Get rid of unbalanced bracket coloring altogether in Razor files. They have a high change of not being accurate since our Textmate grammar isn't great. IMO we could/should eventually use some form of diagnostic to recognize unbalanced brackets instead.
  • There is still some red colorization even after these PR changes, which should be resolved after Parentheses after C# transition are classified differently in syntax tree razor#9371 is fixed.

Before example:
image

After example:
image

@allisonchou allisonchou requested a review from a team October 10, 2023 19:03
"punctuation.definition.string.html"
"punctuation.definition.tag.html"
],
"markupAttributeValue": [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were also missing another semantic classification in our definitions

@@ -5174,7 +5178,10 @@
"entity.other.attribute-name.html"
],
"markupAttributeQuote": [
"punctuation.definition.string.html"
"punctuation.definition.tag.html"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this to be more in line with VS' coloring

@allisonchou allisonchou marked this pull request as ready for review October 10, 2023 19:06
@allisonchou allisonchou requested a review from a team as a code owner October 10, 2023 19:06
@davidwengier
Copy link
Contributor

I think the After picture looks much better

@allisonchou allisonchou merged commit b380b27 into dotnet:main Oct 10, 2023
8 checks passed
@allisonchou allisonchou deleted the dev/allichou/HackyTextmateFix branch October 10, 2023 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants