emacsorphanage / key-chord

Map pairs of simultaneously pressed keys to commands
http://emacswiki.org/emacs/download/key-chord.el
115 stars 23 forks source link

key-chord stops working after Emacs farts #8

Closed ghost-of-freedom closed 8 months ago

ghost-of-freedom commented 1 year ago

For some time now, I had moments where my "hh" global key-chord binding would stop working, and I could not find a way to reliably reproduce the issue, until today, when some code I wrote to answer a friend's question turned out to reproduce it perfectly. After evaluating the call (with eval-last-sexp) to make Emacs emit a fart sound, and typing hhh, the letters would be typed into the buffer, and h would now be bound to self-insert-command, instead of the key-chord.

Minimal config to reproduce:

(use-package key-chord
  :config
  (key-chord-define-global "hh" 'kill-emacs)
  (key-chord-mode +1))

Actions to reproduce:

  1. switch to scratch buffer
  2. evaluate (play-sound '(sound :file "/path/to/fart.wav" :volume 20) using C-x C-e
  3. while Emacs is farting, quickly press 'h' 3 times

If this issue is not something that affects only me, this should cause Emacs to type hhh into the scratch buffer, and make hh key-chord stop working. Some actions seem to make it work again, like doing F1 v (invoking help for variable) and typing some variable name. I thought that maybe switching buffers makes it work again, but that doesn't seem to be the case.

In case your Emacs comes without farting capabilities, I believe you can also try something like (sleep-for 2) instead, to the same effect.

If you type only the key-chord (e.g. hh exactly instead of hhh or hhe or whatever), it works fine, and continues to do so.

To be clear, I'm not talking about the fact that typing something other than the exact key-chord sequence inserts text instead of firing the command (which I assume is because the input is cached somewhere when Emacs is busy evaluating, and then replayed without delays), but that key-chord stops working afterwards.

Emacs version: GNU Emacs 28.2 Keychord version: master@e724def60fdf6473858f2962ae276cf4413473eb

ghost-of-freedom commented 1 year ago

Okay so I briefly looked into it, and it seems that https://github.com/emacsorphanage/key-chord/pull/2 introduced this.

I managed to narrow it down to:

              (cond ((and (key-chord-lookup-key res)
                                 (sit-for key-chord-safety-interval-forward 'no-redisplay))
                         (setq key-chord-defining-kbd-macro ...

which replaced

               (cond ((key-chord-lookup-key res)
                          (setq key-chord-defining-kbd-macro ...

(see https://github.com/emacsorphanage/key-chord/pull/2/files?diff=split&w=1#diff-897872db39af5932c4354fcf6d510539f3da02be917d137b9801fc5bc1a28231R214-R215)

Now, my question is: does anybody even really understand what this PR is useful for, and why the sit-for invocation has to be called in and? Per documentation Value is t if waited the full time with no input arriving, and nil otherwise, which I'm pretty sure causes the wrong condition to fire in the situation described in the OP. When that line is moved out of the and, like this:

               (cond ((key-chord-lookup-key res)
                  (sit-for key-chord-safety-interval-forward 'no-redisplay)
                          (setq key-chord-defining-kbd-macro ...

The issue is gone, and key-chord is still delayed (which, from looking at the issues, I can only assume was the intent of this PR). Do you think that's an acceptable change?

And on another note, I would like to ask: is anybody actually happy about this PR being merged? If anybody can actually explain whether they find it useful, and why, I think that could be helpful. From what I've seen, the discussion went like this:

which is not a good sign. If nobody actually voices support for this change, maybe you could consider reverting it, as it seems to break more things than it improves.

marcoheisig commented 1 year ago

I am also not happy about e724def60fdf6473858f2962ae276cf4413473eb. Even after setting both the forward and the backward safety interval to zero, my working experience with key-chord-mode is significantly worse than before. Every now and then, key-chord mode completely stops detecting chords.

tarsius commented 8 months ago

I've reverted the commit that caused this.