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

Update which-key keybind spec #1901

Closed
Sw3d15h-F1s4 opened this issue Jul 20, 2024 · 8 comments · Fixed by #1944
Closed

Update which-key keybind spec #1901

Sw3d15h-F1s4 opened this issue Jul 20, 2024 · 8 comments · Fixed by #1944

Comments

@Sw3d15h-F1s4
Copy link

Which-key is moving to a new specification for defining keybindings for version 3. This new spec appears to be similar to lazy.nvim's keybind specification. While this is just a warning right now, this could be a breaking change in the future.

Currently, nixvim documentation suggests to use the following format for definining programs.nixvim.plugins.which-key.registrations:

{ "<leader>p" = "Find git files with telescope";}

which generates the following Lua:

{ ["<leader>p"] = "Find git files with telescope" }

This is fine for the old which-key spec, but they are moving to a new spec. That keybinding would now be in Lua:

{
  {"<leader>p", desc = "Find git files with telescope"}
}

I'm relatively new to Nix, so figuring out how to get the "<leader>p" by itself in nix was a little tricky. Ended up reading the source code for toLuaObject and it looks like __unkeyed will do the trick. A simple band-aid fix in Nix could be:

programs.nixvim.plugins.which-key.registrations = [
  { __unkeyed = "<leader>p"; desc = "Find git files with telescope"; }
];

but this doesn't work, as which-key.registrations only takes a type "attribute set of anything," not a list.

Another workaround could be to do the following for multiple keymaps:

programs.nixvim.plugins.which-key.registrations = {
  __unkeyed1 = { __unkeyed = "<leader>p"; desc = "Find git files with telescope"; };
  __unkeyed2 = { __unkeyed = "<leader>e"; desc = "some other keybind"; };
 __unkeyed3 ... ... ...
};

but this is really tedious, and it doesn't seem to work well either. It evaluates, the generated Lua looks right to my eyes, but which-key doesn't like it for some reason. Maybe I'm doing something wrong.

(sidenote: in to-lua.nix the comment says:

        If this option is true, attrsets like { __unkeyed.1 = "a"; b = "b"; }
        will render as { "a", b = "b" }

but this is not correct. Here's what I see in nix-repl.

nix-repl> inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed.1 = "a"; b = "b";}
error: syntax error, unexpected FLOAT, expecting '.' or '='

       at «string»:1:62:

            1| inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed.1
             |                                                              ^

nix-repl> inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed1 = "a"; b = "b";}
"{ \"a\", b = \"b\" }"

nix-repl> inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed1 = "a"; __unkeyed2 = "b";}
"{ \"a\", \"b\" }"

nix-repl> inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed.a = "a"; b = "b";}
"{ { a = \"a\" }, b = \"b\" }"

)

anyway, sorry for the long-winded report. I wanted to at least try to solve the problem before asking about it. Thanks for all the hard work you all do, as a noob to Linux and Nix things have been (mostly) smooth sailing so far.

@MattSturgeon
Copy link
Member

Thanks for making an issue and for being so detailed - this is very useful!

I've been meaning to look into this for a few days now.

We could use our transitionType to convert old values to the new type, but I think the best bet is to incorporate this as part of a general update/refactor of the module to use our new-style freeform settings option.

@michaelhthomas
Copy link
Contributor

Looks like v3 comes with several other breaking changes to configuration options, so to me, it makes sense to just fully overhaul the module as well (in the spirit of which-key v3 being a complete rewrite)

@MattSturgeon
Copy link
Member

(sidenote: in to-lua.nix the comment says:

        If this option is true, attrsets like { __unkeyed.1 = "a"; b = "b"; }
        will render as { "a", b = "b" }

but this is not correct. Here's what I see in nix-repl.

nix-repl> inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed.1 = "a"; b = "b";}
error: syntax error, unexpected FLOAT, expecting '.' or '='

       at «string»:1:62:

            1| inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed.1
             |                                                              ^

nix-repl> inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed1 = "a"; b = "b";}
"{ \"a\", b = \"b\" }"

nix-repl> inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed1 = "a"; __unkeyed2 = "b";}
"{ \"a\", \"b\" }"

nix-repl> inputs.nixvim.lib.x86_64-linux.helpers.toLuaObject {__unkeyed.a = "a"; b = "b";}
"{ { a = \"a\" }, b = \"b\" }"

I skimmed over this the first time I read. The comment should use quoted attr names, but other than that is is "correct". E.g. this will work: { "__unkeyed.1" = "foo"; }.

I can't recall if we normally use . or - separators elsewhere, but the toLua implementation just checks the key starts with __unkeyed.

@MattSturgeon
Copy link
Member

MattSturgeon commented Jul 24, 2024

Looking at the upstream docs: https://github.com/folke/which-key.nvim#%EF%B8%8F-mappings

The wording isn't concrete, but I get the impression the add() method is now preferred over the spec config option?

Either way I think the config option would still be a better fit for nixvim's module.

EDIT:

I also noticed other related issues still open. Some of these may be resolved already:

@pkulak
Copy link

pkulak commented Jul 25, 2024

Not terribly related... but does anyone know how to keep which-key from spewing warnings every single startup?

@clemenscodes
Copy link

Not terribly related... but does anyone know how to keep which-key from spewing warnings every single startup?

I don't know of any native setting to do that, but you could downgrade which-key to a version prior to the breaking change:

{pkgs, ...}: {
  programs = {
    nixvim = {
      plugins = {
        which-key = {
          enable = true;
          package = pkgs.vimPlugins.which-key-nvim.overrideAttrs (oldAttrs: {
            src = pkgs.fetchFromGitHub {
              owner = oldAttrs.src.owner;
              repo = oldAttrs.src.repo;
              rev = "0539da005b98b02cf730c1d9da82b8e8edb1c2d2"; # v2.1.0
              hash = "sha256-gc/WJJ1s4s+hh8Mx8MTDg8pGGNOXxgKqBMwudJtpO4Y=";
            };
          });
        };
      };
    };
  };
}

@MattSturgeon
Copy link
Member

MattSturgeon commented Jul 29, 2024

Another work-around until we provide a propper solution is to remove your plugins.which-key.registrations definition and instead call require('which-key').add with the new format.

I've done this in my config: MattSturgeon/nix-config@e0cb861

EDIT: You can now use plugins.which-key.settings.spec which I've updated my config to use here: MattSturgeon/nix-config@b1fcbde

@mergify mergify bot closed this as completed in #1944 Jul 29, 2024
@khaneliman
Copy link
Contributor

khaneliman commented Jul 30, 2024

Not terribly related... but does anyone know how to keep which-key from spewing warnings every single startup?

While looking through the source, I did see https://github.com/folke/which-key.nvim/blob/6c1584eb76b55629702716995cca4ae2798a9cca/lua/which-key/config.lua#L26 which makes it look like you can suppress the warnings about mappings with the notify = false option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants