Re: Additional cleanup around xterm-mouse

[Jared Finder via "Emacs development discussions." <emacs-devel@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 3391 bytes --]

On 2020-11-18 9:40 am, Eli Zaretskii wrote:
>> Date: Sun, 15 Nov 2020 22:29:20 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> Creating such an alternative function doesn't appear too bad if you're
>> okay with having the same run-with-idle-timer pattern that read-key
>> uses.  I do not think it can be xterm specific as it needs to apply 
>> all
>> of input-decode-map to be able to return function keys such as [f1] on 
>> a
>> native Linux term or an xterm.  (This is important for
>> widget-key-sequence-read-event.)
> 
> I don't think I follow.  All the places where you need changes are
> related to handling mouse events, so why cannot it be specific to
> xt-mouse?

There is a bug in widget-key-sequence-read-event's usage of keyboard 
events specifically.

The function widget-key-sequence-read-event currently does not correctly 
translate function keys for the terminal.  It has code that attempts to 
apply function-key-map, but does not apply input-decode-map, so it can 
not read function keys.  (Additionally, it should be using 
local-function-key-map now.)  Try the following steps.

Git revision: d5ac66
TERM env var: linux

Run "emacs -Q"
M-x load-library RET outline RET
M-x customize-variable RET outline-minor-mode-prefix RET
Move the cursor into the editable field.
C-q (runs the command widget-key-sequence-read-event)
Press f1.

I get the untranslated escape code sequence for [f1] put in.
I should get [f1].

Changing to read-key fixes this as read-key goes through 
read-key-sequence which applies input-decode-map.  However, to get the 
same intended functionality requires some new parameters, which I also 
add in my patch.

And as a point of history, widget-key-sequence-read-event appears to 
have been added in Jan 2006, and input-decode-map was added in Oct 2007.

>> An alternative is to just use read-key as is in most cases and make my
>> change a parameter / special variable.  Most of my patch's changes 
>> work
>> fine with the existing behavior of read-key.  Only the following 
>> changes
>> do not:
>> 
>> * lisp/vc/ediff-wind.el (ediff-get-window-by-clicking)
>> ==> As coded, expects the first mouse event returned by read-event to 
>> be
>> a down-mouse-X event, which it then follows by another call to
>> read-event to get the mouse-X event.  It could be easily changed to 
>> only
>> look for the up event.
>> 
>> * lisp/strokes.el (strokes-read-stroke, strokes-read-complex-stroke)
>> * lisp/textmodes/artist.el (artist-mode-draw-poly)
>> ==> These both expect to detect a mix of down-mouse-X and mouse-X
>> events.
>> 
>> * lisp/wid-edit.el (widget-key-sequence-read-event)
>> ==> This w/o changes to read-key, but with a behavior change.  With no
>> changes to read-key it returns just a single up event.  Currently on
>> other environments you get both a down and up event (e.g. 
>> <down-mouse-1>
>> <mouse-1>).
> 
> I think I like this latter alternative better.  It is slightly less
> elegant, but simpler and less risky.  Can you show a draft of a patch
> along those lines?

Attached.  I added additional parameters to read-key. only strokes.el, 
artist.el, and wid-edit.el use the additional parameters.  Let me know 
what you think of this structure.  There's still further cleanup to do 
(add docs, clean up parameter names, etc.), that I'll do after you're 
happy with the structure.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-Make-libraries-work-with-xterm-mouse-mode.patch --]
[-- Type: text/x-diff; name=0001-WIP-Make-libraries-work-with-xterm-mouse-mode.patch, Size: 15109 bytes --]

From ad7a393f6c3b3f8b8e32714283c635c996d7e4b6 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sun, 15 Nov 2020 13:51:42 -0800
Subject: [PATCH] WIP: Make libraries work with xterm-mouse-mode.

This patch includes a slight modification to read-key and
read_key_sequence, but no change to the default behavior of these
functions.
---
 lisp/foldout.el          |  2 +-
 lisp/isearch.el          |  2 +-
 lisp/mouse-drag.el       |  4 ++--
 lisp/mouse.el            |  2 +-
 lisp/ruler-mode.el       |  4 ++--
 lisp/strokes.el          | 22 +++++++++++-----------
 lisp/subr.el             | 17 +++++++++++++++--
 lisp/textmodes/artist.el |  6 +++---
 lisp/vc/ediff-wind.el    |  5 +++--
 lisp/vc/ediff.el         |  2 +-
 lisp/wid-edit.el         | 15 +++++++++------
 src/keyboard.c           | 10 +++++++++-
 src/lread.c              |  6 ++++++
 13 files changed, 64 insertions(+), 33 deletions(-)

diff --git a/lisp/foldout.el b/lisp/foldout.el
index 0d7a7a88a6..0a33099daf 100644
--- a/lisp/foldout.el
+++ b/lisp/foldout.el
@@ -487,7 +487,7 @@ foldout-mouse-swallow-events
 Signal an error if the final event isn't the same type as the first one."
   (let ((initial-event-type (event-basic-type event)))
     (while (null (sit-for (/ double-click-time 1000.0) 'nodisplay))
-      (setq event (read-event)))
+      (setq event (read-key)))
     (or (eq initial-event-type (event-basic-type event))
 	(error "")))
   event)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 4fba4370d9..aa623652b3 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2967,7 +2967,7 @@ isearch-pre-command-hook
      ((and (eq (car-safe main-event) 'down-mouse-1)
 	   (window-minibuffer-p (posn-window (event-start main-event))))
       ;; Swallow the up-event.
-      (read-event)
+      (read-key)
       (setq this-command 'isearch-edit-string))
      ;; Don't terminate the search for motion commands.
      ((and isearch-yank-on-move
diff --git a/lisp/mouse-drag.el b/lisp/mouse-drag.el
index e80ebba28d..dcffbf0875 100644
--- a/lisp/mouse-drag.el
+++ b/lisp/mouse-drag.el
@@ -225,7 +225,7 @@ mouse-drag-throw
       ;; Don't change the mouse pointer shape while we drag.
       (setq track-mouse 'dragging)
       (while (progn
-	       (setq event (read-event)
+	       (setq event (read-key)
 		     end (event-end event)
 		     row (cdr (posn-col-row end))
 		     col (car (posn-col-row end)))
@@ -286,7 +286,7 @@ mouse-drag-drag
 	  window-last-col (- (window-width) 2))
     (track-mouse
       (while (progn
-	       (setq event (read-event)
+	       (setq event (read-key)
 		     end (event-end event)
 		     row (cdr (posn-col-row end))
 		     col (car (posn-col-row end)))
diff --git a/lisp/mouse.el b/lisp/mouse.el
index 9d4492f1bd..d0cd2f7769 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -1792,7 +1792,7 @@ mouse-drag-secondary
       (let (event end end-point)
 	(track-mouse
 	  (while (progn
-		   (setq event (read-event))
+		   (setq event (read-key))
 		   (or (mouse-movement-p event)
 		       (memq (car-safe event) '(switch-frame select-window))))
 
diff --git a/lisp/ruler-mode.el b/lisp/ruler-mode.el
index 82e6178da1..29afa44323 100644
--- a/lisp/ruler-mode.el
+++ b/lisp/ruler-mode.el
@@ -429,7 +429,7 @@ ruler-mode-mouse-grab-any-column
          ;; `ding' flushes the next messages about setting goal
          ;; column.  So here I force fetch the event(mouse-2) and
          ;; throw away.
-         (read-event)
+         (read-key)
          ;; Ding BEFORE `message' is OK.
          (when ruler-mode-set-goal-column-ding-flag
            (ding))
@@ -460,7 +460,7 @@ ruler-mode-mouse-drag-any-column-iteration
     (track-mouse
       ;; Signal the display engine to freeze the mouse pointer shape.
       (setq track-mouse 'dragging)
-      (while (mouse-movement-p (setq event (read-event)))
+      (while (mouse-movement-p (setq event (read-key)))
         (setq drags (1+ drags))
         (when (eq window (posn-window (event-end event)))
           (ruler-mode-mouse-drag-any-column event)
diff --git a/lisp/strokes.el b/lisp/strokes.el
index c2f03cac0f..e5e6fe3f9f 100644
--- a/lisp/strokes.el
+++ b/lisp/strokes.el
@@ -757,12 +757,12 @@ strokes-read-stroke
 	      (strokes-fill-current-buffer-with-whitespace))
 	    (when prompt
 	      (message "%s" prompt)
-	      (setq event (read-event))
+	      (setq event (read-key nil t))
 	      (or (strokes-button-press-event-p event)
 		  (error "You must draw with the mouse")))
 	    (unwind-protect
 		(track-mouse
-		  (or event (setq event (read-event)
+		  (or event (setq event (read-key nil t)
 				  safe-to-draw-p t))
 		  (while (not (strokes-button-release-event-p event))
 		    (if (strokes-mouse-event-p event)
@@ -777,7 +777,7 @@ strokes-read-stroke
 			    (setq safe-to-draw-p t))
 			  (push (cdr (mouse-pixel-position))
 				pix-locs)))
-		    (setq event (read-event)))))
+		    (setq event (read-key nil t)))))
 	    ;; protected
 	    ;; clean up strokes buffer and then bury it.
 	    (when (equal (buffer-name) strokes-buffer-name)
@@ -788,16 +788,16 @@ strokes-read-stroke
       ;; Otherwise, don't use strokes buffer and read stroke silently
       (when prompt
 	(message "%s" prompt)
-	(setq event (read-event))
+	(setq event (read-key nil t))
 	(or (strokes-button-press-event-p event)
 	    (error "You must draw with the mouse")))
       (track-mouse
-	(or event (setq event (read-event)))
+	(or event (setq event (read-key nil t)))
 	(while (not (strokes-button-release-event-p event))
 	  (if (strokes-mouse-event-p event)
 	      (push (cdr (mouse-pixel-position))
 		    pix-locs))
-	  (setq event (read-event))))
+	  (setq event (read-key nil t))))
       (setq grid-locs (strokes-renormalize-to-grid (nreverse pix-locs)))
       (strokes-fill-stroke
        (strokes-eliminate-consecutive-redundancies grid-locs)))))
@@ -818,10 +818,10 @@ strokes-read-complex-stroke
 	(if prompt
 	    (while (not (strokes-button-press-event-p event))
 	      (message "%s" prompt)
-	      (setq event (read-event))))
+	      (setq event (read-key nil t))))
 	(unwind-protect
 	    (track-mouse
-	      (or event (setq event (read-event)))
+	      (or event (setq event (read-key nil t)))
 	      (while (not (and (strokes-button-press-event-p event)
 			       (eq 'mouse-3
 				   (car (get (car event)
@@ -835,14 +835,14 @@ strokes-read-complex-stroke
 						?\s strokes-character))
 			(push (cdr (mouse-pixel-position))
 			      pix-locs)))
-		  (setq event (read-event)))
+		  (setq event (read-key nil t)))
 		(push strokes-lift pix-locs)
 		(while (not (strokes-button-press-event-p event))
-		  (setq event (read-event))))
+		  (setq event (read-key nil t))))
 	      ;; ### KLUDGE! ### sit and wait
 	      ;; for some useless event to
 	      ;; happen to fix the minibuffer bug.
-	      (while (not (strokes-button-release-event-p (read-event))))
+	      (while (not (strokes-button-release-event-p (read-key nil t))))
 	      (setq pix-locs (nreverse (cdr pix-locs))
 		    grid-locs (strokes-renormalize-to-grid pix-locs))
 	      (strokes-fill-stroke
diff --git a/lisp/subr.el b/lisp/subr.el
index 6e9f66fe97..c462ba0794 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2431,13 +2431,21 @@ read-key-empty-map
 
 (defvar read-key-delay 0.01) ;Fast enough for 100Hz repeat rate, hopefully.
 
-(defun read-key (&optional prompt)
+(defun read-key (&optional prompt all-mouse-events no-function-key-map
+                           no-key-translation-map)
   "Read a key from the keyboard.
 Contrary to `read-event' this will not return a raw event but instead will
 obey the input decoding and translations usually done by `read-key-sequence'.
 So escape sequences and keyboard encoding are taken into account.
 When there's an ambiguity because the key looks like the prefix of
-some sort of escape sequence, the ambiguity is resolved via `read-key-delay'."
+some sort of escape sequence, the ambiguity is resolved via `read-key-delay'.
+
+If the optional argument PROMPT is non-nil, display that as a
+prompt.
+
+If the optional argument ALL-MOUSE-EVENTS is non-nil, then
+button-down and multi-click events will get reported.  Otherwise,
+these are ignored as in `read-key-sequence'."
   ;; This overriding-terminal-local-map binding also happens to
   ;; disable quail's input methods, so although read-key-sequence
   ;; always inherits the input method, in practice read-key does not
@@ -2446,6 +2454,11 @@ read-key
 	(overriding-local-map read-key-empty-map)
         (echo-keystrokes 0)
 	(old-global-map (current-global-map))
+        (inhibit--unbound-mouse-fallback all-mouse-events)
+        (local-function-key-map (if no-function-key-map read-key-empty-map
+                                  local-function-key-map))
+        (key-translation-map (if no-key-translation-map read-key-empty-map
+                               key-translation-map))
         (timer (run-with-idle-timer
                 ;; Wait long enough that Emacs has the time to receive and
                 ;; process all the raw events associated with the single-key.
diff --git a/lisp/textmodes/artist.el b/lisp/textmodes/artist.el
index 5ce9a90ea6..ecffcf3901 100644
--- a/lisp/textmodes/artist.el
+++ b/lisp/textmodes/artist.el
@@ -5016,7 +5016,7 @@ artist-mouse-draw-continously
                   (setq timer (run-at-time interval interval draw-fn x1 y1))))
 
             ;; Read next event
-            (setq ev (read-event))))
+            (setq ev (read-key))))
       ;; Cleanup: get rid of any active timer.
       (if timer
           (cancel-timer timer)))
@@ -5224,7 +5224,7 @@ artist-mouse-draw-poly
 
 	;; Read next event (only if we should not stop)
 	(if (not done)
-	    (setq ev (read-event)))))
+	    (setq ev (read-key nil t)))))
 
     ;; Reverse point-list (last points are cond'ed first)
     (setq point-list (reverse point-list))
@@ -5351,7 +5351,7 @@ artist-mouse-draw-2points
 
 
 	;; Read next event
-	(setq ev (read-event))))
+	(setq ev (read-key))))
 
     ;; If we are not rubber-banding (that is, we were moving around the `2')
     ;; draw the shape
diff --git a/lisp/vc/ediff-wind.el b/lisp/vc/ediff-wind.el
index a23d72070a..1fd386fe39 100644
--- a/lisp/vc/ediff-wind.el
+++ b/lisp/vc/ediff-wind.el
@@ -269,11 +269,12 @@ ediff-get-window-by-clicking
   (let (event)
     (message
      "Select windows by clicking.  Please click on Window %d " wind-number)
-    (while (not (ediff-mouse-event-p (setq event (read-event))))
+    (while (not (ediff-mouse-event-p (setq event (read-key))))
       (if (sit-for 1) ; if sequence of events, wait till the final word
 	  (beep 1))
       (message "Please click on Window %d " wind-number))
-    (read-event) ; discard event
+    ;; No need to read an additional event as read-key only returns up
+    ;; events.
     (posn-window (event-start event))))
 
 
diff --git a/lisp/vc/ediff.el b/lisp/vc/ediff.el
index ae2f8ad6c1..bf35cd2bd1 100644
--- a/lisp/vc/ediff.el
+++ b/lisp/vc/ediff.el
@@ -939,7 +939,7 @@ ediff-windows-linewise
 ;; If WIND-A is nil, use selected window.
 ;; If WIND-B is nil, use window next to WIND-A.
 (defun ediff-windows (dumb-mode wind-A wind-B startup-hooks job-name word-mode)
-  (if (or dumb-mode (not (ediff-window-display-p)))
+  (if (or dumb-mode (not (display-mouse-p)))
       (setq wind-A (ediff-get-next-window wind-A nil)
 	    wind-B (ediff-get-next-window wind-B wind-A))
     (setq wind-A (ediff-get-window-by-clicking wind-A nil 1)
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 4e2cf7416d..8416f73964 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1088,7 +1088,7 @@ widget-button--check-and-call-button
 		  (unless (widget-apply button :mouse-down-action event)
 		    (let ((track-mouse t))
 		      (while (not (widget-button-release-event-p event))
-		        (setq event (read-event))
+                        (setq event (read-key))
 		        (when (and mouse-1 (mouse-movement-p event))
 			  (push event unread-command-events)
 			  (setq event oevent)
@@ -1153,7 +1153,7 @@ widget-button-click
 	    (when up
 	      ;; Don't execute up events twice.
 	      (while (not (widget-button-release-event-p event))
-		(setq event (read-event))))
+		(setq event (read-key))))
 	    (when command
 	      (call-interactively command)))))
     (message "You clicked somewhere weird.")))
@@ -3465,11 +3465,14 @@ 'key-sequence
 (defun widget-key-sequence-read-event (ev)
   (interactive (list
 		(let ((inhibit-quit t) quit-flag)
-		  (read-event "Insert KEY, EVENT, or CODE: "))))
+		  (read-key "Insert KEY, EVENT, or CODE: " t t t))))
   (let ((ev2 (and (memq 'down (event-modifiers ev))
-		  (read-event)))
-	(tr (and (keymapp function-key-map)
-		 (lookup-key function-key-map (vector ev)))))
+		  (read-key)))
+        ;; This is actually a separate bug-fix.  `function-key-map'
+        ;; does not contain any term-specific function key mappings
+        ;; like f13 --> S-f1.
+        (tr (and (keymapp local-function-key-map)
+		 (lookup-key local-function-key-map (vector ev)))))
     (when (and (integerp ev)
 	       (or (and (<= ?0 ev) (< ev (+ ?0 (min 10 read-quoted-char-radix))))
 		   (and (<= ?a (downcase ev))
diff --git a/src/keyboard.c b/src/keyboard.c
index 45e9abc229..483af75158 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -9827,7 +9827,7 @@ read_key_sequence (Lisp_Object *keybuf, Lisp_Object prompt,
       new_binding = follow_key (current_binding, key);
 
       /* If KEY wasn't bound, we'll try some fallbacks.  */
-      if (!NILP (new_binding))
+      if (!NILP (new_binding) || inhibit_unbound_mouse_fallback)
 	/* This is needed for the following scenario:
 	   event 0: a down-event that gets dropped by calling replay_key.
 	   event 1: some normal prefix like C-h.
@@ -12393,6 +12393,14 @@ syms_of_keyboard (void)
 macros, dribble file, and `recent-keys'.
 Internal use only.  */);
 
+  DEFVAR_BOOL ("inhibit--unbound-mouse-fallback",
+               inhibit_unbound_mouse_fallback,
+               doc: /* If non-nil, `read-key-sequence' does not
+transform any unbound mouse events.
+This prevents the usual behavior in `read-key-sequence' where unbound
+button-down events, drag events, and multiple-click events get
+transformed or dropped.  Internal use only.  */);
+
   pdumper_do_now_and_after_load (syms_of_keyboard_for_pdumper);
 }
 
diff --git a/src/lread.c b/src/lread.c
index a3d5fd7bb8..e811de47c1 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -782,6 +782,12 @@ DEFUN ("read-char", Fread_char, Sread_char, 0, 3, 0,
 
 DEFUN ("read-event", Fread_event, Sread_event, 0, 3, 0,
        doc: /* Read an event object from the input stream.
+
+If you want to read mouse events (for example, to discard an expected
+button up event inside a button down command), call `read-key' which
+can return events via `input-decode-map' such as all mouse events
+generated by `xterm-mouse-mode'.
+
 If the optional argument PROMPT is non-nil, display that as a prompt.
 If PROMPT is nil or the string \"\", the key sequence/events that led
 to the current command is used as the prompt.
-- 
2.20.1