nixvim icon indicating copy to clipboard operation
nixvim copied to clipboard

plugins/which-key: support registration with opts

Open justinrubek opened this issue 1 year ago • 1 comments

Currently the which-key module only supports calling register once and without opts: https://github.com/nix-community/nixvim/blob/0d85838d204d0f3d1cda22059d3fed0cd757d0ab/plugins/utils/which-key.nix#L211

I am still configuring this plugin manually because I have different keybinds for normal mode and for visual mode. It looks something like this:

local plugin = require("which-key")
local opts = {
    mode = "n",
    prefix = "<leader>",
    buffer = nil;
    silent = true,
    noremap = true,
    nowait = true,
}
local vopts = {
    mode = "v",
    prefix = "<leader>",
    buffer = nil;
    silent = true,
    noremap = true,
    nowait = true,
}
registrations = { ... }
vregistrations = { ... }
plugin.register(registrations, opts)
plugin.register(vregistrations, vopts)

It seems that I could handle normal mode commands having a different prefix by moving the prefix onto the keybinding, but after looking through the which-key docs I don't see any way to specify what mode a registration should belong to. I'd also prefer to keep these in a separate attrset anyway.

I'm not sure what this kind of change should look like. I only have two registrations but feasibly there could be an arbitrary number of them.

justinrubek avatar Apr 07 '24 21:04 justinrubek

Indeed that is something we would need to think about. In the mean time you can define the attrset of mappings in Nix, then add to your config:

extraLuaConfig = ''
  require("which-key").register(${helpers.toLuaObject yourRegistration}, ${helpers.toLuaObject yourVopts})
'';

traxys avatar Apr 08 '24 10:04 traxys

With which-key v3 registrations (https://github.com/nix-community/nixvim/pull/1944) the nature of this has changed somewhat, but I don't consider it solved. The current way to specify registrations is a single list under plugins.which-key.settings.spec. The only way to have bindings that are grouped (in my above example, I group normal mode vs visual mode) is to avoid using the nixvim module and to handle which-key directly.

I'd say that it has become even less ergonomic to use which-key than it was before. My main issue is that there's no way to specify multiple specs such that you can use inheritance on opts.

Here's what I've done to make my configuration work:

extraConfigLua = let
  names = builtins.attrNames bindings;
  generateRegistrations = name: let
    inherit (bindings.${name}) spec opts;
    finalSpec = (helpers.listToUnkeyedAttrs spec) // opts;
  in "require(\"which-key\").add(${helpers.toLuaObject finalSpec})";
in
  builtins.concatStringsSep "\n" (builtins.map generateRegistrations names);

This is using an attrset containing the spec and opts:

bindings.normal = {
  opts = { ... };
  spec = [ { ... } ];
};

Without something like this, you'd have to specify opts on each and every binding.

justinrubek avatar Aug 04 '24 00:08 justinrubek

I don't see this as something we should be plastering over in nixvim.

There's nothing wrong with having helper functions and completex definitions on the user side if the user wants to DRY up their config; nix is a programming language after all.

For simple configs, you can just use __unkeyed-1 and inherit or concatenate shared options. IIRC, I did something like this in the test.

But TLDR: nixvim wants to be a thin wrapper around plugin config; not actually modifying or transforming config definitions when rendering the nix value to lua.

MattSturgeon avatar Aug 04 '24 09:08 MattSturgeon

With which-key v3 registrations (#1944) the nature of this has changed somewhat, but I don't consider it solved. The current way to specify registrations is a single list under plugins.which-key.settings.spec. The only way to have bindings that are grouped (in my above example, I group normal mode vs visual mode) is to avoid using the nixvim module and to handle which-key directly.

I'd say that it has become even less ergonomic to use which-key than it was before. My main issue is that there's no way to specify multiple specs such that you can use inheritance on opts.

Here's what I've done to make my configuration work:

extraConfigLua = let
  names = builtins.attrNames bindings;
  generateRegistrations = name: let
    inherit (bindings.${name}) spec opts;
    finalSpec = (helpers.listToUnkeyedAttrs spec) // opts;
  in "require(\"which-key\").add(${helpers.toLuaObject finalSpec})";
in
  builtins.concatStringsSep "\n" (builtins.map generateRegistrations names);

This is using an attrset containing the spec and opts:

bindings.normal = {
  opts = { ... };
  spec = [ { ... } ];
};

Without something like this, you'd have to specify opts on each and every binding.

In testing https://github.com/nix-community/nixvim/pull/2023 I am not sure that the current implementation doesn't support this. Unless, it was recently changed by folke. The spec implementation supports nested mappings to share opts. You should be able to modify your code to just place those normal opts within the first item in the spec list and then have each of your normal spec mappings nested inside of that then create a second item for the visual, etc.

settings = {
      spec = [
        {
          mode = "n";
          __unkeyed-1 = [
            {
              __unkeyed = "<leader>b";
              group = "󰓩 Buffers";
            }
            {
              __unkeyed = "<leader>bs";
              group = "󰒺 Sort";
            }
            {
              __unkeyed = "<leader>g";
              group = "󰊢 Git";
            }
            {
              __unkeyed = "<leader>f";
              group = " Find";
            }
            {
              __unkeyed = "<leader>r";
              group = " Refactor";
            }
            {
              __unkeyed = "<leader>t";
              group = " Terminal";
            }
            {
              __unkeyed = "<leader>u";
              group = " UI/UX";
            }
          ];
        }
      ];

khaneliman avatar Aug 15 '24 14:08 khaneliman

I created https://github.com/nix-community/nixvim/pull/2025 to show some examples of the different configurations, including this nesting logic that would support this config.

khaneliman avatar Aug 15 '24 20:08 khaneliman

Yeah, at this point I'm not sure that there's anything worth doing about this, so I'm leaning towards closing this. I'll do that soon(ish) if that hasn't been done already by then.

I'm definitely displeased with how this turned out, but it is mostly on the which-key side of things which isn't relevant to nixvim.

justinrubek avatar Aug 22 '24 02:08 justinrubek

Other than improving how we can represent mixed list/dict tables as nix attrs*, I don't think there's any more we can do. Therefore I'll close the issue.

Thanks all for your efforts identifying the various issues and solutions here.

* I'm in favour of such improvements, but they should be discussed/tracked in another issue.

MattSturgeon avatar Aug 22 '24 07:08 MattSturgeon