PR #321550 causes bugs with nix and markdown grammars

[humemm]

          @MangoIV This pr causes a regression as mentioned in https://github.com/nvim-treesitter/nvim-treesitter/issues/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.

Originally posted by @humemm in #321550 (comment)

[MangoIV]

Please excuse me @humemm I currently do not have time for this, even though I would like to help, afaiu this can be solved by preferring the queries from the nvim treesitter repo.

If you or someone else could add the fix that would be nice, else I’ll open a PR for reverting #321550

[humemm]

@MangoIV I don't mind fixing it however I have zero context and no idea what you mean when you say to prefer the queries from treesitter repo. If you provide a little more context I might be able to help.

[humemm]

@MangoIV can we just revert that PR it seems like there are others reporting issues with it.

[MangoIV]

I will try to do as soon as possible but in the meantime, feel free to open a revert MR.

[teto]

I think the PR does the right thing (except for the nu grammar where files are put in the wrong folder as a report mentioend it), probably @humemm is installing a wrong version of the grammar and/or mismatched with nvim-treesitter ?
We would need more details.
Nix just puts files where neovim expects them, see :h treesitter.

[BirdeeHub]

Surprisingly, it still works in nixCats?

I talked with a guy who was having an issue with pkgs.wrapNeovim

this was his config

I had to remove the substitutor stuff and also fix an input url to get his config to build on my machine, but when I did, it reliably reproduced the error every time I visited his flake.nix file. It throws on EVERY keyboard input in that file. Very annoying.

I took a fairly extensive look and couldnt figure it out, but decided to swap in his dependencies into a nixCats template just to see if it was something I needed to fix asap too.

It worked flawlessly again. no errors whatsoever when I moved his dependencies into the fields for them in the default nixCats template

We were confused for a while, but I think I figured out why treesitter grammars still work just fine in nixCats but not in pkgs.wrapNeovim, at least not when nvim-treesitter is lazy loaded (but with the grammars still in start, as resolved by neovimUtils.packDir) (and they can still find their queries just fine, they dont seem to need this to be able to find them)

https://github.com/BirdeeHub/nixCats-nvim/blob/df8e6a6bee79e68feea4a873bc5b6247382cd157/nix/builder/vim-pack-dir.nix#L86

Here, in the process of consolidating the treesitter grammars to make it easier to add them to rtp when doing the lazy.nvim wrapper, and to not spam my debug info commands, I think I accidentally undid this exact change, completely as a byproduct of consolidating them XD

I think the issue is it can still find them the other way, and now it finds them twice or something? Honestly I have no idea what the heck is up with it but I dodged a bullet because treesitter grammar changes are one of the only things in nixCats that arent under my direct control, along with which neovim-unwrapped is used, and the location of the ruby env directory in nixpkgs

I do know that when you load treesitter grammars after the builtin ones in rtp you get conflicts, but in this case, these are still first, and yet still error.

[BirdeeHub]

so yeah, in conclusion, they could already find the queries before this pr went through and now some things are having trouble finding them because of this change. I say revert it probably but that is probably up to you teto or mango or hummem or whoever.

In the meantime, if you avoid lazy loading nvim-treesitter and load at startup it seems to be more reliable? (EVEN THOUGH THAT SHOULDNT MATTER AND WHY IT DOES IS BLOWING MY MIND because like, grammars dont need nvim-treesitter to work and also they are still resolved via packDir function to be added with start plugins regardless of where you add nvim-treesitter.withAllGrammars)

So highly confusing why it is an issue, I have no clue, but it does seem to be the problem.

[MangoIV]

they could already find the queries before this pr went through and now some things are having trouble finding them because of this change.

What does this refer to? WithAllGrammars? Or? I’m 100% sure that it could not move queries for tree sitter grammars not coming from nvim-treesitter, that’s why I opened the PR. I do believe you that something goes wrong but I still don’t know what and nobody made it clear yet.

As I said, I don’t know what is the problem so if you had something working before and now it doesn’t, feel free to revert but then all the other tree sitter grammars won’t be working, again.

[jkuball]

Maybe I can add some more observations, but still, I don't know what the root cause is and how to fix it.

I am currently working on building my neovim config as a home manager module which creates a derivation of ~/.config/nvim that looks like a normal lazy configuration, while I am using a neovim from the neovim nightly overlay -- so I don't have any fancy use of nixCats or pkgs.wrapNeovim. Still I have the problem with the nix parser, which is the one with the "No handler for is-not?" error.

I am not doing anything fancy, I am just creating a LazySpec with a dir=/path/to/nix/store, and lazy adds it to the rtp. I have explicitely turned off lazy loading for treesitter, still, it is broken for me.
Looking at the generated derivations for the treesitter grammars, the directory structure looks fine for me, so it is at least not "wrongly installed" or something like that.

Nix parser with the problem:

$ tree /nix/store/pm8sxmbfz56xi9747k4aixv7h11g0x38-vimplugin-treesitter-grammar-nix
/nix/store/pm8sxmbfz56xi9747k4aixv7h11g0x38-vimplugin-treesitter-grammar-nix
├── parser
│   └── nix.so -> /nix/store/ggs3cbb75k2721lv79if6i7m06axx2n6-nix-grammar-0.0.0+rev=68d3b79/parser
└── queries
    └── nix
        ├── highlights.scm -> /nix/store/ggs3cbb75k2721lv79if6i7m06axx2n6-nix-grammar-0.0.0+rev=68d3b79/queries/highlights.scm
        ├── injections.scm -> /nix/store/ggs3cbb75k2721lv79if6i7m06axx2n6-nix-grammar-0.0.0+rev=68d3b79/queries/injections.scm
        └── locals.scm -> /nix/store/ggs3cbb75k2721lv79if6i7m06axx2n6-nix-grammar-0.0.0+rev=68d3b79/queries/locals.scm

4 directories, 4 files

Working yaml parser:

$ tree /nix/store/vabdjdk8qa6q20na9bd493nc9xijylmh-vimplugin-treesitter-grammar-yaml
/nix/store/vabdjdk8qa6q20na9bd493nc9xijylmh-vimplugin-treesitter-grammar-yaml
├── parser
│   └── yaml.so -> /nix/store/nxsrilnvqhmi0m8af3cm3immda7f7zlw-yaml-grammar-0.0.0+rev=7b03fee/parser
└── queries
    └── yaml
        └── highlights.scm -> /nix/store/nxsrilnvqhmi0m8af3cm3immda7f7zlw-yaml-grammar-0.0.0+rev=7b03fee/queries/highlights.scm

It is really unclear what went wrong here, but I also think it is better to find a solution to the problem than to just revert to the old behaviour for which I have heard that it is also broken on some parsers.

I happily add more context for my system and try out different versions of nvim and nixpkgs, but I have no pointers on how to fix it myself.

Currently I have locked nixpkgs/nixos-unstable on rev 2100883 and neovim-nightly-overlay with rev 67e84c020323a28f33ad4498f022a7b2c67719ad, following nixpkgs, if that helps in any way.

[BirdeeHub]

they could already find the queries before this pr went through and now some things are having trouble finding them because of this change.

What does this refer to? WithAllGrammars? Or? I’m 100% sure that it could not move queries for tree sitter grammars not coming from nvim-treesitter, that’s why I opened the PR. I do believe you that something goes wrong but I still don’t know what and nobody made it clear yet.

As I said, I don’t know what is the problem so if you had something working before and now it doesn’t, feel free to revert but then all the other tree sitter grammars won’t be working, again.

I mean, before this change went through, the grammars seemed to work just fine is what im saying. you say "all the other grammars won't be working, again" but they seemed to work just fine before this change? Despite them not being in the directory, they seemed to still be able to find the queries.

nixCats UNDOES this change by grabbing only the parser once again, and the queries work fine, despite not being where you would expect.

I think it is capable of following symlinks or something because it was definitely able to find the queries before despite them not being present directly

I have this

pack/myNeovimPackages/start/vimplugin-treesitter-grammar-ALL-INCLUDED/parser

Which contains a bunch of these

perl.so -> /nix/store/qjkc0waiiapjlx07c7f26jaky0fx0r20-perl-grammar-0.0.0+rev=3a21d9c/parser

jsonc.so -> /nix/store/3s7kf066dpky9rs59y8nl6dwd7iz3kib-jsonc-grammar-0.0.0+rev=02b0165/parser

objc.so -> /nix/store/dp3lyz4h2i69mmqpwyp16f53fjizhd4z-objc-grammar-0.0.0+rev=62e61b6/parser

No queries in sight, and yet it DEFINITELY can find the treesitter queries according to checkhealth, all my highlighting and related stuff to queries works in every filetype I have used, it seems to work

If you follow those symlinks, there are queries in there, my theory is that it is following the symlinks

Its weird that it was able to find them before, because they were not copied over, but it could in fact find them

[humemm]

I think the PR does the right thing (except for the nu grammar where files are put in the wrong folder as a report mentioend it), probably @humemm is installing a wrong version of the grammar and/or mismatched with nvim-treesitter ? We would need more details. Nix just puts files where neovim expects them, see :h treesitter.

@teto, your assumption seems broad. Have you attempted to reproduce the bug? Are there automated tests in nixpkgs that cover edge cases for grammar/query locations? It appears multiple grammars/queries were affected as evident by the discussion here and on the linked issue. I'm new here and not fully familiar with how nixpkgs handles package releases and testing. If there’s documentation or a spec, I’d appreciate a pointer. Simply referring to :h tree-sitter isn’t very helpful.

Here are the PR changes. Bespoke bash script that moves queries from one directory to another:

Link to script

# from PR change
echo "moving queries from neovim queries dir"
...
# again 
echo "moving queries from standard queries dir"

The PR lacks unit tests that might clarify expectations. Clearly from the comments being echoed what was done is in no way standard. Any guidance on the issue being addressed here would be appreciated.

For context my grammars were installed using pkgs.vimPlugins.nvim-treesitter.withAllGrammars and pkgs.vimPlugins.nvim-treesitter.withPlugins the errors persist in both cases.

EDIT: removing the queries in postBuild = "rm -r $out/queries" solves the issue which leads me to believe that the cp job is creating duplicates as mentioned in the conversation above or the linked issue.

[teto]

what I meant by Nix just puts files where neovim expects them, see :h treesitter. is that nix knowledge is not needed to diagnose the issue, just to fix the issue so dont let it stop you from investigating. Once we know the issue, we can help with the nixpkgs part.

Have you attempted to reproduce the bug?

I did not because I know from experience that neovim + treesitter issues are 90% misconfiguration. It's not robust at all, one little misconfiguration and it blows up, which is why it's still experimental (I dont blame users, it's the ecosystem that need to make it easier).
For instance your initial message mentions with the nix grammar it messes with markdown/inline_markdown but you dont need to install the inline_markdown grammar, it's builtins with neovim

nixpkgs/pkgs/by-name/ne/neovim-unwrapped/package.nix

Line 92 in 2987ca5

{ markdown_inline = treesitter-parsers.markdown // { language = "markdown_inline"; location = "tree-sitter-markdown-inline"; }; }

!

So I think you override the builtins queries with more recent ones, the mismatch generates the error. Before #321550, one had mismatched libraries and queries but it was less likely to blow up (how neovim leverage queries change more frequently).
In the end I did try a short test but I could not break anything (I am on nightly neovim, which should be why).
My guess is that you are using stable neovim (0.10) ?
and the problem is that nixpkgs follows the development version of nvim-treesitter so it works only with neovim nightly.

It's a quick guess so I might be wrong (you could try nightly neovim see if it fixes your error https://github.com/nix-community/neovim-nightly-overlay).

In case I guessed correclty, nixpkgs should package the last version of nvim-treesitter compatible with neovim 0.10 (and it's indeed a flaw in nixpkgs, but not the incriminated PR).

[BirdeeHub]

I have tried to reproduce and I can confirm that it does cause issues with the new copying.

tried it on someones config, one outside of nixCats, one inside of nixCats. Same set of plugins(I copy pasted them into the default nixCats template), the other one using pkgs.wrapNeovim which I mentioned here previously in this thread

The only difference in loading between the two methods that would have ANY effect is that nixCats posthumorously removes the queries directories (on accident, because when I wrote it, thats all there was, and it worked fine, and somehow also avoids this issue XD).

the one using nixCats works fine, the other one errored on every keypress on its flake.nix

I believe that if we need to copy them in, we also need to stop nvim-treesitter from vendoring them in for us. Or go back to not copying them in.

Either way is fine with me, but the second one is easier and could be done as a temporary measure as well.

Honestly, idk. I know is a bug, I do not know if it is a better plan to keep the copy and stop nvim-treesitter from vendoring in the queries however it is doing that, or if it is a better plan to revert, but one of the two should be done.

I still am not sure how it is resolving the queries. If I knew that, I would fix it there probably

[teto]

@BirdeeHub can we avoid mentioning nixCats and focus on the problem here ?

I believe that if we need to copy them in, we also need to stop nvim-treesitter from vendoring them in for us. Or go back to not copying them in.

I dont think it's questionable here: if nvim-treesitter provides them so we must provide them, we are not half-installing software.
If the problem is actually the one I mentioned, our fault lies in mixing a dev version of nvim-treesitter with a stable version of neovim.

[BirdeeHub]

right, so, we need to stop nvim-treesitter from vendoring them in so that we can copy them without creating duplicates, so that we do it properly

[BirdeeHub]

Im all on board for that plan I just dont know how treesitter is doing it

[BirdeeHub]

and im not mentioning nixCats just to mention it.

I was saying what I tried, because the result of its output is functionally identical to how nixpkgs was before that patch, and that is how I found out.

I suppose I should have said, before the patch and after the patch, but thats not what I tested at first or how I found out.

Heres an even more minimal example below, 1 file, 52 lines

[BirdeeHub]

@teto
@MangoIV
@humemm

Minimal reproduction.

nix run this flake from this comment, use it to look at this flake. The error occurs on enter, and every edit.

Hopefully this helps clear up whats being discussed.

To be even more clear, this should, and does, put the grammars in start and on the rtp, and the nvim-treesitter plugin in opt.

nvim-treesitter isnt supposed to be required to load grammars. So this should be 100% fine.

But it isnt. Whats more, it worked right before the patch, as can be shown by using the hash below.

Basically, if we are going to copy them, we cannot just copy them, we must go deeper as well and figure out exactly how they were being resolved before. But idk where to look, im still semi new around here. I did try, I looked through all the nixpkgs section for it I thought but its likely I missed it due to unfamiliarity.

I know from my experience messing with writing a lazy.nvim wrapper, that if your grammars are not before vim.fn.fnamemodify(vim.v.progpath, ":p:h:h") .. "/lib/nvim" in the rtp you can experience collisions with the builtins, but this is a different problem (possibly related, but still different, because in this case they ARE before it), and a different error, and it definitely happens right after the patch and not right before the patch.

{
  inputs = {
    nixpkgs = {
      # commit from immediately before PR in question, if chosen instead, no bug
      # url = "github:nixos/nixpkgs/19581e2ce8bc43f898ef724f8072ebf62bebb325";
      url = "github:nixos/nixpkgs/nixpkgs-unstable";
    };
    neovim = {
      # It happens regardless of nightly, you can try too if you want.
      url = "github:nix-community/neovim-nightly-overlay";
      inputs.nixpkgs.follows = "nixpkgs";
    };
  };
  outputs = { nixpkgs, neovim, ...}: let
    forAllSys = nixpkgs.lib.genAttrs nixpkgs.lib.platforms.all;
    overlayMyNeovim = prev: final: {
      myNeovim = let
        luaRC = final.writeText "init.lua" /*lua*/''
          vim.schedule(function()
            vim.cmd("packadd nvim-treesitter");
            require("nvim-treesitter.configs").setup({
              auto_install = false,
              highlight = {
                enable = true
              },
            })
          end)
        '';
      in
      final.wrapNeovim final.neovim {
        configure = {
          customRC = ''lua dofile("${luaRC}")'';
          packages.all.start = with final.vimPlugins; [ 
          ];
          packages.all.opt = with final.vimPlugins; [
            nvim-treesitter.withAllGrammars
          ];
        };
        extraMakeWrapperArgs = ''--prefix PATH : "${final.lib.makeBinPath (with final; [ stdenv.cc.cc ])}"'';
      };
    };
  in 
  {
    packages = forAllSys (system: let
      pkgs = import nixpkgs {
        inherit system;
        overlays = [ neovim.overlays.default overlayMyNeovim ];
      };
      in
      { default = pkgs.myNeovim; });
  };
}

[BirdeeHub]

Im happy to do it but idk where to look, ive looked in the vim folder's stuff as well as here but i didnt see where it happens, anyone have any leads for me?

[BirdeeHub]

nixpkgs/pkgs/applications/editors/vim/plugins/nvim-treesitter/overrides.nix

Lines 33 to 55 in 5cbbd23

	# Usage:
	# pkgs.vimPlugins.nvim-treesitter.withPlugins (p: [ p.c p.java ... ])
	# or for all grammars:
	# pkgs.vimPlugins.nvim-treesitter.withAllGrammars
	withPlugins =
	f: self.nvim-treesitter.overrideAttrs {
	passthru.dependencies =
	let
	grammars = map grammarToPlugin
	(f (tree-sitter.builtGrammars // builtGrammars));
	copyGrammar = grammar:
	let name = lib.last (lib.splitString "-" grammar.name); in
	"ln -s ${grammar}/parser/${name}.so $out/parser/${name}.so";
	in
	[
	(runCommand "vimplugin-treesitter-grammars"
	{ meta.platforms = lib.platforms.all; }
	''
	mkdir -p $out/parser
	${lib.concatMapStringsSep "\n" copyGrammar grammars}
	'')
	];
	};

Also, in withPlugins and withAllGrammars, it only links the parser anyway

[BirdeeHub]

So, I think the current set of queries that WAS being used are literally the queries in the nvim-treesitter repo itself?

They werent being included with the grammars and they were using the ones from nvim-treesitter likely to avoid such issues

Reverting would mean no queries for parsers without queries in nvim-treesitter, but possible issues with different queries for parsers with both.

Is there any easy way to solve this?

[PerchunPak]

Using queries from grammars instead of treesitter repo doesn't work, though it is less annoying (I got the issue only after 10-30 seconds, instead of every keypress)

Here is the patch for fix I though about:

diff --git a/pkgs/applications/editors/vim/plugins/nvim-treesitter/overrides.nix b/pkgs/applications/editors/vim/plugins/nvim-treesitter/overrides.nix
index c7cd3266d9c7..ae0212ae44f4 100644
--- a/pkgs/applications/editors/vim/plugins/nvim-treesitter/overrides.nix
+++ b/pkgs/applications/editors/vim/plugins/nvim-treesitter/overrides.nix
@@ -1,4 +1,4 @@
-{ lib, callPackage, tree-sitter, neovim, neovimUtils, runCommand }:
+{ lib, callPackage, tree-sitter, neovim, neovimUtils, runCommand, symlinkJoin }:
 
 self: super:
 
@@ -40,17 +40,12 @@ let
         let
           grammars = map grammarToPlugin
             (f (tree-sitter.builtGrammars // builtGrammars));
-          copyGrammar = grammar:
-            let name = lib.last (lib.splitString "-" grammar.name); in
-            "ln -sf ${grammar}/parser/${name}.so $out/parser/${name}.so";
         in
         [
-          (runCommand "vimplugin-treesitter-grammars"
-            { meta.platforms = lib.platforms.all; }
-            ''
-              mkdir -p $out/parser
-              ${lib.concatMapStringsSep "\n" copyGrammar grammars}
-            '')
+          (symlinkJoin {
+            name = "vimplugin-treesitter-grammars";
+            paths = grammars;
+          })
         ];
     };
 
@@ -59,7 +54,7 @@ in
 
 {
   postPatch = ''
-    rm -r parser
+    rm -r parser queries
   '';
 
   passthru = (super.nvim-treesitter.passthru or { }) // {

See also https://ryantm.github.io/nixpkgs/builders/trivial-builders/#trivial-builder-symlinkJoin

BTW, #321550 was accidentally reverted by #319233

[BirdeeHub]

This looks like a good fix to me at first glance. Does it pass the minimal reproduction case above? Havent tested it out myself yet. Building the minimal case rn.

[BirdeeHub]

So, it actually does a much better job of including the queries and parsers correctly compared to what was being done before. This is a much more proper way of doing what the PR this thread is about was trying to do. MUCH better than the sketchy bash script.

However I pulled master, made these changes and tested it on the minimal reproduction case, and it still has the same issue, just as bad as before. On enter, and then on every edit to the file, it throws the same error.

[BirdeeHub]

I dont know what is the issue though, because, this change also deletes the queries directory from nvim-treesitter in post patch. so conflicts SHOULD be impossible. So what is even the issue in the first place? Why is this error being thrown lmao

[PerchunPak]

it still has the same issue, just as bad as before

Yeah, I also tried the MRC you wrote above, and it didn't work. This is why I left this solution as a comment, and not as a PR. Though for me, it wasn't as bad as you describe, but it doesn't matter. No idea why the problem persists

MUCH better than the sketchy bash script

Thanks to figsoda (I miss him). I found out about linkFarm in some old thread during debugging for #339076

---

P.S. I don't know how I spent so much time debugging treesitter grammars inside nix without ever reading :help treesitter...

Parsers are searched for as parser/{lang}.* in any 'runtimepath' directory.
If multiple parsers for the same language are found, the first one is used.
(NOTE: This typically implies the priority "user config > plugins > bundled".)

[PerchunPak]

Yeah, reading the friendly manual answered a lot of my questions... I believe this is a bug in upstream, see nix-community/tree-sitter-nix#84

BTW, linkFarm might create weird merge results if multiple duplicate grammars with different files are present (e.g. grammar A has a file a.scm and grammar A-but-older has a file a.scm and b.scm. If A comes before A-but-older, result derivation will have a.scm -> A and b.scm -> A-but-older which might create weird errors). We still would need to remove duplicates before feeding it to linkFarm (or I am being mistaken about how linkFarm works; I will need to experiment with it before filing a PR)

[BirdeeHub]

Ya know... I did read the :help treesitter manual... in january when I was building the lazy.nvim wrapper in nixCats

I somehow COMPLETELY forgot about its existence.

TYSM for your work and investigation, it has made a lot of how treesitter grammars work in nix much less mysterious to me.

[juanibiapina]

Also happens with ruby: https://github.com/tree-sitter/tree-sitter-ruby/blob/a66579f70d6f50ffd81a16fc3d3358e2ac173c88/queries/highlights.scm#L4

[PerchunPak]

Also happens with ruby: https://github.com/tree-sitter/tree-sitter-ruby/blob/a66579f70d6f50ffd81a16fc3d3358e2ac173c88/queries/highlights.scm#L4

Looks like I found the issue

https://github.com/nvim-treesitter/nvim-treesitter/blob/929ca9c76ee20bb27cffbde4ee90583b6c54d616/queries/jsonnet/locals.scm#L27

Also, here is what is-not does: https://github.com/tree-sitter/tree-sitter/blob/4f0d463d49a3f1f2a6eaa75ecc284a5719aad94e/docs/section-4-syntax-highlighting.md?plain=1#L242

Solution for ruby would be to use grammars from nvim-treesitter org instead of tree-sitter/tree-sitter-ruby. Probably for Nix it would be the same.

Don't know how this could be done, and would love to hear what @figsoda has to say about this. Also, cannot work on a PR rn, so feel free to create one.

[juanibiapina]

The nvim-treesitter maintainer has confirmed that external queries are incompatible with nvim-treesitter in this comment: nvim-treesitter/nvim-treesitter#6870 (comment)

and also clarified that nvim-treesitter doesn't use locals for highlights: nvim-treesitter/nvim-treesitter#6870 (comment) which could explain why is-not? is not implemented there. It appears that is-not? only appears in highlights, even in the official treesitter documentation linked above.

Does that mean vimPlugins.nvim-treesitter.withAllGrammars makes an incorrect assumption, or am I misunderstanding what it does? I'm using it for downloading the compiled grammars since I can't reliably compile them on the fly on nix: https://github.com/juanibiapina/dotfiles/blob/master/nix/packages/nvim.nix#L7

I'm also confused about the relationship between nvim-treesitter and the native treesitter implementation in nvim 😕

[BirdeeHub]

#344849

This wonderful person once again is saving us with treesitter grammars