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

Grammar: Start implementing expression grammar and use it for template literals #930

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DZakh
Copy link
Contributor

@DZakh DZakh commented Feb 27, 2024

Highlight int, float, bool, and char embeded in template literals.
Also highlight comments.

Before:

image

After:

image

It's not finished, and there are quite a lot of things that need to be done to make highlighting perfect. But it's already a good improvement, so it can be reviewed and merged.

Comment on lines +124 to +128
{ "include": "#constant" },
{ "include": "#bracketAccess" },
{ "include": "#string" },
{ "include": "#number" },
{ "include": "#character" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There should be much more than these 5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And bracketAccess should be replaced with an array constructor.

Comment on lines +194 to +195
{ "include": "#operator" },
{ "include": "#punctuations" },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of highlighting operators and punctuations separately, they should be part of expression

{ "include": "#template-substitution-element" }
]
},
"template-substitution-element": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Structured it more similar to ts grammar

@zth
Copy link
Collaborator

zth commented Feb 27, 2024

@DZakh it's great that you're having a look at the highlighting! I don't know too much about the highlighting, but I'm happy to merge the PRs outstanding now + more if needed, just that we'll need to help each other out testing.

Actually, I'll do a quick stable release of the features unreleased now, and we can merge these + experiment a little bit more calmly.

@zth
Copy link
Collaborator

zth commented Feb 27, 2024

Could you rebase this please?

@DZakh
Copy link
Contributor Author

DZakh commented Feb 27, 2024

I've figured out how it works, so I'll modify the PR to solve the problem in a more complete way. And the #932 is ready to be merged.

How it works:

  • To highlight all the curly brackets we use a very simple regex to just find { and } and highlight them separately. VSCode or whatever counts opening and closing brackets and colors them accordingly.
  • But the problem arises with ${}, since we want to color the template expression brackets differently. In this case we can't find ${ and } separately, so we use begin/end matcher. But the thing is that when we use begin/end, everything inside is not highlighted anymore, so we need to manually say how it should be highlighted. In our case it should be expression, including comments and other possible things. That's why, I've created this variable (or something like this), so we can reuse it in other places.
  • Unfortunatelly there's another problem you can see in the After screenshot. Eventhough I've added a minimalistic expression highlighting, the brackets are still broken. This is happened to be much more difficult to fix, which I'm going to do after polishing the expression. It's caused by the fact I mentioned in the first point, that we highlight { and } separately. And when we try to find a closing } for the ${ it just grabs any } occurred first. The only way to fix it is to start using begin/end matcher for other places as well, so it know that a } is used by something else, and skips it until finds it's own closing bracket.

@zth
Copy link
Collaborator

zth commented Feb 27, 2024

@DZakh fantastic! And great write up.

@cristianoc
Copy link
Collaborator

Can this be done via semantic highlighting?
We don't have a complete matcher for expressions because it would be difficult to do faithfully in a grammar (lots of conflicts e.g. with < when used in arithmetic, jsx, etc), so the way things are set up currently is that the grammar covers the basic things that are easy to highlight, and leaves the rest to semantic highlighting, with uses the parser's output.

@DZakh
Copy link
Contributor Author

DZakh commented Feb 27, 2024

I don't know much about the semantic highlighting. I'll investigate.

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.

3 participants