folke / which-key.nvim

💥 Create key bindings that stick. WhichKey helps you remember your Neovim keymaps, by showing available keybindings in a popup as you type.
Apache License 2.0
5.24k stars 165 forks source link

bug: Keys.update_keymaps only adds, never removes #615

Closed isakbm closed 2 months ago

isakbm commented 3 months ago

Did you check docs and existing issues?

Neovim version (nvim -v)

NVIM v0.10.0 Build type: Release LuaJIT 2.1.1713484068

Operating system/version

Ubuntu 22.04.3 LTS

Describe the bug

When a buffer local keymap is removed, which-key will still show it as if it exists. In other words, which-key only adds buffer local keymaps to the tree, it never removes them.

Steps To Reproduce

Add the following two keymaps, the first one adds the buffer local keymap to current buffer, the second one removes it.

vim.keymap.set('n', '<leader>A', function()
  vim.keymap.set('n', '<leader>aa', function()
    print 'aa'
  end, { buffer = vim.api.nvim_get_current_buf() })
end)

vim.keymap.set('n', '<leader>S', function()
  vim.keymap.del('n', '<leader>aa', { buffer = vim.api.nvim_get_current_buf() })
end)
  1. Open some file like your init.lua.
  2. Do <leader>A
  3. Do <leader> and observe how a -> appears in the which-key help window
  4. Do <leader>S
  5. Do <leader> and observe how a -> still appears in the which-key help window

Expected Behavior

It is expected that in step 5 of the steps to reproduce, that we no longer see the keymap listed.

Repro

Just take any regular kickstart nvim config and add the key bindings from the `steps to reproduce`.
isakbm commented 3 months ago

There is a simple fix for this, simply clearing the buffer local keymaps tree by adding the following single line of code

  M.get_tree(mode, buf).tree = Tree:new()

Specifically adding this to the file lua/which-key/keys.lua in the function responsible for updating keys function M.update_keymaps(mode, buf)

NOTE: however that doing this naive change seems to cause nvim to hang and become unresponsive. The cause seems to be that in addition to the reset of the tree, we need to also delete the "hooks". Will be attempting to do this as well.

isakbm commented 3 months ago

After some more investigation I've found that this also happens with non local keymaps.

isakbm commented 3 months ago

I found something that seems to work , will attempt making a PR soon

function M.update_keymaps(mode, buf)
  ---@type Keymap[]
  local keymaps = buf and vim.api.nvim_buf_get_keymap(buf, mode) or vim.api.nvim_get_keymap(mode)

  -- Clearing up previous tree entries and hooks.
  -- Forgetting to do this will break which-key for users that register and de register
  -- keymaps in different contexts
  do
    local function unhook(node)
      if node.mapping and M.is_hooked(node.mapping.prefix, mode, buf) then
        M.hook_del(node.mapping.prefix, mode, buf)
      end
    end
    M.get_tree(mode, buf).tree:walk(unhook)
    M.get_tree(mode, buf).tree = Tree:new()
  end
ditsuke commented 3 months ago

I can confirm this issue through trying to resolve https://github.com/LazyVim/LazyVim/discussions/3863. Even with the neovim-default keymaps removed through nunmap / vim.keymap.del, which-key retains them and causes conflicts when activated.

folke commented 3 months ago

I'll see if I can find some time today to fully test that PR.

which-key is quite fragile (was my first plugin), so I'm always a bit hesitant to merge a PR.