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

Support modifiers + patterns #38

Merged
merged 3 commits into from
Jan 30, 2023

Conversation

DanTup
Copy link
Collaborator

@DanTup DanTup commented Jan 30, 2023

This expands the tests to include some class modifiers and patterns. It also adds a new file 'support.dart' that is excluded from the golden tests as a place to put some declarations that would otherwise add lots of noise to the test files.

The first commit adds the new test files and generates goldens without any highlighting changes, and the second commit has the changes to syntax + results in goldens.

From a quick scan over these examples (which I copied from the feature spec, but I don't know if I've covered everything), things generally look ok with the exception of these switch expressions:

Screenshot 2023-01-30 at 16 13 17

We have a rule that assumes that foo => bar means foo is a function, so it gets coloured yellow here. This is incorrect here. Given the way this syntax works is very simple and not a full parser, I don't know if it's going to be simple to handle this, but while I figure that out I thought it was worth landing the basic support so keywords are recognised.

Here's an larger example of how the highlighting currently looks in VS Code:

Screenshot 2023-01-30 at 16 17 27

@devoncarew @jwren

+ also add a `support.dart` for test files that is excluded from testing to avoid too much noise in the test files.
@DanTup
Copy link
Collaborator Author

DanTup commented Jan 30, 2023

Oh, FWIW the highlighting above is not what VS Code users will generally see, because Semantic Tokens will be layered over the top, but it's what they'll see until those arrive, roughly what they'll see with a theme that doesn't support semantic tokens, or this grammar used elsewhere (other editors, DevTools, GitHub, etc.).

@jwren jwren self-requested a review January 30, 2023 17:08
@devoncarew
Copy link
Member

Looks great!

re:

We have a rule that assumes that foo => bar means foo is a function, so it gets colored yellow here. This is incorrect here.

Perhaps track this as an issue? I didn't look into the grammar, but perhaps we could use more contextual info to eliminate false positives? For example, a function would need to be proceeded by get or followed by (....

@DanTup
Copy link
Collaborator Author

DanTup commented Jan 30, 2023

@devoncarew I just snook in a fix for this. I looked at the code and found that this seemed bogus - we were already handling parens, but had this extra code to handle where they didn't exist (basically getters). This was inconsistent with the colouring we get from semantic tokens (where getters are not coloured like functions), so I've removed it.

The changes in goldens are the ones we want (patterns.dart/records.dart) and removing the function colouring from some getters (which again, I think we want).

@devoncarew devoncarew merged commit b6324bf into dart-lang:master Jan 30, 2023
@devoncarew
Copy link
Member

This was inconsistent with the colouring we get from semantic tokens (where getters are not coloured like functions), so I've removed it.

Sounds great - thanks for the update.

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