bug#29150: Fwd: 26.0.90; Input decoding is sometimes skipped in TTY (xterm-mouse-mode)

[Olaf Rogalsky]

Eli Zaretskii writes:

>> in case of a button down event, `describe-key' has some trickery to also
>> read the forthcoming up event. The following patch makes this trickery
>> work with xterm-mouse-mode.
>
> Thanks.  Do you understand why read-key-sequence-vector works with
> xt-mouse, while read-event doesn't?  If so, can you elaborate on that?
Unlike read-key-sequence and read-key-sequence-vector, read-event does
not use input-decode-map.  xterm-mouse-mode relies on input-decode-map to
convert special byte sequences (starting with "\e[") into proper mouse
events. read-event stops reading after the first character ("\e") of the
byte sequence -- the remaining charcters are then inserted into the
current buffer.

> Also, I take it that you assume there will be only one element in the
> array returned by read-key-sequence-vector, is that right?  If so, how
> sure are we that this will always be the case?  Because if the
> assumption could be false, this change will have Emacs wait for some
> other input, and the user might think that Emacs hanged.
Yes, that was my assumption. read-key-sequence-vector is called after a
mouse-down event was read.  If someone presses a prefix key before
releasing the mouse or if the up event is a prefix to some binding, then
strange things may happen.  I wasn't able to think of and construct a
case, in which, after the user releases the mouse button,
read-key-sequence-vector still tries to read additional characters. In
any case, \C-g will interrupt read-key-sequence-vector.  But see below.

> Anyway, in general, I'm wary of such changes, which replace one API
> for reading input by another, which works subtly differently.  We had
> in the recent past several incidents where similar changes seemed to
> work, only to reveal many moons later that some rarely-used but useful
> functionality stopped working or became semi-broken.  So I think I'd
> prefer a fix that is specific to xt-mouse (assuming that we can
> reliably detect that the clicks come from xt-mouse), and leave the
> other use cases alone.  If such a solution is possible and makes
> sense, we could even install it on the release branch.
If you are still not convinced, that read-key-sequence-vector is
harmless, then please find a modified patch below.  There I check, as
suggested, that the mouse-down event under consideration was generated
by xterm-mouse-mode.  This is quite easy to accomplish, because
xterm-mouse anyway remembers the last mouse-down event in a terminal
paramerter.

diff --git a/lisp/help.el b/lisp/help.el
index bc8035db0e..fbb9fc8cbe 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -717,7 +717,7 @@ help-read-key-sequence
         (cursor-in-echo-area t)
         saved-yank-menu)
     (unwind-protect
-        (let (key)
+        (let (key down-ev)
           ;; If yank-menu is empty, populate it temporarily, so that
           ;; "Select and Paste" menu can generate a complete event.
           (when (null (cdr yank-menu))
@@ -743,17 +743,21 @@ help-read-key-sequence
                 (let ((last-idx (1- (length key))))
                   (and (eventp (aref key last-idx))
                        (memq 'down (event-modifiers (aref key last-idx)))))
-                (or (and (eventp (aref key 0))
-                         (memq 'down (event-modifiers (aref key 0)))
+                (or (and (eventp (setq down-ev (aref key 0)))
+                         (memq 'down (event-modifiers down-ev))
                          ;; However, for the C-down-mouse-2 popup
                          ;; menu, there is no subsequent up-event.  In
                          ;; this case, the up-event is the next
                          ;; element in the supplied vector.
                          (= (length key) 1))
                     (and (> (length key) 1)
-                         (eventp (aref key 1))
-                         (memq 'down (event-modifiers (aref key 1)))))
-                (read-event))))
+                         (eventp (setq down-ev (aref key 1)))
+                         (memq 'down (event-modifiers down-ev))))
+                (if (and (terminal-parameter nil 'xterm-mouse-mode)
+                         (equal (terminal-parameter nil 'xterm-mouse-last-down)
+                                down-ev))
+                    (aref (read-key-sequence-vector nil) 0)
+                  (read-event)))))
       ;; Put yank-menu back as it was, if we changed it.
       (when saved-yank-menu
         (setq yank-menu (copy-sequence saved-yank-menu))

>> PS: It would be nice, if that person also can have a look at patch #29104
>
> It's in my queue, if no one else beats me to it.  And there, too, more
> detailed description of what you saw and what led you to your proposed
> solution might go a long way towards admitting the change sooner.
Thanks a lot.  I will comment on that patch in a separate email.

Olaf

-- 
Olaf Rogalsky
Schwörhausgasse 5
89073 Ulm
Germany