which-key.nvim icon indicating copy to clipboard operation
which-key.nvim copied to clipboard

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

Open BirdeeHub opened this issue 6 months ago • 5 comments

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 avatar Feb 22 '24 10:02 BirdeeHub

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

BirdeeHub avatar Feb 22 '24 11:02 BirdeeHub

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

BirdeeHub avatar Feb 22 '24 11:02 BirdeeHub

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 avatar Feb 23 '24 02:02 BirdeeHub

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 avatar Feb 23 '24 02:02 BirdeeHub