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

fix: make sure queries are copied to the right place #286

Closed
wants to merge 2 commits into from

Conversation

ckipp01
Copy link
Collaborator

@ckipp01 ckipp01 commented Jun 7, 2023

I noticed that the queries in nvim-treesitter and this repo have
diverged, but we never got an error here. I think this was a regression
introduced in
7d348f5.

Copy link
Collaborator

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ckipp01
Copy link
Collaborator Author

ckipp01 commented Jun 7, 2023

Don't merge this yet, this should be failing, so something still isn't right.

EDIT:

Alright so this does catch them now. I'll sync some stuff that nvim doesn't have yet first in nvim-treesitter/nvim-treesitter#4920, and then I'll sync back the other way because we are missing a change from there as well. Then I'll update this and it should be 🟢 .

I noticed that the queries in nvim-treesitter and this repo have
diverged, but we never got an error here. I think this was a regression
introduced in
tree-sitter@7d348f5.
@ckipp01
Copy link
Collaborator Author

ckipp01 commented Jun 11, 2023

So I sort of forgot about this, but it's failing right now and it's somewhat expected. Basically some of the improvements that were added by @ghostbuster91 in #196 are great and they follow the upstream locals way of doing things. However, this is one area where nvim-treesitter differs. So if we sync the local queries here, it won't be quite as accurate. I'm sort of inclined to update the check to only check against the actual hightlights.scm file since the locals.scm will be different. Anyone have thoughts on this?

@susliko
Copy link
Collaborator

susliko commented Jun 11, 2023

I'd suggest being explicit about the fact that queries are system-specific (as far as I remember, there are also nvim-specific captures for highlights, not only locals). For example, we could have a structure like

queries
  (highlights|locals).scm - vanilla tree-sitter
integrations
  nvim
    queries
      (highlights|locals|etc.).scm

And check only queries/* in the Ci

In this scheme queries from integrations/nvim/ should probably be a source of truth for nvim queries, and be synced to nvim-treesitter upon changes here

@ghostbuster91
Copy link
Contributor

I'd suggest being explicit about the fact that queries are system-specific

👍

I would not revert the changes in the locals.scm as there might be other editors that use treestitter (e.g. kakune, helix), that could benefit from these definitions.

@ckipp01
Copy link
Collaborator Author

ckipp01 commented Jul 5, 2023

Just commenting that I haven't forgotten about this, 😬 just been busy lately with other things. I'll try to return soon.

@ckipp01 ckipp01 closed this Mar 7, 2024
@ckipp01
Copy link
Collaborator Author

ckipp01 commented Mar 7, 2024

Closing this just because I won't have time to work on it.

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.

4 participants