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

view/render: Merge keybindings in UI #1332

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

har7an
Copy link

@har7an har7an commented Feb 20, 2024

This PR updates the keybindings view in the "hints" section in lazy home.
Activation keys for plugins are now iterated in sorted order (by keys) and
common prefixes, if present, are shown instead of all the individual
keybindings.

Given a config like this:

return {
    {
        'rcarriga/nvim-notify',
        lazy = true,
        keys = {
            { "<leader>N",  desc = "+notifications" },
            { "<leader>Ny", "<Cmd>Notifications<CR>", desc = "history (plain)" },
            { "<leader>Nd", function() require("notify").dismiss() end, desc = "dismiss" },
            { "<leader>Np", function() require("notify").pending() end, desc = "show pending" },
        },
    },
}

The UI entry in the Not Loaded plugins now looks like this:

   ○ nvim-notify  <leader>N

instead of previously:

   ○ nvim-notify  <leader>Nd  <leader>Np  <leader>N  <leader>Ny

As a side-effect, this ensures that, even if no "common prefix" exists, keys
are always shown in sorted order.

Additional remarks

Compound keybindings

I assume that the current implementation fails when "Ctrl"-based keybindings
are mixed with anything that has "C" as a prefix, for example. I'm not sure
about the exact notation of Ctrl-/Alt-keybindings in nvim because I never use
them, but a quick test with this setup:

        keys = {
            { "<leader>C",  desc = "+notifications" },
            { "<leader>Cy", "<Cmd>Notifications<CR>", desc = "history (plain)" },
            { "<leader>C-d", function() require("notify").dismiss() end, desc = "dismiss" },
            { "<leader>Cp", function() require("notify").pending() end, desc = "show pending" },
        },

shortens the whole group to <leader>C in the UI, where it should probably
show <leader>C <leader>C-d instead. In a similar fashion, I assume that a
scenario like this fails as well:

        keys = {
            -- Plain "C" with plain "-"
            { "<leader>C-",  desc = "+notifications" },
            { "<leader>C-y", "<Cmd>Notifications<CR>", desc = "history (plain)" },
            { "<leader>C-d", function() require("notify").dismiss() end, desc = "dismiss" },
            { "<leader>C-p", function() require("notify").pending() end, desc = "show pending" },
        },

although I'm unsure whether this is a realistic scenario to consider here. Is
there something like an iterator over individual/compound keys in the raw data?
Or do I have to check whether C-? is part of a compound binding myself? If
so, do you know of any other compound bindings I should be aware of?

which-key integration

Users of which-key will probably not benefit from this change. That is
because keybinding groups must be registered in which-key directly (so the
"group"-prefix is picked up correctly), so lazy doesn't know about them. Since
keybindings can only be performed once, we cannot just "copy" the which-key
group assignment into lazy just for the UI.

I've opened a PR for which-key that solves this inconvenience, in case
you're interested.

in lazy overview when a common prefix exists.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant