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

Switch to latest nvim-treesitter queries #36

Merged
merged 6 commits into from
Feb 5, 2024
Merged

Conversation

pynappo
Copy link
Contributor

@pynappo pynappo commented Jan 21, 2024

nvim-treesitter recently made a bunch of breaking changes to their queries to stay up-to-date with treesitter/helix
(nvim-treesitter/nvim-treesitter#5895), this PR should keep this colorscheme up-to-date.

For this PR, I mostly followed https://github.com/nvim-treesitter/nvim-treesitter/blob/master/CONTRIBUTING.md#highlights and took a peek at https://github.com/Iron-E/nvim-highlite/blob/master-v4/colors/highlite-ayu.lua

Let me know if you want me to just deprecate the old highlights instead of outright removing them.

@Shatur
Copy link
Owner

Shatur commented Jan 21, 2024

Some things doesn't look right. Like, for example, the following should have orange color instead of blue:
image
Colors should match the old ones.

Mistakenly deleted it, this is a valid query.
@pynappo
Copy link
Contributor Author

pynappo commented Jan 21, 2024

Ah, mistakenly removed a highlight, apologies.

Colors should match the old ones.

Understood, although there is 1 remaining color change left in this PR that is more intentional: @lsp.type.struct:

  1. LSP-highlighted decorators are linked to @attribute instead of @function. This ends up being the same as treesitter-python's highlighting:

Screenshot_2024-01-21-05-35-10-92_84d3000e3f4017145260f7618db1d683

(although I think this changes rust-analyzer's decorations? LMK how you feel about that.)

I was thinking these changes aren't controversial, but I could revert/put in different PR?

Also feel free to edit this PR as you please as I won't be available until likely late tomorrow.

Edit: previously, I changed the @lsp.type.struct to link to @variable.member instead of @structure because @structure no longer exists and the nvim-treesitter contributing.md guide recommends @variable.member for structs, but I've realized Structure is better for this.

Seems more reasonable
@Shatur
Copy link
Owner

Shatur commented Jan 21, 2024

I would prefer to keep the highlighting as is and then discuss the suggested changes in a separate PR.
Here is how it look with the current master:
image
Here is how it looks with the current PR:
image

@pynappo
Copy link
Contributor Author

pynappo commented Jan 21, 2024

Oh I didn't know lsp.type.struct affected that (rust-analyzer refuses to install on android so I couldn't check for breaking changes on Rust), reverted both changes then. Thanks for the patience. PR is just renames now.

Was a new query.
@Shatur
Copy link
Owner

Shatur commented Jan 21, 2024

Namespaces are still different, they shouldn't be blue:
image

@pynappo
Copy link
Contributor Author

pynappo commented Jan 21, 2024

Hm, what's the output of :Inspect on the namespaces? I reckon maybe your treesitter parsers are using the older naming for the queries.

For reference this is what I get with latest rust treesitter and my PR:

image

I can add back the old highlight groups as legacy if that would be preferable.

@Shatur
Copy link
Owner

Shatur commented Jan 22, 2024

Ah, I'm sorry, I forgot to update my TS :)

Looks like sometimes it works even better now.
Before, no semantic highlighting:
image
After, no semantic highlighting either:
image

I’ll test this PR for a day, if everything is ok, I’ll merge it and we can discuss the proposed changes.

@Shatur
Copy link
Owner

Shatur commented Feb 5, 2024

Ah, I forgot to merge it, sorry. 2 weeks of testing, no issues, thank you!

@Shatur Shatur merged commit 0a9804d into Shatur:master Feb 5, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants