tree-sitter-dart icon indicating copy to clipboard operation
tree-sitter-dart copied to clipboard

yield, sync* and async* not being highlighted

Open LordMZTE opened this issue 3 years ago • 12 comments

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.

LordMZTE avatar May 09 '21 19:05 LordMZTE

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.

TimWhiting avatar May 14 '21 13:05 TimWhiting

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 🙌🏾

akinsho avatar May 14 '21 14:05 akinsho

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

theHamsta avatar May 14 '21 17:05 theHamsta

@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.

akinsho avatar May 14 '21 17:05 akinsho

@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.

TimWhiting avatar May 14 '21 17:05 TimWhiting

@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

akinsho avatar May 14 '21 18:05 akinsho

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?

TimWhiting avatar May 14 '21 18:05 TimWhiting

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.

theHamsta avatar May 14 '21 22:05 theHamsta

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.

TimWhiting avatar May 14 '21 23:05 TimWhiting

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

akinsho avatar May 15 '21 22:05 akinsho

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.

theHamsta avatar May 15 '21 23:05 theHamsta

const and final List also not highlighted example

DartMitai avatar Dec 13 '22 09:12 DartMitai