Hey there. Thanks for the great plugin. I'm the author of the Pro Colors themes. What I wanted to achieve with this theme was to have every token colored as they should be. This is an example of that in TypeScript:

Every token is colorized. Now, I came across coding with Flutter that I realized the grammar has massive problems and that is not something that I liked:

So I worked into the grammar and fixed and improved as many things as I could, while at the same time adding first-class support to Pro Colors. the result of that is this:

Changes

• So there are lots of things that got changed. First of all, I formatted the code for readability with Virtuous

• ~/ used to be rendered as two separate operators, it's fixed now.

• class definitions added.

• function definitions added.

• punctuations now have grammars, and can be colorized separately, and have the right priority in the rules.

• every keyword and type now includes a $1 in their definition, so for example, keyword.control.dart becomes keyword.control.if.dart which makes it possible for theme authors like me to customize every single word as we wish.

• Type Parameters <T> are colorized in the right way:

• Object Literal Keys are added:

• fixed the rule name "variable.other.source.dart to comment.block.documentation.dart

• matching the => and ( after the function identifier

• moving all the root patterns to the block repository (to use for string interpolation)

• moving $ into the string interpolation variable

• better string interpolation grammar

Added coloring of the object before the dot notation:

The class definition has some problems, I'm going to work on that...

Thanks for the contribution! I haven't had chance to look at this in detail yet, but I'll try to do so next week (although I may wait until you think you're done first).

I had been planning to look at this a little in the near future with a goal of making the textmate grammar more closely match the semantic tokens output (see #4062 for one example of why). We'll need to make sure that any changes made in this PR don't make that any worse (I suspect if anything they will make it better, but it's something we'll need to verify). I'd really like to have some better (automated) testing for that since right now changes to this file can be risky, but it's not something that exists today.

Once we are happy with this, we should send the PR to https://github.com/dart-lang/dart-syntax-highlight since that's really the official source of truth for this file now.

So I worked a little bit on the class definitions and now it includes the inherited class as well:

@DanTup thanks for checking it out. I actually used the JavaScript/TypeScript definitions as a reference (since they are the best grammars implemented so far) and then used their naming (thought well, most themes are targeting those so for example if I change entity.other.inherited-class.js to .dart it'll make the most themes work out of the box with Dart too. So hope it was not a bad break.)

However, Thanks a lot for the heads up! I'll be forking that then :) and probably we can work on it together there

@pouyakary I had a quick compare of this versus the current grammar. Some items like get set as identifiers look better now (they're no longer coloured as keywords), however it looks like String has changed now to no longer be coloured like a class (left is before, right is after):

While String is a built-in type, it's coloured the same as a class when using Semantic Tokens and I definitely don't want to introduce any changes that make additional differences to that (the fewer differences between the grammar and the semantic tokens, the less "flicker" there is when opening a new file when you see the grammar briefly and then the semantic tokens). For references, the same code using semantic tokens looks like this:

I've also added pushed a change to master that now dumps the tokens to snapshot files (see c43941e2 and 65a577da) using the vscode-tmgrammar-test package that will make verifying changes to the grammar a little easier (although the current test files are quite simple).

If you rebase on master you can run npm run update-grammar-snapshots which will re-generate those snapshots with your changes (which will be required for the tests to pass, allowing the changes to be reviewed in the PR). Feel free to add additional code to the files in the grammar_testing project if there are specific things missing you'd like to ensure don't regress.

(Ultimately I'd like to move tests like that to the dart-lang/syntax-highlight repo, but since this is already using TS/npm it was easier to try out here).

Fixed the problem with the class definition where the generics in the extending type did not color:

@DanTup thanks a lot for your awesome investigation 👍🏻 Let me fix the String and get/set right away and go to the tests...

@DanTup So I had counted Strings as primitive types and it seems that they are rendered correctly as primitive. But I can change the string to a class maybe? I don't know that, please guide me on what to expect of strings?

@pouyakary yep, I think we should treat String as a class. This matches the existing grammar and semantic tokens. I understand String is perhaps a little odd because it could be argued to belong to two categories (similar to void), but since a goal here is not to diverge further from the semantic tokens colouring, I think we should keep it as-is.

@DanTup sure thing, I'll fix it then.

Spread operator added (referencing JS as always)

Added the optionality keyword:

Cool, thanks!

I've started some work to try and get some tests into the dart-syntax-highlighting repo, which should make it easier to verify the impact of these changes.

I think we should try to minimise the non-functional changes in this change to make it easier to review. For example the original file is indented with tabs, but this PR changes everything to spaces (which makes every line look like it was modified). If you can re-format using tabs, the diff will be much smaller and easier to review. Thanks!

Fixed the problem with nested generics

Fixed the problem with nested generics

Nice! I just discovered that issue recently while working on some testing support for this and hadn't gotten to looking at it yet.

I've made some steps towards having better automated tests using the DevTools parser (see https://github.com/flutter/devtools/commit/91cf832f9983698597f3030e02473643a73da930), although there are some issues with it right now (it doesn't produce the correct spans compared to other tools). I'd like to resolve those issues so we can then get tests also in the dart-syntax-highlighting repo, which will make it easier to accept larger changes like this without fear of regressing anything.

Are you able to align the indenting in this file with the original so the diff is just what's functionally changed, and remove the changes to the vscode/settings file? That will make it easier to review once everything is ready.

Thanks!

@DanTup I actually can I guess, since every different atomic change has been a commit. So even the formatting is a commit. I don't know if you have seen the history of it, but can I ask you to do so? I have an initial commit that the original version formatted. And then on top of that I have committed new changes. so it's readable what is changed. However I can recommit all of those commits on the original version (although I find this one more readable).

But the thing is: If I'm going to recommit this all with the original format, lets as well do it in the upstream repo so that it contains the history. we can stop this PR here and move that all there?

I understand, but it's far easier for me to review a single diff than go through all 28 commits and review them individually. I don't think changing the tabs to spaces is an improvement to readability. Tab sizes can easily be controlled in the editor but spaces cannot. Additionally, changing every lines indenting makes it much more difficult to track down regressions if they occur, because you have to step through more layers of history to find when a line was last modified.

Sending the PR to dart-syntax-highlighting is fine too. They're both essentially the same file and can be copied either way (from there to here or here to there). In either case, it'll need careful review and testing.

Thanks!

Okay then let me move to that repo and recommit these there with the proper style.

A new full grammar for the get / set pattern so that the next token after them are marked as a function definition (as what they are)

So I didn't know that the get could be async and even generator :) how cool is that :) anyhow, that is fixed:

Extension definition grammar is added:

---

@DanTup sorry that I haven't had a chance to work on the tests. Still working on the grammar as I learn the language

np, I haven't finished setting up testing for the dart-syntax-highlighting repo yet. But what we could do in the meantime is add more files in https://github.com/Dart-Code/Dart-Code/tree/master/src/test/test_projects/grammar_testing/lib that cover the cases you've been fixing and testing. We could add them and generate the snapshot files in another PR, and then rebase this and re-generate them. That way this diff would include both the grammar changes, and the changes to the snapshots, which will make it easier to review and confirm everything is working as expected.

The extends keyword didn't get color in the generics, that is fixed now:

@DanTup awesome

I just noticed that the name in the extension definition is optional and so I made it so:

Class names (in definition) can now also be lowercase.

Added the generics after the destination: