nix-community / nixvim

Configure Neovim with Nix! [maintainer=@GaetanLepage, @traxys, @mattsturgeon]
https://nix-community.github.io/nixvim
MIT License
1.44k stars 220 forks source link

Temporary fix for jsregexp and therefore luasnip not working #1669

Open hyperpastel opened 1 month ago

hyperpastel commented 1 month ago

Hey all,

I have been wanting to get luasnip's transformation and variable snippets to work in nixvim for some time now, but unfortunately, due to what I understand to be packaging issues, this is currently not possible out of the box.

However, after dissecting how luasnip loads jsregexp, I came up with this little one-liner that can be prepended to a neovim config using nixvim's extraConfigLuaPre

extraConfigLuaPre = ''
            package.preload["jsregexp"] = package.loadlib("${pkgs.lua51Packages.jsregexp}/lib/lua/5.1/jsregexp/core.so", "luaopen_jsregexp_core"); 
'';

This method is similar to what luasnip does. luasnip will override the package loader for jsregexp.core to load its own, vendored version. However, if that does not work, it will fall back to loading the global jsregexp module. And for this load of the global module, we too provide an overriden loader, which will make sure the module gets loaded correctly.

Theoretically, this could also be altered to

extraConfigLuaPre = ''
    package.preload["jsregexp"] = package.loadlib("${pkgs.lua51Packages.jsregexp}/lib/lua/5.1/jsregexp/core.so", "luaopen_jsregexp_core")
    require("jsregexp")
    package.preload["jsregexp"] = nil
'';

to clean up after loading and caching jsregexp. This would mirror luasnip even more closely. I don't believe this is crucial, but perhaps it would still be good practice, so we do not keep this behavior which others might not expect.

I've tested this locally on my machine and it works great with the example config that cmp suggests for using with luasnip! (If needed, I will gladly share my config 😊)

Perhaps this is not the most elegant solution and there would be better ways to accomplish this, but I'm not quite caught up on the developments on the issues that prevented this from working in the first place (I believe this is teto's domain?).

I just thought I'd share this, either for it to be added until the resolution of that or as a reference for others that face this problem in case it won't.

Thank you for making nixvim!

MattSturgeon commented 1 month ago

If we added a nullable plugins.luasnip.jsregexpPackage option we could have something like this in luasnip's extraConfig.

package.preload["jsregexp"] = package.loadlib("${pkgs.lua51Packages.jsregexp}/lib/lua/5.1/jsregexp/core.so", "luaopen_jsregexp_core"); 

Although the /lib/lua/5.1/ looks very version-specific and would add extra maintenance cost for us... Is there a way to get that automatically, either from an env var or from the package metadata? :thinking:

hyperpastel commented 1 month ago

Hey Matt!

I took a look at how the jsregexp package is built on nix. It turns out it is built using nixpkgs' buildLuarocksPackage function, which internally builds the package's derivation using luarocks make.

The crucial line is this one - /lib/lua/5.1/ is part of the tree structure that luarocks generates when making the package, it is outlined here.

So, it would seem we are safe here, as long as we stick to the lua51Packages version, which, as far as I understand neovim, we should.

MattSturgeon commented 1 month ago

I took a look at how the jsregexp package is built on nix. It turns out it is built using nixpkgs' buildLuarocksPackage function, which internally builds the package's derivation using luarocks make.

The crucial line is this one - /lib/lua/5.1/ is part of the tree structure that luarocks generates when making the package, it is outlined here.

Thanks! That's helpful

as long as we stick to the lua51Packages version, which, as far as I understand neovim, we should.

Well, by exposing a plugins.luasnip.jsregexpPackage option, we'd be allowing users to set other (potentially non-5.1) versions.

Though maybe that's a non issue atm if 5.1 is required by neovim, but that could change too in the future...

Perhaps we should have a plugins.luasnip.preloaders option; e.g.

plugins.luasnip.preloaders = {
  jsregexp = ''
    package.loadlib("${pkgs.lua51Packages.jsregexp}/lib/lua/5.1/jsregexp/core.so", "luaopen_jsregexp_core")
  '';
};

But I can't think of a good way to add the jsregexp lib by default if we do it that way.

Any thoughts @nix-community/nixvim?

traxys commented 1 month ago

I think that having a fix is nice, but I'm not sure what is the best way to expose it. I feel it should be enabled by default, but that you could opt out. Maybe something like plugins.luasnip.loadLibFix ?

hyperpastel commented 1 month ago

I think that having a fix is nice, but I'm not sure what is the best way to expose it. I feel it should be enabled by default, but that you could opt out. Maybe something like plugins.luasnip.loadLibFix ?

I believe this would be the best way to assess this. It is very likely that everyone using luasnip would also want to use these transform, and such an option that is enabled by default would make that work perfectly.

And anyone that wants to load the lib differently, or not at all, can just disable this and tinker with the extraLuaConfig themselves. I believe however that such cases would be so niche that no further accommodation for them would be required on nixvim's end 😉