UserNobody14 / tree-sitter-dart

Attempt to make a tree-sitter grammar for dart
MIT License
59 stars 36 forks source link

yield, sync* and async* not being highlighted #18

Open LordMZTE opened 3 years ago

LordMZTE commented 3 years ago

It looks like tree-sitter-dart doesn't highlight the keywords yield, sync* and async* here's some code which showcases all 3 of these and compiles:

void main() async {
    print(count(10));
    print(await countStream(10).toList());
}

Iterable<int> count(int to) sync* {
    for (var i = 0; i <= to; i++) {
        yield i;
    }
}

Stream<int> countStream(int to) async* {
    for (var i = 0; i <= to; i++) {
        yield i;
    }
}

in neovim, using nvim-treesitter, it looks like this: image as you can see, the before mentioned keywords are white, and not pink like other keywords.

TimWhiting commented 3 years ago

Just uploaded a fix, but just to note, I think if you are using neovim, it uses it's own highlight file defined here: https://github.com/nvim-treesitter/nvim-treesitter/blob/master/queries/dart/highlights.scm

I've gone ahead and copied into here (since I believe their highlighting is better than the current highlighting from just this repositor), and added the fixes.

akinsho commented 3 years ago

I added the highlights.scm to nvim treesitter ages ago ~but~ so it definitely is in need of some updates since I haven't touched it in months and have definitely noted some missing syntax highlighting. @LordMZTE it's probably best to move this to an issue in nvim-treesitter or a PR 😄 rather than this repo. Thanks for the updates @TimWhiting 🙌🏾

theHamsta commented 3 years ago

If the highlights file is maintained here it might be worth to add a wget command to the automatic parser update workflow. So it would also update the query file

akinsho commented 3 years ago

@theHamsta when I initially tried using the highlights here directly that broke quite a lot in the neovim context. I made some edits to it specifically to improve the highlighting for nvim I don't think the one here is geared specifically for that use case, but I haven't tried it in a while. I'm planning on doing some work to update highlights for dart so could check if it's applicable now but off the top of my head there are a few things I can think of that are different and necessary IMO that I don't think are here.

TimWhiting commented 3 years ago

@theHamsta I think it would be best to maintain the highlights file along with the grammar. So please feel free to submit pull requests here to add missing highlighting files, and include it in an automatic parser update workflow. I'm willing to review and merge in changes quickly. I don't actively work on the highlights, but it doesn't make sense to have two diverging highlight implementations, and it would be beneficial to other users besides nvim. That way if I refactor the parser to include new syntax in dart then I can refactor the highlights at the same time.

@akinsho The highlights here were not maintained really until nvim started using it. Since I'm sure the old implementation was worse than nvim's highlights (and no users depended on it), I've based the current implementation off of the nvim changes. Please add whatever you feel is missing.

In fact if either of you are interested in maintaining the highlights, we should just get you commit access.

akinsho commented 3 years ago

@TimWhiting that's sounds excellent I'm definitely up for that, wasn't sure if you or @UserNobody14 might not be keen. Definitely makes sense to keep things together. As for contributing I work with dart full time using nvim so am pretty invested in making sure it's maintained.

@theHamsta I'm guessing this will preclude the use of any nvim directives though which I think the nvim highlights depends on at least on the vim regex directive and all other things like fold and indent will stay in nvim.

Not sure how to get round the regex thing but will get back to that when I'm nearer a computer and can have a proper look

TimWhiting commented 3 years ago

With an automated script we could probably append / replace any nvim specific directives?

@UserNobody14 can you add @akinsho as a collaborator for help working with the highlights?

theHamsta commented 3 years ago

I'm not sure what is the best solution. The nvim queries for nvim are a bit different from the Atom ones. Until now most maintainers decided to put the nvim queries in the nvim-treesitter repo. I guess the maintainers of their language should decide which fits best for their users.

TimWhiting commented 3 years ago

I think a lot of the queries are shared. I don't see a lot of nvim specific queries in the highlights.scm currently, I could be wrong, since I haven't touched highlights much. Maybe we can have a highlights.scm, and a nvim_highlights.scm, and concatenate them.

akinsho commented 3 years ago

Hmm 🤔 whilst I'd like to have things maintained here if possible I'm beginning to think it's the harder solution.

Looking over highlights.scm in nvim-treesitter we are dependent on vim-match and match not sure if match is general and only vim-match is specific either we need at least the match since it's crucial IMO. Whatever the solution I wouldn't want to restrict the queries being added that target nvim.

A separate general highlights.scm that gets merged with nvim_highlights.scm somehow sounds like an interesting approach. From a cursory check guess it could be something like

wget -O - https://github.com/UserNobody14/tree-sitter-dart/{highlights.scm,nvim_highlights.scm} > highlights.scm

Somewhere in nvim-treesitter..? From what I can see in nvim-treesitter @theHamsta none of the other languages do anything like this or maybe there is a script I'm missing?

Alternatively maybe the best approach would be something more manual like backporting changes from nvim's highlights here i.e. those that aren't too specific to nvim

theHamsta commented 3 years ago

vim-match? is currently the same as match?. At the moment none of the parsers do that. It would make sense for the zig parsers since it already only specialized on nvim-treesitter.

It could be done here https://github.com/theHamsta/nvim-treesitter/blob/f7422402ca675b753ab404ebb8d1a79300e988c3/.github/workflows/update-parsers-pr.yml#L36 before the workflow creates a PR and after the lockfile has been updated.. But maybe just having a separate config in nvim-treesitter would allow to specialize more for nvim. Combining the queries would work too assuming the order of the queries does not matter.

Thinking a bit about it I think having all the queries with nvim-treesitter is not too bad.

DartMitai commented 1 year ago

const and final List also not highlighted example