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
4.75k stars 154 forks source link

Fix: file/buffer-type disable does not prevent errors being thrown from keys.update function #578

Closed BirdeeHub closed 3 hours ago

BirdeeHub commented 4 months ago

in lua/which-key/init.lua show function:

The show function calls update before checking if which-key is enabled for that buffer type.

the update function can throw an error like the following, for example, in an oil.nvim buffer.

because disable option is not checked before calling it, even disabling which-key for that buffer or file type will not prevent these errors, making it impossible to use any keybinding that would have triggered the popup if it were enabled.

This means, to use oil, I would have to uninstall which-key

I added 1 check to prevent it from executing on disabled buffer and file types.

I thought it best to add the check to the update function itself rather than init's show function, in case it can get called from anywhere else in a disabled buffer.

The error I was getting on any keypress tracked by which-key within oil buffers despite it being disabled for oil filetype:

E5108: Error executing lua ...ovimPackages/start/which-key-nvim/lua/which-key/util.lua:128: {
  internal = { "g" },
  keystr = "g\\",
  notation = { "g", "\\" }
}
stack traceback:
        [C]: in function 'error'
        ...ovimPackages/start/which-key-nvim/lua/which-key/util.lua:128: in function 'parse_keys'
        ...ovimPackages/start/which-key-nvim/lua/which-key/keys.lua:425: in function 'update_keymaps'
        ...ovimPackages/start/which-key-nvim/lua/which-key/keys.lua:333: in function 'update'
        ...ovimPackages/start/which-key-nvim/lua/which-key/init.lua:48: in function 'show'
        [string ":lua"]:1: in main chunk

Note: I did not press g. I pressed space (my leader key) within an oil buffer in this particular error.

BirdeeHub commented 4 months ago

Ignore the force push hahaha I was not paying enough attention while combining if statements

BirdeeHub commented 4 months ago

This obviously doesn't fix the issue with compatibility with oil itself, but it is definitely an issue nonetheless

BirdeeHub commented 4 months ago

It appears the underlying issue with oil is caused by its g\\ keybind. Update is choking on it, and when the keybind is disabled, the error is not thrown. Oddly, changing the oil keybind still throws the error, but disabling it completely does not.

BirdeeHub commented 4 months ago

Regardless, the option for disable is broken so here is a fix.

I may make a new PR with an actual fix for oil later if I figure it out.

Edit: it seems it is not an oil issue. update throws an error on keys containing \\ because internal is \ and notation is \\

removing the keybind in oil required also setting it to false explicitly rather than providing a new value. However properly removing it does stop the error. But obviously, because it is a valid keybind, which-key ideally could handle a keybind like "g\\"

However, the fact that disable does not prevent update and thus prevents disabling of which-key for problematic buffers that throw errors is still a bug that needs fixing, which is fixed by this PR

BirdeeHub commented 1 month ago

I did end up figuring out that the issue with Oil was being caused by setting cpoptions without prepending the old value

However this PR is still relevant due to the fact that if other issues crop up with a filetype, it wont be disableable for only that file type

vim.o.cpoptions = (vim.o.cpoptions or '') .. 'I'

but yeah ^ doing the above instead of

vim.o.cpoptions = 'I'

At the very least solved my Oil issue

github-actions[bot] commented 6 days ago

This PR is stale because it has been open 60 days with no activity.

folke commented 3 hours ago

fixed in v3