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

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

Closed
3 tasks done
isakbm opened this issue Jun 29, 2024 · 5 comments
Closed
3 tasks done

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

isakbm opened this issue Jun 29, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@isakbm
Copy link

isakbm commented Jun 29, 2024

Did you check docs and existing issues?

  • I have read all the which-key.nvim docs
  • I have searched the existing issues of which-key.nvim
  • I have searched the existing issues of plugins related to this issue

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 isakbm added the bug Something isn't working label Jun 29, 2024
@isakbm
Copy link
Author

isakbm commented Jun 29, 2024

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
Copy link
Author

isakbm commented Jun 30, 2024

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

@isakbm
Copy link
Author

isakbm commented Jun 30, 2024

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

isakbm added a commit to isakbm/which-key.nvim that referenced this issue Jun 30, 2024
fixes folke#615

current approach fails to support removal of keymaps

this fix ensures that both tree nodes and hooks
related to removed keymaps are cleared
@ditsuke
Copy link

ditsuke commented Jul 4, 2024

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

@folke
Copy link
Owner

folke commented Jul 4, 2024

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.

@folke folke closed this as completed Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants