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

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

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

Conversation

BirdeeHub
Copy link

@BirdeeHub BirdeeHub commented Feb 22, 2024

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

BirdeeHub commented Feb 22, 2024

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

@BirdeeHub
Copy link
Author

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

@BirdeeHub BirdeeHub changed the title Fix: disable does not prevent errors from update function Fix: file/buffer type disable does not prevent errors being thrown from keys.update function Feb 22, 2024
@BirdeeHub BirdeeHub changed the title Fix: file/buffer type disable does not prevent errors being thrown from keys.update function Fix: file/buffer-type disable does not prevent errors being thrown from keys.update function Feb 23, 2024
@BirdeeHub
Copy link
Author

BirdeeHub commented Feb 23, 2024

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

BirdeeHub commented Feb 23, 2024

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

BirdeeHub commented Jun 1, 2024

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

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