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 c43941e and 65a577d) 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).

Fixe a problem with nested generics that looked like this:

Which is now resolved:

String is now a class and all classes can be customizable in the themes:

The precedence of assignment operators are fixed so += will no longer be colored as + and =

I encountered a problem with a dart library that had uppercase function names. And the grammar counted them as classes:

I have to play with the grammar to fix this but for now, I just fixed it enough to not recognize it as anything:

There was a problem with function definition grammar where it counted await functionName() as definition. It's now resolved:

The list of problems that I had found is now fully fixed. the next level would be for me to add tests

It would be cool if you could check the definitions too @DanTup

Bug fix. The expand_less : is recognized as a property key in this screen shot:

And is fixed in this one:

@pouyakary apologies, I haven't been keeping up with this.

It's hard to review currently because of the formatting changes. Can you re-format it with the original settings to reduce the diff just to the actual changes?

It may also be better to send the PR (or perhaps a few smaller PRs to make it easier to review) to https://github.com/dart-lang/dart-syntax-highlight since that's the source-of-truth for this. It would make sense to add some tests for all of the improvements you've made (we can commit the new tests against the original grammar first, and then it will be easier to see the improvements your changes make).

We'll also want to ensure we don't diverge any further from semantic tokens, I'd like to try and reduce the differences in colours between the grammar/semantic tokens (currently there are quite a lot, although I can see from your commits that you've fixed at least some of them 🙂). I don't currently have an automated way to compare these, although it's something I've been thinking about to try and help them converge.

@DanTup don't be, I delayed as well. The formatting is because it's just really hard for me to work with the json and this way make it readable for me. I'll fix the spacing. And yeah I think it should be like that. however I have no idea how to move all the history of commits and comments to there.

For the automated testing. I got blocked on the look behind regrex features of the testing. and have no idea how to fix the grammars with that. If that could be solved sure.

And for the semantic tokens. I have used the TypeScript/JavaScript grammar of Microsoft/vscode to see how they view a grammar and since I think all major themes on vscode first implement the TS/JS grammar, it will have some solid grounds. The tokens that go over the bounds of textmate spec are all based on the namings of the JS. I don't know if that's a right idea but I had no better one.

Needless to say that for each token that you have a better scope I'll appreciate. My only wish doing this was to differentiate between the tokens and have all of them colored.

The formatting is because it's just really hard for me to work with the json and this way make it readable for me. I'll fix the spacing. And yeah I think it should be like that. however I have no idea how to move all the history of commits and comments to there.

I don't think it'll be easy to move all of the commits as-is to another repo, although depending on the scale of the overall changes, a single commit of the new file might not be too difficult to review.

However, you should remove any formatting changes, including changes like this that change the order of fields:

Changes like this make the diff much larger and more difficult to review. Changes are likely to be reviewed and merged much sooner if they are easier to review, because they'll take less time for the reviewer.

For the automated testing. I got blocked on the look behind regrex features of the testing. and have no idea how to fix the grammars with that. If that could be solved sure.

I'm not very familiar with the look behind pattern you used. Could you provide an example (with an explanation of why it's needed) and I can see whether I can find an alternative that works with the Dart regex? Thanks!

Needless to say that for each token that you have a better scope I'll appreciate. My only wish doing this was to differentiate between the tokens and have all of them colored.

Yep, anything working towards this is a great change IMO. Our grammar/semantic tokens are currently quite different (many things are not coloured in the grammar) and it's something I've wanted to improve for a while :-)

@pouyakary what's the current status of this? I replied on dart-lang/dart-syntax-highlight#32 about the issue you had - I didn't really understand the special syntax being used but think we should stick to thinks that Dart can parse (since the grammar is used by more than just VS Code, for example Dart DevTools uses it and parses it using Dart).

@DanTup I'm actually using the grammar everyday and try to fix the bugs as I encounter them. (yesterday I found out dart can have generator getters and my grammar breaks at get something sync* =>, something I have to push a patch for soon) and then the testing remains which unfortunately I still haven't had the time to go through. And moving the commits to the main repository has become of a challenge for me.

Let me make the patch right now and move the commits to the main dart repo. And since now I've written the most of the grammar, I think I have to request access to the repo and maintain it myself. (which I have no idea who to ask for that)

Let me do the patch now and make a PR today to actually ship a version of this.

And moving the commits to the main repository has become of a challenge for me.

I'm happy to help with this if I can, but we'll need to create test cases for the the things being changed so they can be reviewed (and ensure they don't regress). I think a good first step would be to just extend the tests in https://github.com/dart-lang/dart-syntax-highlight to include code that covers all the things you're changing in the golden tests (right now it will produce the wrong results, but this will make it easier to review the differences when committing the grammar afterwards).

And since now I've written the most of the grammar, I think I have to request access to the repo and maintain it myself. (which I have no idea who to ask for that)

Do you mean the https://github.com/dart-lang/dart-syntax-highlight repo? That's maintained by the Dart team and the best way to get changes is by opening pull requests (which is what I do when I have improvements to make). This grammar is used by many tools so its important all changes are carefully reviewed and tests added to prevent regressions.

Let me know if there's anything I can do to help. Thanks!

@DanTup okay sure. Let me gradually add things there and use this issue as the changelog.

Going to close this in favour of landing changes in https://github.com/dart-lang/dart-syntax-highlight and then pulling them into here.

I added some comments on dart-lang/dart-syntax-highlight#40 and we can follow up there.