Re: Additional cleanup around xterm-mouse

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

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

On 2020-12-27 9:10 am, Stefan Monnier wrote:
>> This assumes that ESC is the only prefix key in input-decode-map.
>> Is that an okay assumption to make?
> 
> I think so.

Great!  This drastically simplifies the patch.  Updated version 
attached.  I made sure that all the places that were fixed before are 
still fixed.

I still want to do the comment improvements I made through digging into 
read-key-sequence, but will do that in a separate patch.

   -- MJF

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

From c273c73355ba49eb8dee5b27649c2e8f37babe3f Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Tue, 1 Dec 2020 22:23:43 -0800
Subject: [PATCH] Make libraries work with xterm-mouse-mode.

Change read-event calls in libraries expecting mouse events to
read-key when xterm-mouse-mode is enabled.  Add a parameter that
avoids read-key-sequence from discarding mouse button down events so
libraries can still read them.

For backward compatibility purposes, the above logic is contained in
a new internal-only function: read-potential-mouse-event.

* doc/lispref/commands.texi (Reading One Event): Document new
parameter to read-key, add examples for when to call read-key.
* src/lread.c (Fread_event): Update docstring for read-event to
recommend read-key.
* lisp/subr.el (read-key): Add new parameter, fallbacks-disabled to
prevent discarding mouse events normally discarded by
read-key-sequence.
(read-potential-mouse-event): New function that calls read-key or
read-event depending on if xterm-mouse-mode is set.
* lisp/foldout.el (foldout-mouse-swallow-events):
* lisp/isearch.el (isearch-pre-command-hook):
* lisp/mouse-drag.el (mouse-drag-throw, mouse-drag-drag):
* lisp/mouse.el (mouse-drag-secondary):
* lisp/ruler-mode.el (ruler-mode-mouse-grab-any-column)
(ruler-mode-mouse-drag-any-column-iteration):
* lisp/strokes.el (strokes-read-stroke, strokes-read-complex-stroke):
* lisp/textmodes/artist.el (artist-mouse-draw-continously)
(artist-mouse-draw-poly, artist-mouse-draw-2points):
* lisp/vc/ediff-wind.el (ediff-get-window-by-clicking):
* lisp/wid-edit.el (widget-button--check-and-call-button)
(widget-button-click): Call read-potential-mouse-event.
* lisp/wid-edit.el (widget-key-sequence-read-event): Call read-key
with fallbacks-disabled set.  Apply local-function-key-map instead of
function-key-map, as that contains the full terminal translations.
* lisp/vc/ediff.el (ediff-windows): Use display-mouse-p to check if a
mouse is available.
---
 doc/lispref/commands.texi | 12 +++++++++--
 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           | 23 ++++++++++----------
 lisp/subr.el              | 44 ++++++++++++++++++++++++++++++++++++---
 lisp/textmodes/artist.el  |  6 +++---
 lisp/vc/ediff-wind.el     |  5 +++--
 lisp/vc/ediff.el          |  2 +-
 lisp/wid-edit.el          | 14 +++++++------
 src/lread.c               |  6 ++++++
 13 files changed, 91 insertions(+), 35 deletions(-)

diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
index 15d7e4e3a7..7a329a3f47 100644
--- a/doc/lispref/commands.texi
+++ b/doc/lispref/commands.texi
@@ -2696,9 +2696,11 @@ Reading One Event
 @code{read-event}, @code{read-char}, and @code{read-char-exclusive} do
 not perform the translations described in @ref{Translation Keymaps}.
 If you wish to read a single key taking these translations into
-account, use the function @code{read-key}:
+account (for example, to read @ref{Function Keys} in a terminal or
+@ref{Mouse Events} from @code{xterm-mouse-mode}), use the function
+@code{read-key}:
 
-@defun read-key &optional prompt
+@defun read-key &optional prompt disable-fallbacks
 This function reads a single key.  It is intermediate between
 @code{read-key-sequence} and @code{read-event}.  Unlike the former, it
 reads a single key, not a key sequence.  Unlike the latter, it does
@@ -2708,6 +2710,12 @@ Reading One Event
 
 The argument @var{prompt} is either a string to be displayed in the
 echo area as a prompt, or @code{nil}, meaning not to display a prompt.
+
+If argument @var{disable-fallbacks} is non-@code{nil} then the usual
+fallback logic for unbound keys in @code{read-key-sequence} is not
+applied.  This means that mouse button-down and multi-click events
+will not be discarded and @code{local-function-key-map} and
+@code{key-translation-map} will not get applied.
 @end defun
 
 @defun read-char-choice prompt chars &optional inhibit-quit
diff --git a/lisp/foldout.el b/lisp/foldout.el
index 58455c28b1..ca7776f97a 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-potential-mouse-event)))
     (or (eq initial-event-type (event-basic-type event))
 	(error "")))
   event)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 13173a2857..815906fee5 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2968,7 +2968,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-potential-mouse-event)
       (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..ed26e6f072 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-potential-mouse-event)
 		     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-potential-mouse-event)
 		     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..7433f6404f 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-potential-mouse-event))
 		   (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 d97abca9ee..8a81e213cb 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-potential-mouse-event)
          ;; 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-potential-mouse-event)))
         (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 044872068f..0264c9b1e2 100644
--- a/lisp/strokes.el
+++ b/lisp/strokes.el
@@ -756,12 +756,12 @@ strokes-read-stroke
 	      (strokes-fill-current-buffer-with-whitespace))
 	    (when prompt
 	      (message "%s" prompt)
-	      (setq event (read-event))
+	      (setq event (read-potential-mouse-event))
 	      (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-potential-mouse-event)
 				  safe-to-draw-p t))
 		  (while (not (strokes-button-release-event-p event))
 		    (if (strokes-mouse-event-p event)
@@ -776,7 +776,7 @@ strokes-read-stroke
 			    (setq safe-to-draw-p t))
 			  (push (cdr (mouse-pixel-position))
 				pix-locs)))
-		    (setq event (read-event)))))
+		    (setq event (read-potential-mouse-event)))))
 	    ;; protected
 	    ;; clean up strokes buffer and then bury it.
 	    (when (equal (buffer-name) strokes-buffer-name)
@@ -787,16 +787,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-potential-mouse-event))
 	(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-potential-mouse-event)))
 	(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-potential-mouse-event))))
       (setq grid-locs (strokes-renormalize-to-grid (nreverse pix-locs)))
       (strokes-fill-stroke
        (strokes-eliminate-consecutive-redundancies grid-locs)))))
@@ -817,10 +817,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-potential-mouse-event))))
 	(unwind-protect
 	    (track-mouse
-	      (or event (setq event (read-event)))
+	      (or event (setq event (read-potential-mouse-event)))
 	      (while (not (and (strokes-button-press-event-p event)
 			       (eq 'mouse-3
 				   (car (get (car event)
@@ -834,14 +834,15 @@ strokes-read-complex-stroke
 						?\s strokes-character))
 			(push (cdr (mouse-pixel-position))
 			      pix-locs)))
-		  (setq event (read-event)))
+		  (setq event (read-potential-mouse-event)))
 		(push strokes-lift pix-locs)
 		(while (not (strokes-button-press-event-p event))
-		  (setq event (read-event))))
+		  (setq event (read-potential-mouse-event))))
 	      ;; ### 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-potential-mouse-event))))
 	      (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 725722cbee..0414a94f47 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2454,21 +2454,43 @@ memory-limit
 
 (defconst read-key-empty-map (make-sparse-keymap))
 
+(defconst read-key-full-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map [t] 'dummy)
+
+    ;; ESC needs to be unbound so that escape sequences in
+    ;; `input-decode-map' are still processed by `read-key-sequence'.
+    (define-key map [?\e] nil)
+    map))
+
 (defvar read-key-delay 0.01) ;Fast enough for 100Hz repeat rate, hopefully.
 
-(defun read-key (&optional prompt)
+(defun read-key (&optional prompt disable-fallbacks)
   "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 DISABLE-FALLBACKS is non-nil, all
+unbound fallbacks usually done by `read-key-sequence' are
+disabled such as discarding mouse down events.  This is generally
+what you want as `read-key' temporarily removes all bindings.
+Otherwise, only downcasing of the last event is disabled."
   ;; 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
   ;; inherit the input method (at least not if it's based on quail).
   (let ((overriding-terminal-local-map nil)
-	(overriding-local-map read-key-empty-map)
+	(overriding-local-map
+         ;; FIXME: Audit existing usage of `read-key' to see if they
+         ;; should always specify disable-fallbacks to be more in line
+         ;; with `read-event'.
+         (if disable-fallbacks read-key-full-map read-key-empty-map))
         (echo-keystrokes 0)
 	(old-global-map (current-global-map))
         (timer (run-with-idle-timer
@@ -2522,6 +2544,22 @@ read-key
       (message nil)
       (use-global-map old-global-map))))
 
+;; FIXME: Once there's a safe way to transition away from read-event,
+;; this function should be deleted.
+(defun read-potential-mouse-event ()
+    "Read an event that might be a mouse event.
+
+This function exists for backward compatibility in code packaged
+with Emacs.  Do not call it directly in your own packages."
+    ;; `xterm-mouse-mode' events must go through `read-key' as they
+    ;; are decoded via `input-decode-map'.
+    (if xterm-mouse-mode
+        (read-key nil
+                  ;; Normally `read-key' discards all mouse button
+                  ;; down events.  However, we want them here.
+                  t)
+      (read-event)))
+
 (defvar read-passwd-map
   ;; BEWARE: `defconst' would purecopy it, breaking the sharing with
   ;; minibuffer-local-map along the way!
diff --git a/lisp/textmodes/artist.el b/lisp/textmodes/artist.el
index cc2eaf1e4e..1f663aea75 100644
--- a/lisp/textmodes/artist.el
+++ b/lisp/textmodes/artist.el
@@ -5004,7 +5004,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-potential-mouse-event))))
       ;; Cleanup: get rid of any active timer.
       (if timer
           (cancel-timer timer)))
@@ -5212,7 +5212,7 @@ artist-mouse-draw-poly
 
 	;; Read next event (only if we should not stop)
 	(if (not done)
-	    (setq ev (read-event)))))
+	    (setq ev (read-potential-mouse-event)))))
 
     ;; Reverse point-list (last points are cond'ed first)
     (setq point-list (reverse point-list))
@@ -5339,7 +5339,7 @@ artist-mouse-draw-2points
 
 
 	;; Read next event
-	(setq ev (read-event))))
+	(setq ev (read-potential-mouse-event))))
 
     ;; 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 3d90ccb1cb..ecc25137c2 100644
--- a/lisp/vc/ediff-wind.el
+++ b/lisp/vc/ediff-wind.el
@@ -262,11 +262,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-potential-mouse-event))))
       (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
+    (read-potential-mouse-event) ; discard event
     (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 8250316bcc..e045c57269 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1104,7 +1104,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-potential-mouse-event))
 		        (when (and mouse-1 (mouse-movement-p event))
 			  (push event unread-command-events)
 			  (setq event oevent)
@@ -1169,7 +1169,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-potential-mouse-event))))
 	    (when command
 	      (call-interactively command)))))
     (message "You clicked somewhere weird.")))
@@ -3487,14 +3487,16 @@ 'key-sequence
   :help-echo "C-q: insert KEY, EVENT, or CODE; RET: enter value"
   :tag "Key sequence")
 
+;; FIXME: Consider combining this with help--read-key-sequence which
+;; can also read double and triple mouse events.
 (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))))
   (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 nil t)))
+	(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/lread.c b/src/lread.c
index 3ef874039a..eb19cf7b70 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 non-character events consider calling `read-key'
+instead.  `read-key' can decode events via `input-decode-map' such as
+<f7>, <right>, or mouse events generated by `xterm-mouse-mode' when on
+a terminal.
+
 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