neovimUtils: also move queries #321550
Merged
neovimUtils: also move queries #321550
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MangoIV
force-pushed
the
mangoiv/fix-tree-sitter-to-plugin
branch
from
419e1dc to
f78d01f
Compare
|
finally, stuff works! |
itslychee
approved these changes
Jun 23, 2024
wegank
added
the
12.approvals: 1
|
It's odd you are the first one to hit this issue xD |
|
Result of 281 packages built:
|
Gerg-L
approved these changes
Jun 24, 2024
|
@teto yes, the koka grammar. by accident I have prepared this: and here's an example you can put into a file |
|
the koka grammar puts its queries directly in "queries" folder https://github.com/mtoohey31/tree-sitter-koka/tree/main/queries I think your approach is the correct one since that's how official treesitter grammars are packaged. Could you check if tjdevries is the odd one ? |
|
We can also just check if they’re already present and if they are, just don’t move something there, assuming that the author knew what they were doing. I.e. We don’t have to fail when they already exist. |
Agreed |
|
what would be nice is to have some debugging output that explains what's going on:
|
wegank
added
12.approvals: 2
MangoIV
force-pushed
the
mangoiv/fix-tree-sitter-to-plugin
branch
from
f78d01f to
d5122fd
Compare
MangoIV
force-pushed
the
mangoiv/fix-tree-sitter-to-plugin
branch
from
d5122fd to
6a386d7
Compare
|
I have done some adjustments such that we can also move if the grammar author has already prepared for vim and to tell the user if moving the grammar failed |
|
Result of 281 packages built:
|
Gerg-L
approved these changes
Jun 29, 2024
itslychee
approved these changes
Jun 30, 2024
|
ty, I quickly rewrote the commit message to track the essence of the change |
|
Thank you, yes that’s much better. |
|
This causes problem. The query from parsers are not always compatible with neovim. I thought nvim-treesitter doesn't install queries. |
|
@linsui which grammar are you having issues with? Nvimtreesitter has its own queries in the repo but it definitely installs queries. |
|
Maybe that’s the problem, should we also check whether we should move queries at all? |
|
The nix query from https://github.com/nix-community/tree-sitter-nix is not supported by neovim. |
|
I’ve not encountered any issue with the treesitter grammar of nix, do you install it in some special way? |
|
I was of the impression that this function does not interact with nvim-treesitter at all. This is why I am a bit confused. Do you argue the function behaves in the wrong way now or does something now go wrong that went right before? |
|
Because you don't install the query of nix from https://github.com/nix-community/tree-sitter-nix. You are using the one from nvim-treesitter. You can add the query to runtimepath before the query from nvim-treesitter so that it will be used. |
|
The I’d argue everything goes right, so you just want the parser from the repo you shared? |
|
Yep. The query doesn't always work with neovim. |
|
Then I think you should replace the parser only. I think in the general case you want parser and query because if there’s no query, the entire thing doesn’t work. |
|
For neovim, the query is vendored in nvim-treesitter. It currently works because the query from nvim-treesitter is put before the query from plugins. |
|
I agree that this shouldn't be done for |
|
Ah so this occurs in withAllGrammars and withPlugins. That needs fixing then. |
|
@MangoIV This pr causes a regression as mentioned in nvim-treesitter/nvim-treesitter#6870. In addition to causing problems with the nix grammar it messes with markdown/inline_markdown. Prior to nvim 10.0 it just breaks the markdown in vim help. Because of this issue markdown is displayed as source/plain text. After nvim 10.1 it throws errors like nix and hits the is-not predicate mentioned in that issue. |
|
@humemm I've very quickly skimmed through the other post so I might be wrong but this seems like this PR fixed the installation of the queries, which triggers an error but this is not the fault of the PR. Just dont install the nix grammar or use a working version (I dont know if there is one but I am curious to know) EDIT: you could also remove the query from the nix package or even just create an empty nix query in ~/.config/nvim to override the nix package's one |
|
@teto, as I mentioned above, this isn't just an issue with the nix grammar. Unfortunately, I'm not familiar with the internals of how treesitter grammars are handled in nix packages, so I can't offer a solution. I'm also unclear about what you mean by saying this PR "fixed the installation of the queries." It seems like this PR resolved issues with some grammars while introducing problems with others. Additionally, I'm not sure what issue this PR is addressing—the description is nearly empty, and no context was provided. I’d be happy to contribute and offer help, but... It’s clear this wasn't "fixed" if we're forced to use hacky solutions or avoid installing grammars altogether. Why should one avoid installing the nix grammar? Could you please explain? |
|
This PR is also causing the nushell queries to be places in |
BirdeeHub
added a commit
to BirdeeHub/nixpkgs
that referenced
this pull request
Aug 17, 2024
Addresses issue NixOS#332580 Revert to NixOS#321550 It was finding the queries before. Adding them to where you would expect throws an error. Removing them again afterwards during the build process fixes the error. It is weird, but that at least seems pretty clear to me. Opening this as a draft so that if the decision is made to revert, all that needs to be done is accept it.
PerchunPak
added a commit
to PerchunPak/nixpkgs
that referenced
this pull request
Sep 10, 2024
A NixOS#319233 accidentally reverted NixOS#321550. Last one caused a very annoying regression to any Nix user (see NixOS#332580). I suppose this is a bug in upstream grammar, so I workaround it this way until it is properly resolved.
13 tasks
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.