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

plugins/which-key: support registration with opts #1392

Closed
justinrubek opened this issue Apr 7, 2024 · 7 comments
Closed

plugins/which-key: support registration with opts #1392

justinrubek opened this issue Apr 7, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@justinrubek
Copy link

Currently the which-key module only supports calling register once and without opts:

require("which-key").register(${helpers.toLuaObject cfg.registrations})

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.

@traxys
Copy link
Member

traxys commented Apr 8, 2024

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})
'';

@justinrubek
Copy link
Author

justinrubek commented Aug 4, 2024

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.

@MattSturgeon
Copy link
Member

MattSturgeon commented Aug 4, 2024

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.

@khaneliman
Copy link
Contributor

khaneliman commented Aug 15, 2024

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 #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
Copy link
Contributor

I created #2025 to show some examples of the different configurations, including this nesting logic that would support this config.

@justinrubek
Copy link
Author

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.

@MattSturgeon
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants