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: which_key.show dead-loops if the key has been remapped #743

Closed
4 tasks done
moetayuko opened this issue Jul 17, 2024 · 7 comments
Closed
4 tasks done

bug: which_key.show dead-loops if the key has been remapped #743

moetayuko opened this issue Jul 17, 2024 · 7 comments
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@moetayuko
Copy link

moetayuko commented Jul 17, 2024

Did you check docs and existing issues?

  • I have read all the which-key.nvim docs
  • I have updated the plugin to the latest version before submitting this issue
  • 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

Operating system/version

Arch

Describe the bug

I'm adapting the which-key part of aserowy/tmux.nvim#88 for the recent refactor. The goal is to wrap the " key to invoke require("tmux.copy").sync_registers() before showing register contents.

The old implementation for which-key before refactoring was:

vim.keymap.set("n", [["]], function()
  if vim.env.TMUX then
    require("tmux.copy").sync_registers()
  end
  which_key.show('"', { mode = "n", auto = true })
end)

and I updated it to:

vim.keymap.set("n", [["]], function()
  if vim.env.TMUX then
    require("tmux.copy").sync_registers()
  end
  require("which-key").show({ keys = '"', mode = "n" })
end)

However, which_key.show dead-loops with the new implementation.

Steps To Reproduce

  1. setup the keymap
  2. press " in normal mode
  3. after which-key popup, press any register key such as +
  4. neovim dead-loops, press ctrl-c to break out.

Expected Behavior

no dead loop after selecting the register.

Health

No response

Log

No response

Repro

vim.env.LAZY_STDPATH = ".repro"
load(vim.fn.system("curl -s https://raw.githubusercontent.com/folke/lazy.nvim/main/bootstrap.lua"))()

require("lazy.minit").repro({
        spec = {
                { "folke/which-key.nvim", opts = {} },
                -- add any other plugins here
        },
})

vim.keymap.set("n", [["]], function()
        require("which-key").show({ keys = '"', mode = "n" })
end)
@moetayuko moetayuko added the bug Something isn't working label Jul 17, 2024
@folke
Copy link
Owner

folke commented Jul 17, 2024

You did not:

  • I have updated the plugin to the latest version before submitting this issue

@folke folke closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2024
@moetayuko
Copy link
Author

You did not:

* [ ]   I have updated the plugin to the latest version before submitting this issue

Yes I do!

❯ git -C .repro/data/nvim/lazy/which-key.nvim rev-parse HEAD
f29387d82055b091564b33a805fec8f1b52b39f9

Updated the issue to include an MRE

@folke folke reopened this Jul 17, 2024
@folke
Copy link
Owner

folke commented Jul 17, 2024

ok, sorry about that.

I just looked into it and the reason that used to work is due to a bug in the old version.
The old version just removed triggers without checking that it was one created by which-key. As such it removed your keymap.

The new version won't even create a trigger at " since that already exists.

So what happens is:

  • your keymap runs
  • that executes which-key to show "
  • then you press + for example
  • all wk triggers will now be removed (but not your keymap, since that's not a trigger)
  • feedkeys('"+')
  • that will trigger your keymap again

So not an issue in which-key.

@folke folke closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2024
@folke folke added the wontfix This will not be worked on label Jul 17, 2024
@moetayuko
Copy link
Author

I just looked into it and the reason that used to work is due to a bug in the old version.

This is interesting. Anyway, is it possible to inject function call before " w/o abusing the "bug"?

@folke
Copy link
Owner

folke commented Jul 17, 2024

Probably, but that's for you to find out?

Maybe keep a wk variable that is by defult false. Set it to true when you run wk, and set to false again in a vim.schedule.
Then in your keymap, when it's true, return " instead. Change the keymap to be an expr keymap.

Might work, maybe not. I'm sure you'll figure something out

@folke
Copy link
Owner

folke commented Jul 17, 2024

A lot easier would be to just do:

local reg = require("which-key.plugins.registers")
local expand = reg.expand

function reg.expand()
  if vim.env.TMUX then
    require("tmux.copy").sync_registers()
  end
  return expand()
end

This way you don't have to mess with keymaps at all.

@moetayuko
Copy link
Author

A lot easier would be to just do:

local reg = require("which-key.plugins.registers")
local expand = reg.expand

function reg.expand()
  if vim.env.TMUX then
    require("tmux.copy").sync_registers()
  end
  return expand()
end

This way you don't have to mess with keymaps at all.

This solution looks much simpler and more robust, thanks for pointing it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants