search

d12frosted / flyspell-correct

Distraction-free words correction with flyspell via selected interface.
Other
203 stars 14 forks source link

Buffer-local post-command-hook #95

Open Stebalien opened 3 years ago

Stebalien commented 3 years ago

It turns out minibuffer buffers are reused. Seems a bit nuts, but that's how it is. Unfortunately, this means the post-command-hook registered by this library sticks around.

To reproduce:

  1. Type a misspelled word.
  2. Run flyspell-correct, exit without selecting anything.
  3. Open any minibuffer (e.g., read-string, M-x, etc.).
  4. Type "1".

The minibuffer will exit.

d12frosted commented 3 years ago

Hey @Stebalien, just tried to reproduce, but failed to. How do you exit flyspell-correct in the second step? I used C-g.

And btw, what completion framework and interface are you using? In my case it's selectrum and flyspell-correct-completing-read.

d12frosted commented 3 years ago

Ah, and just FYI/cc @minad 😸

Stebalien commented 3 years ago

vertico (but I can reproduce with the default as well) and flyspell-correct-at-point. And exiting with C-g does it.

minad commented 3 years ago

It turns out minibuffer buffers are reused. Seems a bit nuts, but that's how it is. Unfortunately, this means the post-command-hook registered by this library sticks around.

No, this is not the case. There is a bug with lambda expressions in hooks however, which is probably triggered here. But this only happens in combination with some other packages, e.g., evil. I assume you cannot reproduce the issue with emacs -Q?

In Consult I am using a symbol indirection to avoid the problem: https://github.com/minad/consult/blob/9acd32a764b6990d89eb1c9d74e1c43f6eaa24aa/consult.el#L1254-L1258. I assume that this should fix the issue. @Stebalien can you please give this a try?

Furthermore I am also using my own version of minibuffer--with-setup-hook in order to avoid a problem with large closures and add/remove-hook. See https://github.com/minad/consult/blob/9acd32a764b6990d89eb1c9d74e1c43f6eaa24aa/consult.el#L687-L688. This bug has fortunately been fixed on Emacs 28.

Stebalien commented 3 years ago

Ah, I see. So the minibuffer is reused, but there's was a bug in kill-all-local-variables (this one? https://git.savannah.gnu.org/cgit/emacs.git/commit/etc/NEWS?id=b4fabb316dfe59c75525cd37eaf87020582a9d12).

And yeah, I use evil. I will try to make fewer assumptions next time.

Stebalien commented 3 years ago

Yep. That fixes it. Thanks!

minad commented 3 years ago

So the minibuffer is reused, but there's was a bug in kill-all-local-variables

Okay, it seems like it. This is nonsense. At least the cleanup is fixed now.