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

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

Open
LordMZTE opened this issue May 9, 2021 · 12 comments
Open

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

LordMZTE opened this issue May 9, 2021 · 12 comments
Assignees

Comments

@LordMZTE
Copy link

LordMZTE commented May 9, 2021

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
Copy link
Collaborator

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
Copy link
Collaborator

akinsho commented May 14, 2021

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
Copy link
Contributor

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
Copy link
Collaborator

akinsho commented May 14, 2021

@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
Copy link
Collaborator

TimWhiting commented May 14, 2021

@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
Copy link
Collaborator

akinsho commented May 14, 2021

@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
Copy link
Collaborator

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 TimWhiting reopened this May 14, 2021
@theHamsta
Copy link
Contributor

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
Copy link
Collaborator

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
Copy link
Collaborator

akinsho commented May 15, 2021

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
Copy link
Contributor

theHamsta commented May 15, 2021

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
Copy link

DartMitai commented Dec 13, 2022

const and final List also not highlighted
example

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

No branches or pull requests

6 participants