all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Additional cleanup around xterm-mouse
@ 2020-11-15  8:49 Jared Finder via Emacs development discussions.
  2020-11-15 18:11 ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-11-15  8:49 UTC (permalink / raw)
  To: emacs-devel

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

Two more patches doing misc cleanup around xterm-mouse.

The first patch is very straightforward and should be trivial to review 
and merge.

I have a question about the right way to proceed with the second patch.  
Redirecting to read-key seems like the right solution here as it is 
"read-event but also take input-decode-map into account".  I needed to 
make a change to read-key's implementation to make it also return button 
down events and I saw two ways to do that.  I would like advice on which 
way is preferred.  In the patch you can see the two options commented in 
subr.el with Option 1 enabled by default.

Let me know what you think.

   -- MJF

[-- Attachment #2: 0001-Migrate-usage-of-GPM_CLICK_EVENT-to-MOUSE_CLICK_EVEN.patch --]
[-- Type: text/x-diff, Size: 3555 bytes --]

From d9ac1bb6cdb270c1ab3b3b660837590b535a68a5 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 14 Nov 2020 23:44:26 -0800
Subject: [PATCH 1/2] Migrate usage of GPM_CLICK_EVENT to MOUSE_CLICK_EVENT.

* src/termhooks.h (enum event_kind):
* src/term.c (term_mouse_click, handle_one_term_event):
* src/keyboard.c (discard_mouse_events, make_lispy_event): Migrate
usage of GPM_CLICK_EVENT to MOUSE_CLICK_EVENT.
---
 src/keyboard.c  | 12 +-----------
 src/term.c      |  8 ++++----
 src/termhooks.h |  4 ----
 3 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index 49a0a8bd23..45e9abc229 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3736,9 +3736,6 @@ discard_mouse_events (void)
       if (sp->kind == MOUSE_CLICK_EVENT
 	  || sp->kind == WHEEL_EVENT
           || sp->kind == HORIZ_WHEEL_EVENT
-#ifdef HAVE_GPM
-	  || sp->kind == GPM_CLICK_EVENT
-#endif
 	  || sp->kind == SCROLL_BAR_CLICK_EVENT
 	  || sp->kind == HORIZONTAL_SCROLL_BAR_CLICK_EVENT)
 	{
@@ -5542,9 +5539,6 @@ make_lispy_event (struct input_event *event)
       /* A mouse click.  Figure out where it is, decide whether it's
          a press, click or drag, and build the appropriate structure.  */
     case MOUSE_CLICK_EVENT:
-#ifdef HAVE_GPM
-    case GPM_CLICK_EVENT:
-#endif
 #ifndef USE_TOOLKIT_SCROLL_BARS
     case SCROLL_BAR_CLICK_EVENT:
     case HORIZONTAL_SCROLL_BAR_CLICK_EVENT:
@@ -5559,11 +5553,7 @@ make_lispy_event (struct input_event *event)
 	position = Qnil;
 
 	/* Build the position as appropriate for this mouse click.  */
-	if (event->kind == MOUSE_CLICK_EVENT
-#ifdef HAVE_GPM
-	    || event->kind == GPM_CLICK_EVENT
-#endif
-	    )
+	if (event->kind == MOUSE_CLICK_EVENT)
 	  {
 	    struct frame *f = XFRAME (event->frame_or_window);
 	    int row, column;
diff --git a/src/term.c b/src/term.c
index a0738594bf..fee3b55575 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2481,7 +2481,7 @@ term_mouse_click (struct input_event *result, Gpm_Event *event,
 {
   int i, j;
 
-  result->kind = GPM_CLICK_EVENT;
+  result->kind = MOUSE_CLICK_EVENT;
   for (i = 0, j = GPM_B_LEFT; i < 3; i++, j >>= 1 )
     {
       if (event->buttons & j) {
@@ -2567,11 +2567,11 @@ handle_one_term_event (struct tty_display_info *tty, Gpm_Event *event)
     {
       f->mouse_moved = 0;
       term_mouse_click (&ie, event, f);
-      /* eassert (ie.kind == GPM_CLICK_EVENT); */
+      /* eassert (ie.kind == MOUSE_CLICK_EVENT); */
       if (tty_handle_tab_bar_click (f, event->x, event->y,
                                     (ie.modifiers & down_modifier) != 0, &ie))
         {
-          /* eassert (ie.kind == GPM_CLICK_EVENT
+          /* eassert (ie.kind == MOUSE_CLICK_EVENT
            *          || ie.kind == TAB_BAR_EVENT); */
           /* tty_handle_tab_bar_click stores 2 events in the event
              queue, so we are done here.  */
@@ -2581,7 +2581,7 @@ handle_one_term_event (struct tty_display_info *tty, Gpm_Event *event)
           count += 2;
           return count;
         }
-      /* eassert (ie.kind == GPM_CLICK_EVENT); */
+      /* eassert (ie.kind == MOUSE_CLICK_EVENT); */
       kbd_buffer_store_event (&ie);
       count++;
     }
diff --git a/src/termhooks.h b/src/termhooks.h
index 6ab06ceff9..44ab14225f 100644
--- a/src/termhooks.h
+++ b/src/termhooks.h
@@ -220,10 +220,6 @@ #define EMACS_TERMHOOKS_H
      save yourself before shutdown. */
   SAVE_SESSION_EVENT
 
-#ifdef HAVE_GPM
-  , GPM_CLICK_EVENT
-#endif
-
 #ifdef HAVE_DBUS
   , DBUS_EVENT
 #endif
-- 
2.20.1


[-- Attachment #3: 0002-Options-for-making-libraries-work-with-xt-mouse.patch --]
[-- Type: text/x-diff, Size: 14729 bytes --]

From 8a2cc39ab17a1e2e6f8ea0294b538a8971146a68 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 14 Nov 2020 21:42:34 -0800
Subject: [PATCH 2/2] Options for making libraries work with xt-mouse.

Libraries changed by making them call `read-key' instead of
`read-event'.
---
 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             | 23 +++++++++++++++++++++++
 lisp/textmodes/artist.el |  6 +++---
 lisp/vc/ediff-wind.el    |  4 ++--
 lisp/vc/ediff.el         |  2 +-
 lisp/wid-edit.el         |  9 ++++++---
 src/keyboard.c           | 10 +++++++++-
 src/lread.c              |  6 ++++++
 13 files changed, 68 insertions(+), 28 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..788930c105 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))
 	      (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)
 				  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)))))
 	    ;; 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))
 	(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)))
 	(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))))
       (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))))
 	(unwind-protect
 	    (track-mouse
-	      (or event (setq event (read-event)))
+	      (or event (setq event (read-key)))
 	      (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)))
 		(push strokes-lift pix-locs)
 		(while (not (strokes-button-press-event-p event))
-		  (setq event (read-event))))
+		  (setq event (read-key))))
 	      ;; ### 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))))
 	      (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..36381dc4e6 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2446,6 +2446,9 @@ read-key
 	(overriding-local-map read-key-empty-map)
         (echo-keystrokes 0)
 	(old-global-map (current-global-map))
+        ;; Option 1: New variable that prevents mouse events from
+        ;; being transformed or discarded.
+        (inhibit--unbound-mouse-fallback t)
         (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.
@@ -2481,6 +2484,26 @@ read-key
 	       ;; This hack avoids evaluating the :filter (Bug#9922).
 	       (or (cdr (assq 'tool-bar global-map))
 		   (lookup-key global-map [tool-bar])))
+             ;; Option 2: Bind all the mouse events to prevent
+             ;; dropping / transformation.
+             ;;
+             ;; Note: this is not an exhaustive list.  To fully work,
+             ;; this should bind all possible prefix combinations (the
+             ;; power set of A-, C-, H-, S-, s-) for all possible
+             ;; mouse events.  For example, this currently does not
+             ;; bind C-S-down-mouse-1.
+             ;; (define-key map [down-mouse-1] 'ignore)
+             ;; (define-key map [C-down-mouse-1] 'ignore)
+             ;; (define-key map [M-down-mouse-1] 'ignore)
+             ;; (define-key map [C-M-down-mouse-1] 'ignore)
+             ;; (define-key map [down-mouse-2] 'ignore)
+             ;; (define-key map [C-down-mouse-2] 'ignore)
+             ;; (define-key map [M-down-mouse-2] 'ignore)
+             ;; (define-key map [C-M-down-mouse-3] 'ignore)
+             ;; (define-key map [down-mouse-3] 'ignore)
+             ;; (define-key map [C-down-mouse-3] 'ignore)
+             ;; (define-key map [M-down-mouse-3] 'ignore)
+             ;; (define-key map [C-M-down-mouse-3] 'ignore)
              map))
           (let* ((keys
                   (catch 'read-key (read-key-sequence-vector prompt nil t)))
diff --git a/lisp/textmodes/artist.el b/lisp/textmodes/artist.el
index 5ce9a90ea6..5155d4100c 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)))))
 
     ;; 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..9843669e78 100644
--- a/lisp/vc/ediff-wind.el
+++ b/lisp/vc/ediff-wind.el
@@ -269,11 +269,11 @@ 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
+    (read-key) ; 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 4e2cf7416d..bdcf8255a1 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,6 +1153,9 @@ widget-button-click
 	    (when up
 	      ;; Don't execute up events twice.
 	      (while (not (widget-button-release-event-p event))
+                ;; FIXME: This should probably be read-key to get
+                ;; mouse events through xterm-mouse-mode, but it is
+                ;; unclear how to trigger this code path normally.
 		(setq event (read-event))))
 	    (when command
 	      (call-interactively command)))))
@@ -3465,9 +3468,9 @@ '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: "))))
   (let ((ev2 (and (memq 'down (event-modifiers ev))
-		  (read-event)))
+		  (read-key)))
 	(tr (and (keymapp function-key-map)
 		 (lookup-key function-key-map (vector ev)))))
     (when (and (integerp 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


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-15  8:49 Jared Finder via Emacs development discussions.
@ 2020-11-15 18:11 ` Eli Zaretskii
  0 siblings, 0 replies; 41+ messages in thread
From: Eli Zaretskii @ 2020-11-15 18:11 UTC (permalink / raw)
  To: Jared Finder; +Cc: emacs-devel

> Date: Sun, 15 Nov 2020 00:49:03 -0800
> From: Jared Finder via "Emacs development discussions." <emacs-devel@gnu.org>
> 
> The first patch is very straightforward and should be trivial to review 
> and merge.

Agreed.

> I have a question about the right way to proceed with the second patch.  
> Redirecting to read-key seems like the right solution here as it is 
> "read-event but also take input-decode-map into account".  I needed to 
> make a change to read-key's implementation to make it also return button 
> down events and I saw two ways to do that.  I would like advice on which 
> way is preferred.  In the patch you can see the two options commented in 
> subr.el with Option 1 enabled by default.

Ouch, this is scary.  We have a lot of gray hair from replacing input
functions by seemingly-similar other input functions.  And on top of
that, you need changes to read-key.  These changes will affect every
"native" mouse subsystem out there, with the benefit being a single
niche mouse subsystem that is an emulator.  This sounds like not the
best way, as the risk will be shared by many important configurations
and the benefits by only one not very important one.

Can you think about a way of doing this that will affect only
xterm-mouse?  I'm okay with, for example, replacing read-event in
those cases with some new function that will call a special
xterm-mouse API when xterm-mouse is in effect, and will call
read-event otherwise.  Is something like this feasible?

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
@ 2020-11-16  6:29 Jared Finder via Emacs development discussions.
  2020-11-16 17:30 ` Jared Finder via Emacs development discussions.
  2020-11-18 17:40 ` Eli Zaretskii
  0 siblings, 2 replies; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-11-16  6:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On 2020-11-15 10:11 am, Eli Zaretskii wrote:
>> Date: Sun, 15 Nov 2020 00:49:03 -0800
>> From: Jared Finder via "Emacs development discussions." 
>> <emacs-devel@gnu.org>
>> 
>> The first patch is very straightforward and should be trivial to 
>> review
>> and merge.
> 
> Agreed.

Great. It's completely independent of the other change, feel free to 
merge at any time.

>> I have a question about the right way to proceed with the second 
>> patch.
>> [...]
> 
> Ouch, this is scary.  We have a lot of gray hair from replacing input
> functions by seemingly-similar other input functions.  And on top of
> that, you need changes to read-key.  These changes will affect every
> "native" mouse subsystem out there, with the benefit being a single
> niche mouse subsystem that is an emulator.  This sounds like not the
> best way, as the risk will be shared by many important configurations
> and the benefits by only one not very important one.
> 
> Can you think about a way of doing this that will affect only
> xterm-mouse?  I'm okay with, for example, replacing read-event in
> those cases with some new function that will call a special
> xterm-mouse API when xterm-mouse is in effect, and will call
> read-event otherwise.  Is something like this feasible?

I was a little nervous about changing read-key's default behavior too.  
Happy to explore other options. :)

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.)  However, it can avoid the rest of the 
complexity of read-key-sequence.  I'm imagining something like this 
(untested code follows, just wanted to give a flavor of it):

(defun read-decoded-key ()  ; I'd love a better name here.
   ;; Start of code like read-key's code.
   (let ((keys '())
         (timer (run-with-idle-timer
                 read-key-delay t
                 (lambda ()
                   (unless (null keys)
                     (throw 'read-key nil))))))
     (unwind-protect
         (while t (push (read-event) keys))
       (cancel-timer timer))

     ;; Start of new stuff: Apply transformations from input-decode-map.
     (do-stuff)

     (vconcat (nreverse keys))))

As you can see, this avoids all the complexity around managing the 
different keymaps that read-key currently has since it calls 
read-key-sequence.


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>).

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-16  6:29 Jared Finder via Emacs development discussions.
@ 2020-11-16 17:30 ` Jared Finder via Emacs development discussions.
  2020-11-18 17:40 ` Eli Zaretskii
  1 sibling, 0 replies; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-11-16 17:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel



On 2020-11-15 10:29 pm, Jared Finder wrote:
> On 2020-11-15 10:11 am, Eli Zaretskii wrote:
>>> Date: Sun, 15 Nov 2020 00:49:03 -0800
>>> From: Jared Finder via "Emacs development discussions." 
>>> <emacs-devel@gnu.org>
>>> 
>>> The first patch is very straightforward and should be trivial to 
>>> review
>>> and merge.
>> 
>> Agreed.
> 
> Great. It's completely independent of the other change, feel free to
> merge at any time.
> 
>>> I have a question about the right way to proceed with the second 
>>> patch.
>>> [...]
>> 
>> Ouch, this is scary.  We have a lot of gray hair from replacing input
>> functions by seemingly-similar other input functions.  And on top of
>> that, you need changes to read-key.  These changes will affect every
>> "native" mouse subsystem out there, with the benefit being a single
>> niche mouse subsystem that is an emulator.  This sounds like not the
>> best way, as the risk will be shared by many important configurations
>> and the benefits by only one not very important one.
>> 
>> Can you think about a way of doing this that will affect only
>> xterm-mouse?  I'm okay with, for example, replacing read-event in
>> those cases with some new function that will call a special
>> xterm-mouse API when xterm-mouse is in effect, and will call
>> read-event otherwise.  Is something like this feasible?
> 
> I was a little nervous about changing read-key's default behavior too.
>  Happy to explore other options. :)
> 
> 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.)  However, it can avoid the rest of
> the complexity of read-key-sequence.  I'm imagining something like
> this (untested code follows, just wanted to give a flavor of it):
> 
> (defun read-decoded-key ()  ; I'd love a better name here.
>   ;; Start of code like read-key's code.
>   (let ((keys '())
>         (timer (run-with-idle-timer
>                 read-key-delay t
>                 (lambda ()
>                   (unless (null keys)
>                     (throw 'read-key nil))))))
>     (unwind-protect
>         (while t (push (read-event) keys))
>       (cancel-timer timer))
> 
>     ;; Start of new stuff: Apply transformations from input-decode-map.
>     (do-stuff)
> 
>     (vconcat (nreverse keys))))
> 
> As you can see, this avoids all the complexity around managing the
> different keymaps that read-key currently has since it calls
> read-key-sequence.
> 
> 
> 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 meant to say here "This **works** w/o changes to read key, but with a 
behavior change." Sorry for any confusion caused.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-16  6:29 Jared Finder via Emacs development discussions.
  2020-11-16 17:30 ` Jared Finder via Emacs development discussions.
@ 2020-11-18 17:40 ` Eli Zaretskii
  2020-11-19  8:03   ` Jared Finder via Emacs development discussions.
  2020-11-21  8:23   ` Eli Zaretskii
  1 sibling, 2 replies; 41+ messages in thread
From: Eli Zaretskii @ 2020-11-18 17:40 UTC (permalink / raw)
  To: Jared Finder; +Cc: emacs-devel

> Date: Sun, 15 Nov 2020 22:29:20 -0800
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> >> The first patch is very straightforward and should be trivial to 
> >> review
> >> and merge.
> > 
> > Agreed.
> 
> Great. It's completely independent of the other change, feel free to 
> merge at any time.

Soon.

> > Can you think about a way of doing this that will affect only
> > xterm-mouse?  I'm okay with, for example, replacing read-event in
> > those cases with some new function that will call a special
> > xterm-mouse API when xterm-mouse is in effect, and will call
> > read-event otherwise.  Is something like this feasible?
> 
> I was a little nervous about changing read-key's default behavior too.  
> Happy to explore other options. :)
> 
> 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?

> However, it can avoid the rest of the complexity of
> read-key-sequence.  I'm imagining something like this (untested code
> follows, just wanted to give a flavor of it):
> 
> (defun read-decoded-key ()  ; I'd love a better name here.
>    ;; Start of code like read-key's code.
>    (let ((keys '())
>          (timer (run-with-idle-timer
>                  read-key-delay t
>                  (lambda ()
>                    (unless (null keys)
>                      (throw 'read-key nil))))))
>      (unwind-protect
>          (while t (push (read-event) keys))
>        (cancel-timer timer))
> 
>      ;; Start of new stuff: Apply transformations from input-decode-map.
>      (do-stuff)
> 
>      (vconcat (nreverse keys))))

Doesn't look too bad, but I don't think I have a clear idea of how you
thought to use this in those places where you need xt-mouse to be
supported?

> 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?

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-18 17:40 ` Eli Zaretskii
@ 2020-11-19  8:03   ` Jared Finder via Emacs development discussions.
  2020-11-21  9:31     ` Eli Zaretskii
  2020-11-21 17:00     ` Stefan Monnier
  2020-11-21  8:23   ` Eli Zaretskii
  1 sibling, 2 replies; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-11-19  8:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- 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


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-18 17:40 ` Eli Zaretskii
  2020-11-19  8:03   ` Jared Finder via Emacs development discussions.
@ 2020-11-21  8:23   ` Eli Zaretskii
  1 sibling, 0 replies; 41+ messages in thread
From: Eli Zaretskii @ 2020-11-21  8:23 UTC (permalink / raw)
  To: jared; +Cc: emacs-devel

> Date: Wed, 18 Nov 2020 19:40:42 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > Date: Sun, 15 Nov 2020 22:29:20 -0800
> > From: Jared Finder <jared@finder.org>
> > Cc: emacs-devel@gnu.org
> > 
> > >> The first patch is very straightforward and should be trivial to 
> > >> review
> > >> and merge.
> > > 
> > > Agreed.
> > 
> > Great. It's completely independent of the other change, feel free to 
> > merge at any time.
> 
> Soon.

Done.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-19  8:03   ` Jared Finder via Emacs development discussions.
@ 2020-11-21  9:31     ` Eli Zaretskii
  2020-11-22 23:56       ` Jared Finder via Emacs development discussions.
  2020-11-21 17:00     ` Stefan Monnier
  1 sibling, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2020-11-21  9:31 UTC (permalink / raw)
  To: Jared Finder; +Cc: emacs-devel

> Date: Thu, 19 Nov 2020 00:03:30 -0800
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> > 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.)

OK, but that means widget-key-sequence-read-event has a bug that needs
to be fixed regardless.

So I'm still asking why can't we do something like

  (if xterm-mouse-mode
      (read-key)
    (read-event))

in all the affected places that currently call read-event?

> 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.

The problem is that we get lots of unintended functionalities even
with the new parameters.  read-key-sequence consults many internal
variables that read-event doesn't.  So I'm still wary of making such a
significant change, even though we are talking about a small number of
relatively minor feature (with the sole exception of Ediff, perhaps).
If the above less-elegant change which will only affect usage under
xterm-mouse-mode is feasible, I'd prefer to go that way instead.

>    (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)))))

Let's fix this part separately.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-19  8:03   ` Jared Finder via Emacs development discussions.
  2020-11-21  9:31     ` Eli Zaretskii
@ 2020-11-21 17:00     ` Stefan Monnier
  1 sibling, 0 replies; 41+ messages in thread
From: Stefan Monnier @ 2020-11-21 17:00 UTC (permalink / raw)
  To: Jared Finder via Emacs development discussions.
  Cc: Eli Zaretskii, Jared Finder

> 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.

It's wickedly difficult to apply those maps correctly.
The only sane way to do that it to reuse the `read-key-sequence` code,
which is why I wrote `read-key` in the first place.

I think the only sane way forward is to work on top of
`read-key-sequence`, and more specifically to start using `read-key`
much more extensively and when bumping into problems to try and fix them
in `read-key` rather than retreating to some ad-hoc hack.


        Stefan




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-21  9:31     ` Eli Zaretskii
@ 2020-11-22 23:56       ` Jared Finder via Emacs development discussions.
  2020-11-28 16:36         ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-11-22 23:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On 2020-11-21 1:31 am, Eli Zaretskii wrote:
>> Date: Thu, 19 Nov 2020 00:03:30 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> > 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.)
> 
> OK, but that means widget-key-sequence-read-event has a bug that needs
> to be fixed regardless.
> 
> So I'm still asking why can't we do something like
> 
>   (if xterm-mouse-mode
>       (read-key)
>     (read-event))
> 
> in all the affected places that currently call read-event?

This can be done in all places except widget-key-sequence-read-event 
(see below for an explanation).  Though it feels like a bad solution as 
the info pages clearly state right now that if you want to read 
translated events, you should use read-key, not read-event.  Mouse 
events always could be translated because of xterm-mouse-mode, so isn't 
this just a bug?

I completely agree with the goal to make sure to preserve backward 
compatibility issues, but I think a better approach would be to add the 
additional functionality to read-key to make it a safe replacement for 
read-event.  (This seems aligned with how I read Stefan's recent comment 
on this thread as well.)


By the way, the info text I'm referring to is the following excerpt from 
(elisp)Top > Command Loop > Reading Input > Reading One Event.

    We emphasize that, unlike ‘read-key-sequence’, the functions
‘read-event’, ‘read-char’, and ‘read-char-exclusive’ do not perform the
translations described in Translation Keymaps.  If you wish to
read a single key taking these translations into account, use the
function ‘read-key’:

> So I'm still wary of making such a
> significant change, even though we are talking about a small number of
> relatively minor feature (with the sole exception of Ediff, perhaps).

Note: the wid-edit.el change is also pretty major and where I first 
noticed this.  Without some fix there, interaction with widgets in 
Customize buffers with an xterm mouse will cause infinite loops that 
only C-g can escape.

>>    (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)))))
> 
> Let's fix this part separately.

The underlying issue with function keys is the same as the xterm-mouse 
issue (failure to apply input-decode-map), so the solutions may be 
intertwined.  Terminal function keys and xterm mouse events both get 
decoded through input-decode-map.

Note that because function keys are also affected, for this function the 
check whether to use read-key or read-event would need to be something 
like:

(if (eq window-system nil) ; This is correct on Linux and macOS, not 
sure
                            ; if accurate for other platforms
     (read-key)
   (read-event))

Without the change from read-event to read-key throughout this function, 
'ev' will never contain an event that would be translated by 
local-function-key-map or function-key-map from what I've seen.  It may 
be a better path to convert this function to just call (read-key 
"Prompt" t) like the rest of the modifications.  If this is done, then 
local-function-key-map is already applied.  I realize that is a behavior 
change, but given how fragile reconstructing the way translation maps 
already is, I think it is worth considering.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-22 23:56       ` Jared Finder via Emacs development discussions.
@ 2020-11-28 16:36         ` Eli Zaretskii
  2020-12-01  7:36           ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2020-11-28 16:36 UTC (permalink / raw)
  To: Jared Finder; +Cc: emacs-devel

> Date: Sun, 22 Nov 2020 15:56:50 -0800
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> > So I'm still asking why can't we do something like
> > 
> >   (if xterm-mouse-mode
> >       (read-key)
> >     (read-event))
> > 
> > in all the affected places that currently call read-event?
> 
> This can be done in all places except widget-key-sequence-read-event 
> (see below for an explanation).  Though it feels like a bad solution as 
> the info pages clearly state right now that if you want to read 
> translated events, you should use read-key, not read-event.  Mouse 
> events always could be translated because of xterm-mouse-mode, so isn't 
> this just a bug?

Maybe it is a bug, but somehow we've lived with it for a very long
time.

Btw, my main worry is not that we will begin to use translated events
where we formerly didn't, it's that we will use an input function
whose code is very different, and thus some aspects of its behavior in
various situations could very well be subtly different, thus breaking
some use cases.  Try to compare the two functions and figure out
whether they behave the same or not, and in what cases.  The code is
so different that even understanding how they both receive the same
events is non-trivial.  How does one assess risk from this change in
cases like this one?

> By the way, the info text I'm referring to is the following excerpt from 
> (elisp)Top > Command Loop > Reading Input > Reading One Event.
> 
>     We emphasize that, unlike ‘read-key-sequence’, the functions
> ‘read-event’, ‘read-char’, and ‘read-char-exclusive’ do not perform the
> translations described in Translation Keymaps.  If you wish to
> read a single key taking these translations into account, use the
> function ‘read-key’:

That's okay.  We frequently tell "do as I say, not as I do", and
there's nothing wrong with that: the ELisp manual is primarily a text
for Lisp programmers, not a faithful description of the Emacs design
and implementation (although it does include some of the latter).

> Note: the wid-edit.el change is also pretty major and where I first 
> noticed this.  Without some fix there, interaction with widgets in 
> Customize buffers with an xterm mouse will cause infinite loops that 
> only C-g can escape.

I'm okay with making such a change in wid-edit.el, mainly because that
case is unlikely to hit the problems I worry about, and even if it
does, it's just one package.

> Note that because function keys are also affected, for this function the 
> check whether to use read-key or read-event would need to be something 
> like:
> 
> (if (eq window-system nil) ; This is correct on Linux and macOS, not 
> sure
>                             ; if accurate for other platforms
>      (read-key)
>    (read-event))

That won't be necessary: in wid-edit.el, just replace read-event with
read-key, and let's see how much this breaks.

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-11-28 16:36         ` Eli Zaretskii
@ 2020-12-01  7:36           ` Jared Finder via Emacs development discussions.
  2020-12-01 15:21             ` Stefan Monnier
  2020-12-01 18:23             ` Eli Zaretskii
  0 siblings, 2 replies; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-01  7:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On 2020-11-28 8:36 am, Eli Zaretskii wrote:
>> Date: Sun, 22 Nov 2020 15:56:50 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> > So I'm still asking why can't we do something like
>> >
>> >   (if xterm-mouse-mode
>> >       (read-key)
>> >     (read-event))
>> >
>> > in all the affected places that currently call read-event?
>> 
>> This can be done in all places except widget-key-sequence-read-event
>> (see below for an explanation).  Though it feels like a bad solution 
>> as
>> the info pages clearly state right now that if you want to read
>> translated events, you should use read-key, not read-event.  Mouse
>> events always could be translated because of xterm-mouse-mode, so 
>> isn't
>> this just a bug?
> 
> Maybe it is a bug, but somehow we've lived with it for a very long
> time.
> 
> Btw, my main worry is not that we will begin to use translated events
> where we formerly didn't, it's that we will use an input function
> whose code is very different, and thus some aspects of its behavior in
> various situations could very well be subtly different, thus breaking
> some use cases.  Try to compare the two functions and figure out
> whether they behave the same or not, and in what cases.  The code is
> so different that even understanding how they both receive the same
> events is non-trivial.  How does one assess risk from this change in
> cases like this one?

This makes sense to me.  I agree that read-key is likely to currently 
behave different in some situations from read-event.  Are these 
differences bugs that should be fixed or are they now features that can 
not be changed?  I ask because I feel bad about adding yet another 
difference between xterm-mouse and "native" mouse support.

I'm about 33% through analyzing read-key and have already found the 
following differences:

* read-key resets this-command-keys, read-event appends to it.
* read-key does not return switch-frame events as they happen, 
read-event does.

Are these bugs to fix?  If so perhaps an exhaustive analysis with good 
unit test coverage could address the risk here and provide a path to 
removing such a difference in the future.  I agree that in the short 
term, adding (if xterm-mouse-mode (read-key) (read-event)) is the safest 
path.

>> Note that because function keys are also affected, for this function 
>> the
>> check whether to use read-key or read-event would need to be something
>> like:
>> 
>> (if (eq window-system nil) ; This is correct on Linux and macOS, not
>> sure
>>                             ; if accurate for other platforms
>>      (read-key)
>>    (read-event))
> 
> That won't be necessary: in wid-edit.el, just replace read-event with
> read-key, and let's see how much this breaks.

Sounds good. I will modify my patch soon.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-01  7:36           ` Jared Finder via Emacs development discussions.
@ 2020-12-01 15:21             ` Stefan Monnier
  2020-12-01 18:23             ` Eli Zaretskii
  1 sibling, 0 replies; 41+ messages in thread
From: Stefan Monnier @ 2020-12-01 15:21 UTC (permalink / raw)
  To: Jared Finder via Emacs development discussions.
  Cc: Eli Zaretskii, Jared Finder

> I'm about 33% through analyzing read-key and have already found the
> following differences:
>
> * read-key resets this-command-keys, read-event appends to it.

I think this is a misfeature of `read-key`.  I believe the better fix is
to introduce a new function which is like `read-key-sequence` but with
fewer side effects and on top of which we would re-implement `read-key`
and `read-key-sequence`.

By "fewer side effects", I mean that it should return a set of data:
- the key-sequence read.
- the corresponding list of undecoded input-events (instead of
  modifying this-command-keys).
- the command that was found in the keymaps and which made the function
  decide that this key sequence was complete (instead of setting
  read_key_sequence_cmd).
- maybe more...

> * read-key does not return switch-frame events as they happen,
>   read-event does.

I think this is what we want.

BTW, related to that, there is `read-char` which I believe can (almost?)
always be replaced with `read-key`.


        Stefan




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-01  7:36           ` Jared Finder via Emacs development discussions.
  2020-12-01 15:21             ` Stefan Monnier
@ 2020-12-01 18:23             ` Eli Zaretskii
  2020-12-02  6:45               ` Jared Finder via Emacs development discussions.
  2020-12-14  0:36               ` Jared Finder via Emacs development discussions.
  1 sibling, 2 replies; 41+ messages in thread
From: Eli Zaretskii @ 2020-12-01 18:23 UTC (permalink / raw)
  To: Jared Finder; +Cc: emacs-devel

> Date: Mon, 30 Nov 2020 23:36:23 -0800
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> This makes sense to me.  I agree that read-key is likely to currently 
> behave different in some situations from read-event.  Are these 
> differences bugs that should be fixed or are they now features that can 
> not be changed?

I don't know.  I think we need to discuss each difference separately.

> I'm about 33% through analyzing read-key and have already found the 
> following differences:
> 
> * read-key resets this-command-keys, read-event appends to it.
> * read-key does not return switch-frame events as they happen, 
> read-event does.

Thank you for doing this.  We should at least document these changes
in some place, and probably also discuss at least some of them.

> Sounds good. I will modify my patch soon.

Great, thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-01 18:23             ` Eli Zaretskii
@ 2020-12-02  6:45               ` Jared Finder via Emacs development discussions.
  2020-12-02 16:53                 ` Stefan Monnier
  2020-12-14  0:36               ` Jared Finder via Emacs development discussions.
  1 sibling, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-02  6:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

On 2020-12-01 10:23 am, Eli Zaretskii wrote:
>> Date: Mon, 30 Nov 2020 23:36:23 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> Sounds good. I will modify my patch soon.
> 
> Great, thanks.

Updated patch attached.

   -- 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: 18655 bytes --]

From b4718077ee7c969a86fb4e6c0617b3cf1bc58263 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.

* src/lread.c (Fread_event): Update docstring for read-event to
recommend read-key.
* lisp/subr.el (read-key): Add new parameter, all-mouse-events to
prevent discarding mouse events normally discarded by
read-key-sequence.
* doc/lispref/commands.texi (Reading One Event): Document new
parameter.
* src/keyboard.c (read_key_sequence): Implement new behavior for
read-key.
(inhibit--unbound-mouse-fallback): New variable to enable new
behavior.
(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 it.
* lisp/wid-edit.el (widget-key-sequence-read-event): Call read-key,
delete manual application of function-key-map.
* lisp/vc/ediff.el (ediff-windows): Use display-mouse-p to check if a
mouse is available.
---
 doc/lispref/commands.texi |  6 +++++-
 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              | 23 +++++++++++++++++++++--
 lisp/textmodes/artist.el  |  6 +++---
 lisp/vc/ediff-wind.el     |  5 +++--
 lisp/vc/ediff.el          |  2 +-
 lisp/wid-edit.el          | 17 +++++------------
 src/keyboard.c            | 10 +++++++++-
 src/lread.c               |  6 ++++++
 14 files changed, 72 insertions(+), 40 deletions(-)

diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
index ebfda01671..29d56a434c 100644
--- a/doc/lispref/commands.texi
+++ b/doc/lispref/commands.texi
@@ -2691,7 +2691,7 @@ Reading One Event
 If you wish to read a single key taking these translations into
 account, use the function @code{read-key}:
 
-@defun read-key &optional prompt
+@defun read-key &optional prompt all-mouse-events
 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
@@ -2701,6 +2701,10 @@ 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{all-mouse-events} is non-@code{nil} then button-down
+and multi-click events will get returned.  Otherwise, these are
+ignored as in @code{read-key-sequence}.
 @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 a0aa250c4b..0364baf826 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2974,7 +2974,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 82e6178da1..6815e76ccb 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 11bc07a29c..a007568470 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 0b92a4f9b5..d4a7282529 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2431,13 +2431,20 @@ 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)
   "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 +2453,7 @@ 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)
         (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.
@@ -2497,6 +2505,17 @@ 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."
+    (if xterm-mouse-mode
+        (read-key nil 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 90e8d360c1..df9335ea42 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-potential-mouse-event))))
       ;; 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-potential-mouse-event)))))
 
     ;; 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-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 c68dc71884..ecd69d54ea 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..55c0bf5526 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.")))
@@ -3490,25 +3490,18 @@ '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))))
   (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))))
     (when (and (integerp ev)
 	       (or (and (<= ?0 ev) (< ev (+ ?0 (min 10 read-quoted-char-radix))))
 		   (and (<= ?a (downcase ev))
 			(< (downcase ev) (+ ?a -10 (min 36 read-quoted-char-radix))))))
       (setq unread-command-events (cons ev unread-command-events)
-	    ev (read-quoted-char (format "Enter code (radix %d)" read-quoted-char-radix))
-	    tr nil)
+	    ev (read-quoted-char (format "Enter code (radix %d)" read-quoted-char-radix)))
       (if (and (integerp ev) (not (characterp ev)))
 	  (insert (char-to-string ev))))  ;; throw invalid char error
     (setq ev (key-description (list ev)))
-    (when (arrayp tr)
-      (setq tr (key-description (list (aref tr 0))))
-      (if (y-or-n-p (format "Key %s is translated to %s -- use %s? " ev tr tr))
-	  (setq ev tr ev2 nil)))
     (insert (if (= (char-before) ?\s)  "" " ") ev " ")
     (if ev2
 	(insert (key-description (list ev2)) " "))))
diff --git a/src/keyboard.c b/src/keyboard.c
index 49261fcc3e..9395448c52 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


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-02  6:45               ` Jared Finder via Emacs development discussions.
@ 2020-12-02 16:53                 ` Stefan Monnier
  2020-12-03  5:46                   ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Stefan Monnier @ 2020-12-02 16:53 UTC (permalink / raw)
  To: Jared Finder via Emacs development discussions.
  Cc: Eli Zaretskii, Jared Finder

> Subject: [PATCH] Make libraries work with xterm-mouse-mode.

Could you explain (at least in the code, and ideally here as well) why
we need this new `all-mouse-events` behavior?

`function-key-map` has very similar effects (and to a large extent, the
downgrading of mouse-down events controlled by `all-mouse-events` could
(I'd even say should) be implemented in `function-key-map` rather than
via the current ad-hoc code in read-key-sequence), so I'm not very
comfortable with treating these mouse-event-rewritings differently from
other rewritings.

The other question is why you made `all-mouse-events` an argument to
`read-key` but not to `read-key-sequence` (where you pass the info via
a dynamically-scoped var instead)?


        Stefan


> * src/lread.c (Fread_event): Update docstring for read-event to
> recommend read-key.
> * lisp/subr.el (read-key): Add new parameter, all-mouse-events to
> prevent discarding mouse events normally discarded by
> read-key-sequence.
> * doc/lispref/commands.texi (Reading One Event): Document new
> parameter.
> * src/keyboard.c (read_key_sequence): Implement new behavior for
> read-key.
> (inhibit--unbound-mouse-fallback): New variable to enable new
> behavior.
> (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 it.
> * lisp/wid-edit.el (widget-key-sequence-read-event): Call read-key,
> delete manual application of function-key-map.
> * lisp/vc/ediff.el (ediff-windows): Use display-mouse-p to check if a
> mouse is available.
> ---
>  doc/lispref/commands.texi |  6 +++++-
>  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              | 23 +++++++++++++++++++++--
>  lisp/textmodes/artist.el  |  6 +++---
>  lisp/vc/ediff-wind.el     |  5 +++--
>  lisp/vc/ediff.el          |  2 +-
>  lisp/wid-edit.el          | 17 +++++------------
>  src/keyboard.c            | 10 +++++++++-
>  src/lread.c               |  6 ++++++
>  14 files changed, 72 insertions(+), 40 deletions(-)
>
> diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
> index ebfda01671..29d56a434c 100644
> --- a/doc/lispref/commands.texi
> +++ b/doc/lispref/commands.texi
> @@ -2691,7 +2691,7 @@ Reading One Event
>  If you wish to read a single key taking these translations into
>  account, use the function @code{read-key}:
>  
> -@defun read-key &optional prompt
> +@defun read-key &optional prompt all-mouse-events
>  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
> @@ -2701,6 +2701,10 @@ 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{all-mouse-events} is non-@code{nil} then button-down
> +and multi-click events will get returned.  Otherwise, these are
> +ignored as in @code{read-key-sequence}.
>  @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 a0aa250c4b..0364baf826 100644
> --- a/lisp/isearch.el
> +++ b/lisp/isearch.el
> @@ -2974,7 +2974,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 82e6178da1..6815e76ccb 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 11bc07a29c..a007568470 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 0b92a4f9b5..d4a7282529 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -2431,13 +2431,20 @@ 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)
>    "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 +2453,7 @@ 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)
>          (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.
> @@ -2497,6 +2505,17 @@ 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."
> +    (if xterm-mouse-mode
> +        (read-key nil 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 90e8d360c1..df9335ea42 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-potential-mouse-event))))
>        ;; 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-potential-mouse-event)))))
>  
>      ;; 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-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 c68dc71884..ecd69d54ea 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..55c0bf5526 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.")))
> @@ -3490,25 +3490,18 @@ '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))))
>    (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))))
>      (when (and (integerp ev)
>  	       (or (and (<= ?0 ev) (< ev (+ ?0 (min 10 read-quoted-char-radix))))
>  		   (and (<= ?a (downcase ev))
>  			(< (downcase ev) (+ ?a -10 (min 36 read-quoted-char-radix))))))
>        (setq unread-command-events (cons ev unread-command-events)
> -	    ev (read-quoted-char (format "Enter code (radix %d)" read-quoted-char-radix))
> -	    tr nil)
> +	    ev (read-quoted-char (format "Enter code (radix %d)" read-quoted-char-radix)))
>        (if (and (integerp ev) (not (characterp ev)))
>  	  (insert (char-to-string ev))))  ;; throw invalid char error
>      (setq ev (key-description (list ev)))
> -    (when (arrayp tr)
> -      (setq tr (key-description (list (aref tr 0))))
> -      (if (y-or-n-p (format "Key %s is translated to %s -- use %s? " ev tr tr))
> -	  (setq ev tr ev2 nil)))
>      (insert (if (= (char-before) ?\s)  "" " ") ev " ")
>      (if ev2
>  	(insert (key-description (list ev2)) " "))))
> diff --git a/src/keyboard.c b/src/keyboard.c
> index 49261fcc3e..9395448c52 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.




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-02 16:53                 ` Stefan Monnier
@ 2020-12-03  5:46                   ` Jared Finder via Emacs development discussions.
  2020-12-03 14:45                     ` Stefan Monnier
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-03  5:46 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Jared Finder via "Emacs development discussions.",
	Eli Zaretskii

On 2020-12-02 8:53 am, Stefan Monnier wrote:
>> Subject: [PATCH] Make libraries work with xterm-mouse-mode.
> 
> Could you explain (at least in the code, and ideally here as well) why
> we need this new `all-mouse-events` behavior?

I updated the function locally to look like as follows.  Let me know if 
you have further questions.

(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)))

> `function-key-map` has very similar effects (and to a large extent, the
> downgrading of mouse-down events controlled by `all-mouse-events` could
> (I'd even say should) be implemented in `function-key-map` rather than
> via the current ad-hoc code in read-key-sequence), so I'm not very
> comfortable with treating these mouse-event-rewritings differently from
> other rewritings.

Just a few comments:

Wouldn't that require binding 2^6 * 3 * 3 * 5 = 2880 events in 
function-key-map? (2^6 for six modifier keys, 3 for down vs up vs drag, 
3 for single vs double vs triple, 5 for the mouse buttons).  Is that an 
okay overhead to add on every key press?  If not, it could instead be 
done with a default binding in function-key-map.

In both of the above cases, I believe the mapping would need to be a 
Lisp function so it can conditionally decay a triple down event to a 
double, single, or nothing depending on what is bound.

And such behavior would want a special variable (as the code is 
currently in my patch) to disable it to avoid copying all of 
function-key-map in read-key.  So I think it is fully independent of my 
current patch.

> The other question is why you made `all-mouse-events` an argument to
> `read-key` but not to `read-key-sequence` (where you pass the info via
> a dynamically-scoped var instead)?

I did this to create an API for read-key but not for read-key-sequence.  
Any package that wants to read down mouse events and work with 
xterm-mouse should call (read-key nil t).  I didn't want to create a way 
to do the same think for read-key-sequence at I could not think of a use 
case.  The special variable is named inhibit--unbound-mouse-fallback to 
indicate it is private and could change or be removed in the future.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-03  5:46                   ` Jared Finder via Emacs development discussions.
@ 2020-12-03 14:45                     ` Stefan Monnier
  2020-12-03 17:31                       ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Stefan Monnier @ 2020-12-03 14:45 UTC (permalink / raw)
  To: Jared Finder
  Cc: Eli Zaretskii,
	Jared Finder via "Emacs development discussions."

Jared Finder [2020-12-02 21:46:53] wrote:

> On 2020-12-02 8:53 am, Stefan Monnier wrote:
>>> Subject: [PATCH] Make libraries work with xterm-mouse-mode.
>> Could you explain (at least in the code, and ideally here as well) why
>> we need this new `all-mouse-events` behavior?
>
> I updated the function locally to look like as follows.  Let me know if you
> have further questions.
>
> (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)))

That doesn't say what this function should do with non-mouse events, so
it makes it hard to decide what its behavior should be.

OK, so what you specifically need is for down events not to be
dropped, right?


>> `function-key-map` has very similar effects (and to a large extent, the
>> downgrading of mouse-down events controlled by `all-mouse-events` could
>> (I'd even say should) be implemented in `function-key-map` rather than
>> via the current ad-hoc code in read-key-sequence), so I'm not very
>> comfortable with treating these mouse-event-rewritings differently from
>> other rewritings.
> Just a few comments:
> Wouldn't that require binding 2^6 * 3 * 3 * 5 = 2880 events in
> function-key-map?

Yes, but that's only because of the limited form available in keymaps.
To make it practical, we'd need to add "computed keymaps".  This is
a long-standing desire of mine, which would be useful in various
circumstances (and should make it possible to remove a lot of ad-hoc
rewritings in read_key_sequence).

> And such behavior would want a special variable (as the code is currently in
> my patch) to disable it to avoid copying all of function-key-map in
> read-key.  So I think it is fully independent of my current patch.

Yes.  My point is just that a functionally "don't discard mouse-events"
is weird in a function which is not specifically about mouse events.
It naturally leads to "don't down case events", "don't remap `tab` to
TAB", etc...
There has to be a more general underlying principle.

Maybe we could (re)use the DONT-DOWNCASE-LAST arg of `read-key-sequence`
for that?  This would mean no change in `read-key` but a change in
`read-key-sequence` instead (and hence further-reaching consequences).

Or maybe an option to `read-key` to disable all
function-key-map-like remappings (i.e. remappings which are only applied
if there's no binding for that key-sequence)?


        Stefan




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-03 14:45                     ` Stefan Monnier
@ 2020-12-03 17:31                       ` Jared Finder via Emacs development discussions.
  2020-12-14  0:54                         ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-03 17:31 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Jared Finder via "Emacs development discussions.",
	Eli Zaretskii

On 2020-12-03 6:45 am, Stefan Monnier wrote:
> Jared Finder [2020-12-02 21:46:53] wrote:
> 
>> On 2020-12-02 8:53 am, Stefan Monnier wrote:
>>>> Subject: [PATCH] Make libraries work with xterm-mouse-mode.
>>> Could you explain (at least in the code, and ideally here as well) 
>>> why
>>> we need this new `all-mouse-events` behavior?
>> 
>> I updated the function locally to look like as follows.  Let me know 
>> if you
>> have further questions.
>> 
>> (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)))
> 
> That doesn't say what this function should do with non-mouse events, so
> it makes it hard to decide what its behavior should be.
> 
> OK, so what you specifically need is for down events not to be
> dropped, right?

I want no mouse event to get dropped (drag events also get dropped, 
right?) and no mouse events to get degraded to simpler forms.  In 
summary, I want mouse events to get returned unprocessed, as if from 
read-event, but with input-decode-map applied.

The alternative I presented further up in the thread was to apply 
input-decode-map manually to successive calls to read-event.  I got this 
working, though it sounded like modifying read-key was preferred earlier 
in this thread.

For context, here's the alternative:

(defun read-decoded-key ()
   "Read a single key, as if by `read-key'.

Unlike `read-key', this does not call `read-key-sequence' and
instead has a bare-bones implementation of its functionality.  In
particular, it applies `input-decode-map' but does not apply
`local-function-key-map' or `input-translation-map'."
   (let* ((keys '())
          (decoder input-decode-map)
          (echo-keystrokes 0)
          (timer (run-with-idle-timer
                  read-key-delay t
                  (lambda ()
                    (unless (null keys)
                      (throw 'read-decoded-key nil)))))
          result)
     (catch 'read-decoded-key
       (unwind-protect
           (while t
             (let ((next-key (read-event)))
               (push next-key keys)
               (setq decoder (lookup-key decoder (vector next-key))))
             (when (pcase decoder
                     ;; A direct decoding (common for function keys)
                     ((pred arrayp)
                      (setq result decoder))

                     ;; A function that does decoding (like for
                     ;; `xterm-mouse-mode')
                     ((pred functionp)
                      (setq result (funcall decoder nil))))
               (if (zerop (length result))
                   ;; The decoding is an empty vector to say "continue
                   ;; reading".  This happens when the key would be
                   ;; mouse-movement but `track-mouse' is nil.
                   (setq keys '()
                         decoder input-decode-map)
                 (throw 'read-decoded-key nil))))
         (cancel-timer timer)))

     ;; If no decoding, the accumulated keys are the result.
     (or result (vconcat (nreverse keys)))))

>>> `function-key-map` has very similar effects (and to a large extent, 
>>> the
>>> downgrading of mouse-down events controlled by `all-mouse-events` 
>>> could
>>> (I'd even say should) be implemented in `function-key-map` rather 
>>> than
>>> via the current ad-hoc code in read-key-sequence), so I'm not very
>>> comfortable with treating these mouse-event-rewritings differently 
>>> from
>>> other rewritings.
>> Just a few comments:
>> Wouldn't that require binding 2^6 * 3 * 3 * 5 = 2880 events in
>> function-key-map?
> 
> Yes, but that's only because of the limited form available in keymaps.
> To make it practical, we'd need to add "computed keymaps".  This is
> a long-standing desire of mine, which would be useful in various
> circumstances (and should make it possible to remove a lot of ad-hoc
> rewritings in read_key_sequence).

Makes sense.  I think the easiest way to do this would be to allow 
keymaps to have multiple conditional fallbacks.  Perhaps allow a binding 
in a keymap to be a hook that runs via 
`run-hook-with-args-until-success'?  It's a slight generalization of the 
current logic which allows functions.

Technically this can be done now with just a layer of indirection on the 
[t] binding in a keymap.

>> And such behavior would want a special variable (as the code is 
>> currently in
>> my patch) to disable it to avoid copying all of function-key-map in
>> read-key.  So I think it is fully independent of my current patch.
> 
> Yes.  My point is just that a functionally "don't discard mouse-events"
> is weird in a function which is not specifically about mouse events.
> It naturally leads to "don't down case events", "don't remap `tab` to
> TAB", etc...
> There has to be a more general underlying principle.
> 
> Maybe we could (re)use the DONT-DOWNCASE-LAST arg of 
> `read-key-sequence`
> for that?  This would mean no change in `read-key` but a change in
> `read-key-sequence` instead (and hence further-reaching consequences).
> 
> Or maybe an option to `read-key` to disable all
> function-key-map-like remappings (i.e. remappings which are only 
> applied
> if there's no binding for that key-sequence)?

One way to do this would be to add new behavior if DONT-DOWNCASE-LAST is 
a list.  In other words, if DONT-DOWNCASE-LAST is non-nil and not a 
list, existing behavior.  But if DONT-DOWNCASE-LAST is a list, we can 
look at the members of the list to determine what to do.  This is a 
slight change in existing behavior, but it is very unlikely to break 
anything.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-01 18:23             ` Eli Zaretskii
  2020-12-02  6:45               ` Jared Finder via Emacs development discussions.
@ 2020-12-14  0:36               ` Jared Finder via Emacs development discussions.
  1 sibling, 0 replies; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-14  0:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On 2020-12-01 10:23 am, Eli Zaretskii wrote:
>> Date: Mon, 30 Nov 2020 23:36:23 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> This makes sense to me.  I agree that read-key is likely to currently
>> behave different in some situations from read-event.  Are these
>> differences bugs that should be fixed or are they now features that 
>> can
>> not be changed?
> 
> I don't know.  I think we need to discuss each difference separately.
> 
>> I'm about 33% through analyzing read-key and have already found the
>> following differences:
>> 
>> * read-key resets this-command-keys, read-event appends to it.
>> * read-key does not return switch-frame events as they happen,
>> read-event does.
> 
> Thank you for doing this.  We should at least document these changes
> in some place, and probably also discuss at least some of them.

I went through the rest of the code path and did not find any other 
behavior differences.  I do not fully understand the logic around 
downcasing unbound keys (I think this is every key during read-key).  
However, I believe this doesn't matter as dont_downcase_last will be 
true from calls to read-key.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-03 17:31                       ` Jared Finder via Emacs development discussions.
@ 2020-12-14  0:54                         ` Jared Finder via Emacs development discussions.
  2020-12-14 15:32                           ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-14  0:54 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Eli Zaretskii,
	Jared Finder via "Emacs development discussions."

Any updates here?

I do not know what the etiquette is here in terms of replies, so I'd 
like to apologize in advance if I'm just being impatient.

And FYI, I have five other patches around the xterm-mouse that I'd like 
to also submit but have been holding back to avoid creating too much 
noise.  These just address issues I've noticed locally when running with 
my changes.

Two of the changes are straightforward:

* Make command `help-for-help` use input-decode-map (for term function 
keys and xterm mouse) and allow mouse wheel scrolling.
* Clicking tab-line-mode's "+" pops up TTY menu or TMM based on 
tty-menu-open-use-tmm.

Three will likely merit some discussion:

* Make menus popped up on a down mouse event not trigger on the first up 
event unless the mouse has moved. (This is due to [mode-line mouse-1] 
being newly defined to `tty-menu-select` in `tty-menu-navigation-map` in 
0695c9e8.)
* Do not clear echo area when input-decode-map returns an empty vector 
(e.g. when the event is mouse movement but track-mouse is nil)
* Make menu-bar-at-x properly handle gud-gdb's "toolbar"-like menubar 
and undefined menu items.

Let me know how you'd ideally like me to share these.  My initial 
thought was to avoid dumping a bunch of changes all at once and share 
these sequentially.  Thanks.

   -- MJF

On 2020-12-03 9:31 am, Jared Finder wrote:
> On 2020-12-03 6:45 am, Stefan Monnier wrote:
>> Jared Finder [2020-12-02 21:46:53] wrote:
>> 
>>> On 2020-12-02 8:53 am, Stefan Monnier wrote:
>>>>> Subject: [PATCH] Make libraries work with xterm-mouse-mode.
>>>> Could you explain (at least in the code, and ideally here as well) 
>>>> why
>>>> we need this new `all-mouse-events` behavior?
>>> 
>>> I updated the function locally to look like as follows.  Let me know 
>>> if you
>>> have further questions.
>>> 
>>> (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)))
>> 
>> That doesn't say what this function should do with non-mouse events, 
>> so
>> it makes it hard to decide what its behavior should be.
>> 
>> OK, so what you specifically need is for down events not to be
>> dropped, right?
> 
> I want no mouse event to get dropped (drag events also get dropped,
> right?) and no mouse events to get degraded to simpler forms.  In
> summary, I want mouse events to get returned unprocessed, as if from
> read-event, but with input-decode-map applied.
> 
> The alternative I presented further up in the thread was to apply
> input-decode-map manually to successive calls to read-event.  I got
> this working, though it sounded like modifying read-key was preferred
> earlier in this thread.
> 
> For context, here's the alternative:
> 
> (defun read-decoded-key ()
>   "Read a single key, as if by `read-key'.
> 
> Unlike `read-key', this does not call `read-key-sequence' and
> instead has a bare-bones implementation of its functionality.  In
> particular, it applies `input-decode-map' but does not apply
> `local-function-key-map' or `input-translation-map'."
>   (let* ((keys '())
>          (decoder input-decode-map)
>          (echo-keystrokes 0)
>          (timer (run-with-idle-timer
>                  read-key-delay t
>                  (lambda ()
>                    (unless (null keys)
>                      (throw 'read-decoded-key nil)))))
>          result)
>     (catch 'read-decoded-key
>       (unwind-protect
>           (while t
>             (let ((next-key (read-event)))
>               (push next-key keys)
>               (setq decoder (lookup-key decoder (vector next-key))))
>             (when (pcase decoder
>                     ;; A direct decoding (common for function keys)
>                     ((pred arrayp)
>                      (setq result decoder))
> 
>                     ;; A function that does decoding (like for
>                     ;; `xterm-mouse-mode')
>                     ((pred functionp)
>                      (setq result (funcall decoder nil))))
>               (if (zerop (length result))
>                   ;; The decoding is an empty vector to say "continue
>                   ;; reading".  This happens when the key would be
>                   ;; mouse-movement but `track-mouse' is nil.
>                   (setq keys '()
>                         decoder input-decode-map)
>                 (throw 'read-decoded-key nil))))
>         (cancel-timer timer)))
> 
>     ;; If no decoding, the accumulated keys are the result.
>     (or result (vconcat (nreverse keys)))))
> 
>>>> `function-key-map` has very similar effects (and to a large extent, 
>>>> the
>>>> downgrading of mouse-down events controlled by `all-mouse-events` 
>>>> could
>>>> (I'd even say should) be implemented in `function-key-map` rather 
>>>> than
>>>> via the current ad-hoc code in read-key-sequence), so I'm not very
>>>> comfortable with treating these mouse-event-rewritings differently 
>>>> from
>>>> other rewritings.
>>> Just a few comments:
>>> Wouldn't that require binding 2^6 * 3 * 3 * 5 = 2880 events in
>>> function-key-map?
>> 
>> Yes, but that's only because of the limited form available in keymaps.
>> To make it practical, we'd need to add "computed keymaps".  This is
>> a long-standing desire of mine, which would be useful in various
>> circumstances (and should make it possible to remove a lot of ad-hoc
>> rewritings in read_key_sequence).
> 
> Makes sense.  I think the easiest way to do this would be to allow
> keymaps to have multiple conditional fallbacks.  Perhaps allow a
> binding in a keymap to be a hook that runs via
> `run-hook-with-args-until-success'?  It's a slight generalization of
> the current logic which allows functions.
> 
> Technically this can be done now with just a layer of indirection on
> the [t] binding in a keymap.
> 
>>> And such behavior would want a special variable (as the code is 
>>> currently in
>>> my patch) to disable it to avoid copying all of function-key-map in
>>> read-key.  So I think it is fully independent of my current patch.
>> 
>> Yes.  My point is just that a functionally "don't discard 
>> mouse-events"
>> is weird in a function which is not specifically about mouse events.
>> It naturally leads to "don't down case events", "don't remap `tab` to
>> TAB", etc...
>> There has to be a more general underlying principle.
>> 
>> Maybe we could (re)use the DONT-DOWNCASE-LAST arg of 
>> `read-key-sequence`
>> for that?  This would mean no change in `read-key` but a change in
>> `read-key-sequence` instead (and hence further-reaching consequences).
>> 
>> Or maybe an option to `read-key` to disable all
>> function-key-map-like remappings (i.e. remappings which are only 
>> applied
>> if there's no binding for that key-sequence)?
> 
> One way to do this would be to add new behavior if DONT-DOWNCASE-LAST
> is a list.  In other words, if DONT-DOWNCASE-LAST is non-nil and not a
> list, existing behavior.  But if DONT-DOWNCASE-LAST is a list, we can
> look at the members of the list to determine what to do.  This is a
> slight change in existing behavior, but it is very unlikely to break
> anything.
> 
>   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-14  0:54                         ` Jared Finder via Emacs development discussions.
@ 2020-12-14 15:32                           ` Eli Zaretskii
  2020-12-16  5:30                             ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2020-12-14 15:32 UTC (permalink / raw)
  To: Jared Finder; +Cc: monnier, emacs-devel

> Date: Sun, 13 Dec 2020 16:54:26 -0800
> From: Jared Finder <jared@finder.org>
> Cc: "Jared Finder via \"Emacs development discussions.\""
>  <emacs-devel@gnu.org>, Eli Zaretskii <eliz@gnu.org>
> 
> Any updates here?
> 
> I do not know what the etiquette is here in terms of replies, so I'd 
> like to apologize in advance if I'm just being impatient.

FWIW, I was waiting for Stefan to react to your responses.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-14 15:32                           ` Eli Zaretskii
@ 2020-12-16  5:30                             ` Jared Finder via Emacs development discussions.
  2020-12-19 18:32                               ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-16  5:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

On 2020-12-14 7:32 am, Eli Zaretskii wrote:
>> Date: Sun, 13 Dec 2020 16:54:26 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: "Jared Finder via \"Emacs development discussions.\""
>>  <emacs-devel@gnu.org>, Eli Zaretskii <eliz@gnu.org>
>> 
>> Any updates here?
>> 
>> I do not know what the etiquette is here in terms of replies, so I'd
>> like to apologize in advance if I'm just being impatient.
> 
> FWIW, I was waiting for Stefan to react to your responses.

It seems Stefan is very busy right now.  Would you be okay deferring the 
issues Stefan raised?  Or would you prefer to just wait?

My impression is that his concern is around if it makes sense to add a 
new parameter to read-key and if inhibit--unbound-mouse-fallback should 
instead be done through function-key-map (avoiding adding a new var).  
But these can both be avoided:

1. I can just not add a new parameter to read-key. I only did this so 
that external packages had a solution that didn't involve 
inhibit--unbound-mouse-fallback or read-potential-mouse-event.

2. Since inhibit--unbound-mouse-fallback is internal (both in doc 
comment and in its name), it can safely be removed in the future.

With the above done, if you / Stefan want to instead make this work via 
function-key-map and not add a parameter to read-key, that can be done 
while safely removing inhibit--unbound-mouse-fallback.

Thoughts?  I'm okay either way.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-16  5:30                             ` Jared Finder via Emacs development discussions.
@ 2020-12-19 18:32                               ` Eli Zaretskii
  2020-12-19 22:50                                 ` Stefan Monnier
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2020-12-19 18:32 UTC (permalink / raw)
  To: Jared Finder; +Cc: monnier, emacs-devel

> Date: Tue, 15 Dec 2020 21:30:24 -0800
> From: Jared Finder <jared@finder.org>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> >> Any updates here?
> >> 
> >> I do not know what the etiquette is here in terms of replies, so I'd
> >> like to apologize in advance if I'm just being impatient.
> > 
> > FWIW, I was waiting for Stefan to react to your responses.
> 
> It seems Stefan is very busy right now.  Would you be okay deferring the 
> issues Stefan raised?  Or would you prefer to just wait?

Let's see what Stefan suggests here.  Stefan?



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-19 18:32                               ` Eli Zaretskii
@ 2020-12-19 22:50                                 ` Stefan Monnier
  2020-12-20  7:26                                   ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Stefan Monnier @ 2020-12-19 22:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Jared Finder, emacs-devel

> Let's see what Stefan suggests here.  Stefan?

I'm afraid I don't have much time to think about this.

My suggestion is to add an option to `read-key` which makes it ignore
`function-key-map` and all other hardcoded remappings that are only
applied when a key sequence is not bound (i.e. those remappings which
could/should conceptually be applied by `function-key-map` but aren't
for various historical/technical reasons, such as dropping `down` events,
demoting `drag` events, downcasing letters, ...).

At the level of `read-key-sequence` this should use the
`dont-downcase-last` argument (since `read-key` only reads one event,
it's OK if `read-key-sequence` only applies it to the last event).


        Stefan




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-19 22:50                                 ` Stefan Monnier
@ 2020-12-20  7:26                                   ` Jared Finder via Emacs development discussions.
  2020-12-20 14:07                                     ` Stefan Monnier
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-20  7:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

On 2020-12-19 2:50 pm, Stefan Monnier wrote:
>> Let's see what Stefan suggests here.  Stefan?
> 
> I'm afraid I don't have much time to think about this.
> 
> My suggestion is to add an option to `read-key` which makes it ignore
> `function-key-map` and all other hardcoded remappings that are only
> applied when a key sequence is not bound (i.e. those remappings which
> could/should conceptually be applied by `function-key-map` but aren't
> for various historical/technical reasons, such as dropping `down` 
> events,
> demoting `drag` events, downcasing letters, ...).
> 
> At the level of `read-key-sequence` this should use the
> `dont-downcase-last` argument (since `read-key` only reads one event,
> it's OK if `read-key-sequence` only applies it to the last event).

This sounds straightforward to me to implement.  The only question I 
have is how to handle backward compatibility.

I see a few options:

1. Change the behavior of the dont-downcase-last parameter to this more 
extensive meaning.  Update global-set-key (the only other caller who 
sets dont-downcase-last in Emacs' code) to take this new behavior into 
account.

2. Make the dont-downcase-last parameter have the new behavior only if 
it is passed some new value (for example: 'all-fallbacks).  Leave the 
existing behavior for any other value, especially 'nil and 't.

3. [My preference] Like 2, but with a deprecation message on values 
other than 'nil, 'all-fallbacks, or 't (or maybe 'downcase-last if we 
want full explicitness).  This allows maximal ability to define new 
behaviors in the future.


I like #3 as existing code would run unchanged.  The chance that any 
existing code passed 'all-fallbacks is extremely low.  And anyone 
supporting a package outside of Emacs passing some other value will 
notice the message when they upgrade to Emacs 28.

Thoughts?

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-20  7:26                                   ` Jared Finder via Emacs development discussions.
@ 2020-12-20 14:07                                     ` Stefan Monnier
  2020-12-20 23:27                                       ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Stefan Monnier @ 2020-12-20 14:07 UTC (permalink / raw)
  To: Jared Finder; +Cc: Eli Zaretskii, emacs-devel

> I see a few options:
>
> 1. Change the behavior of the dont-downcase-last parameter to this more
>    extensive meaning.  Update global-set-key (the only other caller who sets
>    dont-downcase-last in Emacs' code) to take this new behavior
>    into account.

I believe `global-set-key` does want to use this new behavior, yes.

> 2. Make the dont-downcase-last parameter have the new behavior only if it is
>    passed some new value (for example: 'all-fallbacks).  Leave the existing
>   behavior for any other value, especially 'nil and 't.
>
> 3. [My preference] Like 2, but with a deprecation message on values other
>    than 'nil, 'all-fallbacks, or 't (or maybe 'downcase-last if we want full
>    explicitness).  This allows maximal ability to define new behaviors in
>   the future.

These sound good as well.  I'd even put a deprecation message on the
`downcase-last` case.

> I like #3 as existing code would run unchanged.  The chance that any
> existing code passed 'all-fallbacks is extremely low.  And anyone supporting
> a package outside of Emacs passing some other value will notice the message
> when they upgrade to Emacs 28.
>
> Thoughts?

By order of preference, I'd say: #1, then #3 and finally #2.
But any one of those 3 is OK for me.


        Stefan




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-20 14:07                                     ` Stefan Monnier
@ 2020-12-20 23:27                                       ` Jared Finder via Emacs development discussions.
  2020-12-23 16:52                                         ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-20 23:27 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

On 2020-12-20 6:07 am, Stefan Monnier wrote:
>> I see a few options:
>> 
>> 1. Change the behavior of the dont-downcase-last parameter to this 
>> more
>>    extensive meaning.  Update global-set-key (the only other caller 
>> who sets
>>    dont-downcase-last in Emacs' code) to take this new behavior
>>    into account.
> 
> I believe `global-set-key` does want to use this new behavior, yes.
> 
>> 2. Make the dont-downcase-last parameter have the new behavior only if 
>> it is
>>    passed some new value (for example: 'all-fallbacks).  Leave the 
>> existing
>>   behavior for any other value, especially 'nil and 't.
>> 
>> 3. [My preference] Like 2, but with a deprecation message on values 
>> other
>>    than 'nil, 'all-fallbacks, or 't (or maybe 'downcase-last if we 
>> want full
>>    explicitness).  This allows maximal ability to define new behaviors 
>> in
>>   the future.
> 
> These sound good as well.  I'd even put a deprecation message on the
> `downcase-last` case.
> 
>> I like #3 as existing code would run unchanged.  The chance that any
>> existing code passed 'all-fallbacks is extremely low.  And anyone 
>> supporting
>> a package outside of Emacs passing some other value will notice the 
>> message
>> when they upgrade to Emacs 28.
>> 
>> Thoughts?
> 
> By order of preference, I'd say: #1, then #3 and finally #2.
> But any one of those 3 is OK for me.

Sounds good to me! Eli, do you have a preference here? I'd like to make 
sure I implement the right option.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-20 23:27                                       ` Jared Finder via Emacs development discussions.
@ 2020-12-23 16:52                                         ` Eli Zaretskii
  2020-12-23 17:21                                           ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2020-12-23 16:52 UTC (permalink / raw)
  To: Jared Finder; +Cc: monnier, emacs-devel

> Date: Sun, 20 Dec 2020 15:27:10 -0800
> From: Jared Finder <jared@finder.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> >> I like #3 as existing code would run unchanged.  The chance that any
> >> existing code passed 'all-fallbacks is extremely low.  And anyone 
> >> supporting
> >> a package outside of Emacs passing some other value will notice the 
> >> message
> >> when they upgrade to Emacs 28.
> >> 
> >> Thoughts?
> > 
> > By order of preference, I'd say: #1, then #3 and finally #2.
> > But any one of those 3 is OK for me.
> 
> Sounds good to me! Eli, do you have a preference here? I'd like to make 
> sure I implement the right option.

I'm afraid I've lost context here.  Your suggestion was to use this:

> +(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."
> +    (if xterm-mouse-mode
> +        (read-key nil t)
> +      (read-event)))

This uses read-key when xterm-mouse-mode is in use, otherwise uses
read-event as we did before.

But now the discussion is about read-key only, and also about
read-key-sequence, which was not on the table at all, AFAIR?  There
seems to be some gap here that I couldn't bridge upon.

So I'm sorry for slowing you down, but could you explain how the 3
possibilities you describe are related to the original issue, which
was that read-event is insufficient to support xterm-mouse-mode in
those places?

TIA



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-23 16:52                                         ` Eli Zaretskii
@ 2020-12-23 17:21                                           ` Jared Finder via Emacs development discussions.
  2020-12-24 18:43                                             ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-23 17:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

On 2020-12-23 8:52 am, Eli Zaretskii wrote:
>> Date: Sun, 20 Dec 2020 15:27:10 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
>> 
>> >> I like #3 as existing code would run unchanged.  The chance that any
>> >> existing code passed 'all-fallbacks is extremely low.  And anyone
>> >> supporting
>> >> a package outside of Emacs passing some other value will notice the
>> >> message
>> >> when they upgrade to Emacs 28.
>> >>
>> >> Thoughts?
>> >
>> > By order of preference, I'd say: #1, then #3 and finally #2.
>> > But any one of those 3 is OK for me.
>> 
>> Sounds good to me! Eli, do you have a preference here? I'd like to 
>> make
>> sure I implement the right option.
> 
> I'm afraid I've lost context here.  Your suggestion was to use this:
> 
>> +(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."
>> +    (if xterm-mouse-mode
>> +        (read-key nil t)
>> +      (read-event)))
> 
> This uses read-key when xterm-mouse-mode is in use, otherwise uses
> read-event as we did before.
> 
> But now the discussion is about read-key only, and also about
> read-key-sequence, which was not on the table at all, AFAIR?  There
> seems to be some gap here that I couldn't bridge upon.

The additional thing to keep in mind is that read-key is implemented on 
top of read-key-sequence.  read-key currently will never return down 
mouse events due to them being discarded in the (C function) 
read_key_sequence.

I highlighted a way to do this in the original patch in this email 
thread.  Stefan requested an alternative, which is to extend the 
behavior of the dont-downcase-last parameter to read-key-sequence. As 
this changes behavior for read-key-sequence, I want to know what is the 
right way to make this behavior change.

To simplify things, I think we can only look at #1 and #3, as both 
Stefan and I agree #3 is better than #2. The three choices I presented 
were:

1. [Stefan's preference] Change the behavior of the dont-downcase-last 
parameter to this more extensive meaning. Update global-set-key (the 
only other caller who sets dont-downcase-last in Emacs' code) to take 
this new behavior into account.

2. Make the dont-downcase-last parameter have the new behavior only if 
it is passed some new value (for example: 'all-fallbacks). Leave the 
existing behavior for any other value, especially 'nil and 't.

3. [My preference] Like 2, but with a deprecation message on values 
other than 'nil, 'all-fallbacks, or 't (or maybe 'downcase-last if we 
want full explicitness). This allows maximal ability to define new 
behaviors in the future.

> So I'm sorry for slowing you down, but could you explain how the 3
> possibilities you describe are related to the original issue, which
> was that read-event is insufficient to support xterm-mouse-mode in
> those places?

No need to apologize. I realize read_key_sequnce is the *CORE* part of 
Emacs' input handling so any change needs to be handled with utmost 
care.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-23 17:21                                           ` Jared Finder via Emacs development discussions.
@ 2020-12-24 18:43                                             ` Eli Zaretskii
  0 siblings, 0 replies; 41+ messages in thread
From: Eli Zaretskii @ 2020-12-24 18:43 UTC (permalink / raw)
  To: Jared Finder; +Cc: monnier, emacs-devel

> Date: Wed, 23 Dec 2020 09:21:10 -0800
> From: Jared Finder <jared@finder.org>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> >> +(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."
> >> +    (if xterm-mouse-mode
> >> +        (read-key nil t)
> >> +      (read-event)))
> > 
> > This uses read-key when xterm-mouse-mode is in use, otherwise uses
> > read-event as we did before.
> > 
> > But now the discussion is about read-key only, and also about
> > read-key-sequence, which was not on the table at all, AFAIR?  There
> > seems to be some gap here that I couldn't bridge upon.
> 
> The additional thing to keep in mind is that read-key is implemented on 
> top of read-key-sequence.  read-key currently will never return down 
> mouse events due to them being discarded in the (C function) 
> read_key_sequence.

And xterm-mouse-mode does need these down-mouse events?  For what
purpose?

> 1. [Stefan's preference] Change the behavior of the dont-downcase-last 
> parameter to this more extensive meaning. Update global-set-key (the 
> only other caller who sets dont-downcase-last in Emacs' code) to take 
> this new behavior into account.
> 
> 2. Make the dont-downcase-last parameter have the new behavior only if 
> it is passed some new value (for example: 'all-fallbacks). Leave the 
> existing behavior for any other value, especially 'nil and 't.
> 
> 3. [My preference] Like 2, but with a deprecation message on values 
> other than 'nil, 'all-fallbacks, or 't (or maybe 'downcase-last if we 
> want full explicitness). This allows maximal ability to define new 
> behaviors in the future.

I prefer 3 or 2.  1 sounds too radical to me.  It's true that in core
there's only one caller, but we have no idea what happens outside of
the core.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
@ 2020-12-26 23:49 Jared Finder via Emacs development discussions.
  2020-12-27 15:36 ` Stefan Monnier
  2021-01-02  8:17 ` Eli Zaretskii
  0 siblings, 2 replies; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-26 23:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

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

On 2020-12-24 10:43 am, Eli Zaretskii wrote:
>> Date: Wed, 23 Dec 2020 09:21:10 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
>> 
>> The additional thing to keep in mind is that read-key is implemented 
>> on
>> top of read-key-sequence.  read-key currently will never return down
>> mouse events due to them being discarded in the (C function)
>> read_key_sequence.
> 
> And xterm-mouse-mode does need these down-mouse events?  For what
> purpose?

Libraries need those events.  For example, artist mode relies on reading 
both down mouse events when drawing a polyline.

>> 1. [Stefan's preference] Change the behavior of the dont-downcase-last
>> parameter to this more extensive meaning. Update global-set-key (the
>> only other caller who sets dont-downcase-last in Emacs' code) to take
>> this new behavior into account.
>> 
>> 2. Make the dont-downcase-last parameter have the new behavior only if
>> it is passed some new value (for example: 'all-fallbacks). Leave the
>> existing behavior for any other value, especially 'nil and 't.
>> 
>> 3. [My preference] Like 2, but with a deprecation message on values
>> other than 'nil, 'all-fallbacks, or 't (or maybe 'downcase-last if we
>> want full explicitness). This allows maximal ability to define new
>> behaviors in the future.
> 
> I prefer 3 or 2.  1 sounds too radical to me.  It's true that in core
> there's only one caller, but we have no idea what happens outside of
> the core.

Great, I've coded behavior #3.  Updated patch attached.  The remaining 
work to do is just documentation, but I was hoping to defer that until 
the code otherwise looks good.  Let me know what you think.

   -- MJF

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

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

Still todo:
* Finalize comment.
* Update manual.
* Update release notes (changing read-key-sequence is a BIG DEAL).

WIP comment (this needs to be updated):

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 an option that tells
read-key-sequence (which read-key is built on top of) to return mouse
button down events.

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

* src/lread.c (Fread_event): Update docstring for read-event to
recommend read-key.
* lisp/subr.el (read-key): Add new parameter, all-mouse-events to
prevent discarding mouse events normally discarded by
read-key-sequence.
* doc/lispref/commands.texi (Reading One Event): Document new
parameter.
* src/keyboard.c (read_key_sequence): Implement new behavior for
read-key.
(inhibit--unbound-mouse-fallback): New variable to enable new
behavior.
(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 it.
* lisp/wid-edit.el (widget-key-sequence-read-event): Call read-key,
delete manual application of function-key-map.
* lisp/vc/ediff.el (ediff-windows): Use display-mouse-p to check if a
mouse is available.

WIP
---
 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              |  48 ++++++++++++++++--
 lisp/textmodes/artist.el  |   6 +--
 lisp/vc/ediff-wind.el     |   5 +-
 lisp/vc/ediff.el          |   2 +-
 lisp/wid-edit.el          |  12 ++---
 src/keyboard.c            | 101 ++++++++++++++++++++++++++++----------
 src/lread.c               |   6 +++
 14 files changed, 166 insertions(+), 63 deletions(-)

diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
index 15d7e4e3a7..ae41053d6e 100644
--- a/doc/lispref/commands.texi
+++ b/doc/lispref/commands.texi
@@ -2434,6 +2434,8 @@ Key Sequence Input
 with a mouse event is read using the keymaps of the buffer in the
 window that the mouse was in, not the current buffer.)
 
+TODO: update docs here.
+
 If the events are all characters and all can fit in a string, then
 @code{read-key-sequence} returns a string (@pxref{Strings of Events}).
 Otherwise, it returns a vector, since a vector can hold all kinds of
@@ -2486,7 +2488,7 @@ Key Sequence Input
 and does not set @code{quit-flag}.  @xref{Quitting}.
 @end defun
 
-@defun read-key-sequence-vector prompt &optional continue-echo dont-downcase-last switch-frame-ok command-loop
+@defun read-key-sequence-vector prompt &optional continue-echo fallbacks-disabled can-return-switch-frame command-loop
 This is like @code{read-key-sequence} except that it always
 returns the key sequence as a vector, never as a string.
 @xref{Strings of Events}.
@@ -2698,7 +2700,7 @@ Reading One Event
 If you wish to read a single key taking these translations into
 account, use the function @code{read-key}:
 
-@defun read-key &optional prompt
+@defun read-key &optional prompt all-fallbacks-disabled
 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
@@ -2706,8 +2708,14 @@ Reading One Event
 according to @code{input-decode-map}, @code{local-function-key-map},
 and @code{key-translation-map} (@pxref{Translation Keymaps}).
 
+TODO: update docs here.
+
 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{all-mouse-events} is non-@code{nil} then button-down
+and multi-click events will get returned.  Otherwise, these are
+ignored as in @code{read-key-sequence}.
 @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..36eb27312d 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1123,7 +1123,16 @@ global-set-key
 that you make with this function."
   (interactive
    (let* ((menu-prompting nil)
-          (key (read-key-sequence "Set key globally: " nil t)))
+          (key (read-key-sequence
+                "Set key globally: " nil
+                ;; FIXME: It'd be nicer if this was set to
+                ;; 'all-fallbacks, however that would not apply the
+                ;; transformations applied by local-function-key-map,
+                ;; for example control modified function keys.
+                ;; `widget-key-sequence-read-event' queries the user
+                ;; which key they want in this case, perhaps do the
+                ;; same here?
+                'downcase-last)))
      (list key
            (read-command (format "Set key %s to command: "
                                  (key-description key))))))
@@ -2456,13 +2465,24 @@ 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-fallbacks-disabled)
   "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-FALLBACKS-DISABLED 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.  See
+the parameter FALLBACKS-DISABLED to `read-key-sequence' for more
+details."
   ;; 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
@@ -2508,7 +2528,11 @@ read-key
 		   (lookup-key global-map [tool-bar])))
              map))
           (let* ((keys
-                  (catch 'read-key (read-key-sequence-vector prompt nil t)))
+                  (catch 'read-key
+                    (read-key-sequence-vector prompt nil
+                                              (if all-fallbacks-disabled
+                                                  'all-fallbacks
+                                                'downcase-last))))
                  (key (aref keys 0)))
             (if (and (> (length keys) 1)
                      (memq key '(mode-line header-line
@@ -2522,6 +2546,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..6be64218b3 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.")))
@@ -3490,11 +3490,11 @@ '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))))
   (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/keyboard.c b/src/keyboard.c
index 54232aaea1..60c8fb2619 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -1230,8 +1230,8 @@ some_mouse_moved (void)
    sans error-handling encapsulation.  */
 
 enum { READ_KEY_ELTS = 30 };
-static int read_key_sequence (Lisp_Object *, Lisp_Object,
-                              bool, bool, bool, bool);
+static int read_key_sequence (Lisp_Object *, Lisp_Object, Lisp_Object, bool,
+                              bool, bool);
 static void adjust_point_for_property (ptrdiff_t, bool);
 
 Lisp_Object
@@ -1350,7 +1350,7 @@ command_loop_1 (void)
       /* Read next key sequence; i gets its length.  */
       raw_keybuf_count = 0;
       Lisp_Object keybuf[READ_KEY_ELTS];
-      int i = read_key_sequence (keybuf, Qnil, false, true, true, false);
+      int i = read_key_sequence (keybuf, Qnil, Qnil, true, true, false);
 
       /* A filter may have run while we were reading the input.  */
       if (! FRAME_LIVE_P (XFRAME (selected_frame)))
@@ -1597,7 +1597,7 @@ read_menu_command (void)
   specbind (Qecho_keystrokes, make_fixnum (0));
 
   Lisp_Object keybuf[READ_KEY_ELTS];
-  int i = read_key_sequence (keybuf, Qnil, false, true, true, true);
+  int i = read_key_sequence (keybuf, Qnil, Qnil, true, true, true);
 
   unbind_to (count, Qnil);
 
@@ -9203,15 +9203,28 @@ test_undefined (Lisp_Object binding)
    These include any minor mode keymaps active in the current buffer,
    the current buffer's local map, and the global map.
 
-   If a key sequence has no other bindings, we check Vfunction_key_map
-   to see if some trailing subsequence might be the beginning of a
-   function key's sequence.  If so, we try to read the whole function
-   key, and substitute its symbolic name into the key sequence.
+   There are a handful of fallbacks that apply only if a key sequence
+   has no other bindings.  These apply conditionally based on the
+   value of FALLBACKS_DISABLED:
 
-   We ignore unbound `down-' mouse clicks.  We turn unbound `drag-' and
-   `double-' events into similar click events, if that would make them
-   bound.  We try to turn `triple-' events first into `double-' events,
-   then into clicks.
+   * If a key sequence is unbound, we check Vfunction_key_map to see
+   if some trailing subsequence might be the beginning of a function
+   key's sequence.  If so, we try to read the whole function key, and
+   substitute its symbolic name into the key sequence.
+
+   * If a `down-' mouse click event is unbound, it is discarded.
+
+   * If a `drag-' or `double-' event is unbound, it is turned into
+   similar click events if that would make them bound.  We try to turn
+   `triple-' events first into `double-' events, then into clicks.
+
+   * If a capital letter is unbound, it is turned into a lower case
+   letter if that would make it bound.
+
+   If FALLBACKS_DISABLED is Qnil, all of the above fallbacks are
+   applied.  If it is Qall_fallbacks, then none of the fallbacks are
+   applied.  If it is Qdowncase_last, the capital letter fallback is
+   not applied, but all other ones are applied.
 
    If we get a mouse click in a mode line, vertical divider, or other
    non-text area, we treat the click as if it were prefixed by the
@@ -9230,8 +9243,9 @@ test_undefined (Lisp_Object binding)
 
 static int
 read_key_sequence (Lisp_Object *keybuf, Lisp_Object prompt,
-		   bool dont_downcase_last, bool can_return_switch_frame,
-		   bool fix_current_buffer, bool prevent_redisplay)
+		   Lisp_Object fallbacks_disabled,
+                   bool can_return_switch_frame, bool fix_current_buffer,
+                   bool prevent_redisplay)
 {
   ptrdiff_t count = SPECPDL_INDEX ();
 
@@ -9774,7 +9788,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) || EQ (fallbacks_disabled, Qall_fallbacks))
 	/* 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.
@@ -9986,7 +10000,8 @@ read_key_sequence (Lisp_Object *keybuf, Lisp_Object prompt,
 				     first_binding >= nmaps) we don't want
 				     to apply this function-key-mapping.  */
 				  (fkey.end + 1 == t
-				   && test_undefined (current_binding)),
+				   && test_undefined (current_binding)
+                                   && !EQ (fallbacks_disabled, Qall_fallbacks)),
 				  &diff, prompt);
 	    if (done)
 	      {
@@ -10007,7 +10022,8 @@ read_key_sequence (Lisp_Object *keybuf, Lisp_Object prompt,
 	  int diff;
 
 	  done = keyremap_step (keybuf, &keytran, max (t, mock_input),
-				true, &diff, prompt);
+                                !EQ (fallbacks_disabled, Qall_fallbacks),
+                                &diff, prompt);
 	  if (done)
 	    {
 	      mock_input = diff + max (t, mock_input);
@@ -10027,7 +10043,8 @@ read_key_sequence (Lisp_Object *keybuf, Lisp_Object prompt,
 	 use the corresponding lower-case letter instead.  */
       if (NILP (current_binding)
 	  && /* indec.start >= t && fkey.start >= t && */ keytran.start >= t
-	  && FIXNUMP (key))
+	  && FIXNUMP (key)
+          && !EQ (fallbacks_disabled, Qall_fallbacks))
 	{
 	  Lisp_Object new_key;
 	  EMACS_INT k = XFIXNUM (key);
@@ -10073,7 +10090,8 @@ read_key_sequence (Lisp_Object *keybuf, Lisp_Object prompt,
 	 and is a shifted function key,
 	 use the corresponding unshifted function key instead.  */
       if (NILP (current_binding)
-	  && /* indec.start >= t && fkey.start >= t && */ keytran.start >= t)
+	  && /* indec.start >= t && fkey.start >= t && */ keytran.start >= t
+          && !EQ (fallbacks_disabled, Qall_fallbacks))
 	{
 	  Lisp_Object breakdown = parse_modifiers (key);
 	  int modifiers
@@ -10127,7 +10145,9 @@ read_key_sequence (Lisp_Object *keybuf, Lisp_Object prompt,
 
   /* Don't downcase the last character if the caller says don't.
      Don't downcase it if the result is undefined, either.  */
-  if ((dont_downcase_last || NILP (current_binding))
+  if ((EQ (fallbacks_disabled, Qdowncase_last)
+       || EQ (fallbacks_disabled, Qall_fallbacks)
+       || NILP (current_binding))
       && t > 0
       && t - 1 == original_uppercase_position)
     {
@@ -10156,7 +10176,7 @@ read_key_sequence (Lisp_Object *keybuf, Lisp_Object prompt,
 
 static Lisp_Object
 read_key_sequence_vs (Lisp_Object prompt, Lisp_Object continue_echo,
-		      Lisp_Object dont_downcase_last,
+		      Lisp_Object fallbacks_disabled,
 		      Lisp_Object can_return_switch_frame,
 		      Lisp_Object cmd_loop, bool allow_string)
 {
@@ -10166,6 +10186,29 @@ read_key_sequence_vs (Lisp_Object prompt, Lisp_Object continue_echo,
     CHECK_STRING (prompt);
   maybe_quit ();
 
+  if (!EQ (fallbacks_disabled, Qnil)
+      && !EQ (fallbacks_disabled, Qall_fallbacks)
+      && !EQ (fallbacks_disabled, Qdowncase_last))
+    {
+      /* Prior to Emacs 28, fallbacks-disabled was named
+         dont-downcase-last and choose between two behaviors:
+
+         1. no fallbacks disabled (nil)
+         2. disable downcase of last event (any other value)
+
+         Warn for any code that has not yet made an explicit choice in
+         behavior.  */
+      fallbacks_disabled = Qdowncase_last;
+      Vdelayed_warnings_list
+        = Fcons
+        (list2 (Qemacs, build_string
+                ("Deprecated value for fallbacks-disabled passed to "
+                "`read-key-sequence'.\n"
+                "Prior to Emacs 28, this parameter was called "
+                "dont-downcase-last.")),
+         Vdelayed_warnings_list);
+    }
+
   specbind (Qinput_method_exit_on_first_char,
 	    (NILP (cmd_loop) ? Qt : Qnil));
   specbind (Qinput_method_use_echo_area,
@@ -10184,7 +10227,7 @@ read_key_sequence_vs (Lisp_Object prompt, Lisp_Object continue_echo,
 
   raw_keybuf_count = 0;
   Lisp_Object keybuf[READ_KEY_ELTS];
-  int i = read_key_sequence (keybuf, prompt, ! NILP (dont_downcase_last),
+  int i = read_key_sequence (keybuf, prompt, fallbacks_disabled,
 			     ! NILP (can_return_switch_frame), false, false);
 
 #if 0  /* The following is fine for code reading a key sequence and
@@ -10216,6 +10259,8 @@ DEFUN ("read-key-sequence", Fread_key_sequence, Sread_key_sequence, 1, 5, 0,
 Second (optional) arg CONTINUE-ECHO, if non-nil, means this key echos
 as a continuation of the previous key.
 
+TODO: update docs here.
+
 The third (optional) arg DONT-DOWNCASE-LAST, if non-nil, means do not
 convert the last event to lower case.  (Normally any upper case event
 is converted to lower case if the original event is undefined and the lower
@@ -10254,19 +10299,19 @@ Second (optional) arg CONTINUE-ECHO, if non-nil, means this key echos
 that this key sequence is being read by something that will
 read commands one after another.  It should be nil if the caller
 will read just one key sequence.  */)
-  (Lisp_Object prompt, Lisp_Object continue_echo, Lisp_Object dont_downcase_last, Lisp_Object can_return_switch_frame, Lisp_Object cmd_loop)
+  (Lisp_Object prompt, Lisp_Object continue_echo, Lisp_Object fallbacks_disabled, Lisp_Object can_return_switch_frame, Lisp_Object cmd_loop)
 {
-  return read_key_sequence_vs (prompt, continue_echo, dont_downcase_last,
+  return read_key_sequence_vs (prompt, continue_echo, fallbacks_disabled,
 			       can_return_switch_frame, cmd_loop, true);
 }
 
 DEFUN ("read-key-sequence-vector", Fread_key_sequence_vector,
        Sread_key_sequence_vector, 1, 5, 0,
        doc: /* Like `read-key-sequence' but always return a vector.  */)
-  (Lisp_Object prompt, Lisp_Object continue_echo, Lisp_Object dont_downcase_last, Lisp_Object can_return_switch_frame, Lisp_Object cmd_loop)
+  (Lisp_Object prompt, Lisp_Object continue_echo, Lisp_Object fallbacks_disabled, Lisp_Object can_return_switch_frame, Lisp_Object cmd_loop)
 {
-  return read_key_sequence_vs (prompt, continue_echo, dont_downcase_last,
-			       can_return_switch_frame, cmd_loop, false);
+  return read_key_sequence_vs (prompt, continue_echo, fallbacks_disabled,
+                               can_return_switch_frame, cmd_loop, false);
 }
 \f
 /* Return true if input events are pending.  */
@@ -11687,6 +11732,8 @@ syms_of_keyboard (void)
 
   DEFSYM (Qcommand_execute, "command-execute");
   DEFSYM (Qinternal_echo_keystrokes_prefix, "internal-echo-keystrokes-prefix");
+  DEFSYM (Qall_fallbacks, "all-fallbacks");
+  DEFSYM (Qdowncase_last, "downcase-last");
 
   accent_key_syms = Qnil;
   staticpro (&accent_key_syms);
diff --git a/src/lread.c b/src/lread.c
index 3ef874039a..74ed474944 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


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-26 23:49 Additional cleanup around xterm-mouse Jared Finder via Emacs development discussions.
@ 2020-12-27 15:36 ` Stefan Monnier
  2020-12-27 16:30   ` Jared Finder via Emacs development discussions.
  2021-01-02  8:17 ` Eli Zaretskii
  1 sibling, 1 reply; 41+ messages in thread
From: Stefan Monnier @ 2020-12-27 15:36 UTC (permalink / raw)
  To: Jared Finder; +Cc: Eli Zaretskii, emacs-devel

> -@defun read-key &optional prompt
> +@defun read-key &optional prompt all-fallbacks-disabled

FWIW, I would call it "fallbacks-disabled".

> -          (key (read-key-sequence "Set key globally: " nil t)))
> +          (key (read-key-sequence
> +                "Set key globally: " nil
> +                ;; FIXME: It'd be nicer if this was set to
> +                ;; 'all-fallbacks, however that would not apply the
> +                ;; transformations applied by local-function-key-map,
> +                ;; for example control modified function keys.
> +                ;; `widget-key-sequence-read-event' queries the user
> +                ;; which key they want in this case, perhaps do the
> +                ;; same here?
> +                'downcase-last)))

The name `all-fallbacks` makes it sound like it *enables* all fallbacks.
Similarly `downcase-last` makes it sound like we'll downcase the last event.

Not sure what would be best, but I'd suggest `dont-fallback` and `dont-downcase-last`.

> -                  (catch 'read-key (read-key-sequence-vector prompt nil t)))
> +                  (catch 'read-key
> +                    (read-key-sequence-vector prompt nil
> +                                              (if all-fallbacks-disabled
> +                                                  'all-fallbacks
> +                                                'downcase-last))))

BTW, just like above, it might be the case that many/most uses of
`read-key` would actually prefer `dont-fallback`, so I think a FIXME
should be added here saying we should review all uses of `read-key`
to see if they would benefit from the use of the new argument.

> @@ -3490,11 +3490,11 @@ '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))))
>    (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))

AFAICT this function doesn't make it possible to read double or triple clicks.
Maybe it should be consolidated with `help--read-key-sequence`.

> +   * If a key sequence is unbound, we check Vfunction_key_map to see
> +   if some trailing subsequence might be the beginning of a function
> +   key's sequence.  If so, we try to read the whole function key, and
> +   substitute its symbolic name into the key sequence.

Nowadays this deserves some serious quotes around "function key" since
function keys are normally handled in `input-decode-map` instead.

> +   * If a `down-' mouse click event is unbound, it is discarded.
> +
> +   * If a `drag-' or `double-' event is unbound, it is turned into
> +   similar click events if that would make them bound.  We try to turn
> +   `triple-' events first into `double-' events, then into clicks.
> +
> +   * If a capital letter is unbound, it is turned into a lower case
> +   letter if that would make it bound.
> +
> +   If FALLBACKS_DISABLED is Qnil, all of the above fallbacks are
> +   applied.  If it is Qall_fallbacks, then none of the fallbacks are
> +   applied.  If it is Qdowncase_last, the capital letter fallback is
> +   not applied, but all other ones are applied.

Hmm... now that I think about it, I wonder if we need to make changes in
`read_key_sequence` at all, because we can instead arrange for all keys
to be bound:

    diff --git a/lisp/subr.el b/lisp/subr.el
    index 725722cbee..1ca1d51d44 100644
    --- a/lisp/subr.el
    +++ b/lisp/subr.el
    @@ -2453,10 +2453,11 @@ memory-limit
     ;;;; Input and display facilities.
     
     (defconst read-key-empty-map (make-sparse-keymap))
    -
    +(defconst read-key-full-map
    +  (let ((map (make-sparse-keymap))) (define-key map [t] 'dummy) map))
     (defvar read-key-delay 0.01) ;Fast enough for 100Hz repeat rate, hopefully.
     
    -(defun read-key (&optional prompt)
    +(defun read-key (&optional prompt dont-fallback)
       "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'.
    @@ -2468,7 +2469,8 @@ read-key
       ;; 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
    +        (if dont-fallback read-key-full-map read-key-empty-map))
             (echo-keystrokes 0)
            (old-global-map (current-global-map))
             (timer (run-with-idle-timer

WDYT?

> diff --git a/src/lread.c b/src/lread.c
> index 3ef874039a..74ed474944 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.

I like that.  I'd say "decode events via" instead of "return events
via", tho.  And I'd also mention function keys since the same applies if
you want to read `f7` in a tty.


        Stefan




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-27 15:36 ` Stefan Monnier
@ 2020-12-27 16:30   ` Jared Finder via Emacs development discussions.
  2020-12-27 17:10     ` Stefan Monnier
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-27 16:30 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

On 2020-12-27 7:36 am, Stefan Monnier wrote:
>> -@defun read-key &optional prompt
>> +@defun read-key &optional prompt all-fallbacks-disabled
> 
> FWIW, I would call it "fallbacks-disabled".

I want to distinguish it from the very similar intentioned but somewhat 
differing behaving parameter to read-key-sequence.  If I don't need to 
change read-key-sequence's behavior (see below), then I am 100% on 
board.

> Hmm... now that I think about it, I wonder if we need to make changes 
> in
> `read_key_sequence` at all, because we can instead arrange for all keys
> to be bound:
> 
>     diff --git a/lisp/subr.el b/lisp/subr.el
>     index 725722cbee..1ca1d51d44 100644
>     --- a/lisp/subr.el
>     +++ b/lisp/subr.el
>     @@ -2453,10 +2453,11 @@ memory-limit
>      ;;;; Input and display facilities.
> 
>      (defconst read-key-empty-map (make-sparse-keymap))
>     -
>     +(defconst read-key-full-map
>     +  (let ((map (make-sparse-keymap))) (define-key map [t] 'dummy) 
> map))
>      (defvar read-key-delay 0.01) ;Fast enough for 100Hz repeat rate, 
> hopefully.
> 
>     -(defun read-key (&optional prompt)
>     +(defun read-key (&optional prompt dont-fallback)
>        "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'.
>     @@ -2468,7 +2469,8 @@ read-key
>        ;; 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
>     +        (if dont-fallback read-key-full-map read-key-empty-map))
>              (echo-keystrokes 0)
>             (old-global-map (current-global-map))
>              (timer (run-with-idle-timer
> 
> WDYT?

This needs to also avoid binding ESC as well, e.g. adding (define-key 
map [?\e] nil).  Cursory testing locally with that as well shows this 
working out well.

This assumes that ESC is the only prefix key in input-decode-map.  Is 
that an okay assumption to make?  It appears true locally on my xterm.


All other suggestions integrated into my local patch.

   -- MJF



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-27 16:30   ` Jared Finder via Emacs development discussions.
@ 2020-12-27 17:10     ` Stefan Monnier
  2020-12-28  0:22       ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Stefan Monnier @ 2020-12-27 17:10 UTC (permalink / raw)
  To: Jared Finder; +Cc: Eli Zaretskii, emacs-devel

>>> -@defun read-key &optional prompt
>>> +@defun read-key &optional prompt all-fallbacks-disabled
>> FWIW, I would call it "fallbacks-disabled".
> I want to distinguish it from the very similar intentioned but somewhat
> differing behaving parameter to read-key-sequence.

I wouldn't worry 'bout that.
It also occurs to me that it could use a more active form, like `disable-fallbacks`.

> This needs to also avoid binding ESC as well, e.g. adding (define-key map
> [?\e] nil).  Cursory testing locally with that as well shows this working
> out well.

Oh, that brings back memories of when I wrote `read-key`.
This is quite delicate indeed.

We're in dire need of tests for those things.

> This assumes that ESC is the only prefix key in input-decode-map.
> Is that an okay assumption to make?

I think so.


        Stefan




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-27 17:10     ` Stefan Monnier
@ 2020-12-28  0:22       ` Jared Finder via Emacs development discussions.
  0 siblings, 0 replies; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2020-12-28  0:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

[-- 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


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2020-12-26 23:49 Additional cleanup around xterm-mouse Jared Finder via Emacs development discussions.
  2020-12-27 15:36 ` Stefan Monnier
@ 2021-01-02  8:17 ` Eli Zaretskii
  2021-01-02 22:20   ` Jared Finder via Emacs development discussions.
  1 sibling, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2021-01-02  8:17 UTC (permalink / raw)
  To: Jared Finder; +Cc: monnier, emacs-devel

> Date: Sat, 26 Dec 2020 15:49:19 -0800
> From: Jared Finder <jared@finder.org>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > I prefer 3 or 2.  1 sounds too radical to me.  It's true that in core
> > there's only one caller, but we have no idea what happens outside of
> > the core.
> 
> Great, I've coded behavior #3.  Updated patch attached.  The remaining 
> work to do is just documentation, but I was hoping to defer that until 
> the code otherwise looks good.  Let me know what you think.

LGTM.  I didn't review the documentation changes, since they are
incomplete and somewhat outdated; I will do that when you post the
final result.

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2021-01-02  8:17 ` Eli Zaretskii
@ 2021-01-02 22:20   ` Jared Finder via Emacs development discussions.
  2021-01-09 12:27     ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2021-01-02 22:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

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

On 2021-01-02 12:17 am, Eli Zaretskii wrote:
>> Date: Sat, 26 Dec 2020 15:49:19 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
>> 
>> > I prefer 3 or 2.  1 sounds too radical to me.  It's true that in core
>> > there's only one caller, but we have no idea what happens outside of
>> > the core.
>> 
>> Great, I've coded behavior #3.  Updated patch attached.  The remaining
>> work to do is just documentation, but I was hoping to defer that until
>> the code otherwise looks good.  Let me know what you think.
> 
> LGTM.  I didn't review the documentation changes, since they are
> incomplete and somewhat outdated; I will do that when you post the
> final result.

Thanks!  I did a pass over the comments now.  Updated (hopefully final) 
patch attached.

   -- MJF

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

From 6b851818cbcf57464d9282a7ec6939725625add1 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 2 Jan 2021 14:10:17 -0800
Subject: [PATCH] Make libraries works with xterm-mouse-mode.

Change calls from 'read-event' to 'read-key' in libraries expecting
mouse events.  Do this only when 'xterm-mouse-mode' is enabled.  That
way those libraries read decoded mouse events instead of the
underlying escape sequence.  Add a parameter to 'read-key' that avoids
running any of the unbound fallbacks in 'read-key-sequence' so the
libraries can read mouse button-down events.

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'.  Mention that non-character events on
terminals need 'read-key'.
* lisp/subr.el (read-key-full-map): Add new keymap used by 'read-key'.
(read-key): Add new parameter 'fallbacks-disabled' to prevent running
any of the unbound fallbacks normally run by 'read-key-sequence'.
(read-potential-mouse-event): Add 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' instead of
'read-event'.
* lisp/wid-edit.el (widget-key-sequence-read-event): Call 'read-key'
with 'fallbacks-disabled' set instead of 'read-event'.  Unlike above
changes, this is unconditionally applied so it works for function
keys too.  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.
* src/lread.c (Fread_event): Recommend 'read-key' in docstring for
'read-event' for non-character events.
---
 doc/lispref/commands.texi | 13 ++++++++++--
 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, 92 insertions(+), 35 deletions(-)

diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
index 6c68f70482..0e9b455876 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,13 @@ 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.  Otherwise only
+downcasing of the last event is disabled.
 @end defun
 
 @defun read-char-choice prompt chars &optional inhibit-quit
diff --git a/lisp/foldout.el b/lisp/foldout.el
index 771b81e5be..7a4f451778 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 fefdd16d25..a1cb562afb 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2989,7 +2989,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 f6612600bd..ba03943be6 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 0da82882fc..fe2737d423 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 7cda6c96af..08e900a946 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 b0ab4f990f..16cfda3f00 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 1acc3c3250..cc9a1e7dfa 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2453,21 +2453,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
@@ -2521,6 +2543,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 ce620821d6..3a501fc680 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 72b345874f..f330a41fb3 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 e3612dd8e3..ed375738b4 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 f920130226..7e32f23d59 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 1ff0828e85..4a2e51d2a7 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' will decode events via `input-decode-map' that
+`read-event' will not.  On a terminal this includes function keys such
+as <f7> and <right>, or 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


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2021-01-02 22:20   ` Jared Finder via Emacs development discussions.
@ 2021-01-09 12:27     ` Eli Zaretskii
  2021-01-09 23:01       ` Jared Finder via Emacs development discussions.
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2021-01-09 12:27 UTC (permalink / raw)
  To: Jared Finder; +Cc: monnier, emacs-devel

> Date: Sat, 02 Jan 2021 14:20:06 -0800
> From: Jared Finder <jared@finder.org>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > LGTM.  I didn't review the documentation changes, since they are
> > incomplete and somewhat outdated; I will do that when you post the
> > final result.
> 
> Thanks!  I did a pass over the comments now.  Updated (hopefully final) 
> patch attached.

Thanks.

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

Should 'read-potential-mouse-event' be named
'read--potential-mouse-event', to signal its being an internal
function?

> +(defconst read-key-full-map
> +  (let ((map (make-sparse-keymap)))
> +    (define-key map [t] 'dummy)

Can we have either a doc string or a comment explaining the need and
the use of this function?

> +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.

The last sentence here is confusing, and even alarming: what is meant
"removing all bindings" there?  This needs to be explained or
reworded.

> +Otherwise, only downcasing of the last event is disabled."

I think this also needs a clearer wording.  First, "Otherwise" is
ambiguous: do you mean if DISABLE-FALLBACKS is nil or omitted, or do
you mean something else?  And second, the "downcasing of the last
event" part was not described anywhere, so it is a mystery for the
reader.

> +         ;; FIXME: Audit existing usage of `read-key' to see if they
                                     ^^^^^
"uses", I think.  It will also better fit with "they".

> +;; FIXME: Once there's a safe way to transition away from read-event,
> +;; this function should be deleted.

When the function is deleted, we will also need to make other changes,
like replace its use with something else, right?  Then please say that
here.

>  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'
                                           ^
A comma is missing there.

> +instead.  `read-key' will decode events via `input-decode-map' that
> +`read-event' will not.  On a terminal this includes function keys such
> +as <f7> and <right>, or mouse events generated by `xterm-mouse-mode'.

Please use upper case for function keys: <F7> and <RIGHT>.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2021-01-09 12:27     ` Eli Zaretskii
@ 2021-01-09 23:01       ` Jared Finder via Emacs development discussions.
  2021-01-15 11:54         ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jared Finder via Emacs development discussions. @ 2021-01-09 23:01 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

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

On 2021-01-09 4:27 am, Eli Zaretskii wrote:
>> Date: Sat, 02 Jan 2021 14:20:06 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
>> 
>> > LGTM.  I didn't review the documentation changes, since they are
>> > incomplete and somewhat outdated; I will do that when you post the
>> > final result.
>> 
>> Thanks!  I did a pass over the comments now.  Updated (hopefully 
>> final)
>> patch attached.
> 
> Thanks.

<List of feedback elided.>

I've addressed the feedback you provided.  Updated patch attached.

   -- MJF

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

From 3c7f42bf70d84b41409107a7696f3c5b4297cbbc Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 2 Jan 2021 14:10:17 -0800
Subject: [PATCH] Make libraries works with xterm-mouse-mode.

Change calls from 'read-event' to 'read-key' in libraries expecting
mouse events.  Do this only when 'xterm-mouse-mode' is enabled.  That
way those libraries read decoded mouse events instead of the
underlying escape sequence.  Add a parameter to 'read-key' that avoids
running any of the unbound fallbacks in 'read-key-sequence' so the
libraries can read mouse button-down events.

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'.  Mention that non-character events on
terminals need 'read-key'.
* lisp/subr.el (read-key-full-map): Add new keymap used by 'read-key'.
(read-key): Add new parameter 'fallbacks-disabled' to prevent running
any of the unbound fallbacks normally run by 'read-key-sequence'.
(read--potential-mouse-event): Add 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' instead of
'read-event'.
* lisp/wid-edit.el (widget-key-sequence-read-event): Call 'read-key'
with 'fallbacks-disabled' set instead of 'read-event'.  Unlike above
changes, this is unconditionally applied so it works for function
keys too.  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.
* src/lread.c (Fread_event): Recommend 'read-key' in docstring for
'read-event' for non-character events.
---
 doc/lispref/commands.texi | 14 ++++++++--
 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              | 54 ++++++++++++++++++++++++++++++++++++---
 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, 102 insertions(+), 36 deletions(-)

diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
index 6c68f70482..3a2c7d019e 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,14 @@ 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.  If @code{nil} or
+unspecified, the only fallback disabled is downcasing of the last
+event.
 @end defun
 
 @defun read-char-choice prompt chars &optional inhibit-quit
diff --git a/lisp/foldout.el b/lisp/foldout.el
index 771b81e5be..4c479d68e9 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 67cc7bed15..0c805b6546 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2997,7 +2997,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 f6612600bd..907ef06159 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 0da82882fc..8732fb8086 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 7cda6c96af..1e81904419 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 b0ab4f990f..55f2ae8cc4 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 260202945b..ab41d6836a 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2543,23 +2543,52 @@ memory-limit
 \f
 ;;;; Input and display facilities.
 
-(defconst read-key-empty-map (make-sparse-keymap))
+;; The following maps are used by `read-key' to remove all key
+;; bindings while calling `read-key-sequence'.  This way the keys
+;; returned are independent of the key binding state.
+
+(defconst read-key-empty-map (make-sparse-keymap)
+  "Used internally by `read-key'.")
+
+(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)
+  "Used internally by `read-key'.")
 
 (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
+while calling `read-key-sequence'.  If nil or unspecified, the
+only unbound fallback disabled is downcasing of the last event."
   ;; 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 uses 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
@@ -2613,6 +2642,23 @@ read-key
       (message nil)
       (use-global-map old-global-map))))
 
+;; FIXME: Once there's a safe way to transition away from read-event,
+;; callers to this function should be updated to that way and 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 ce620821d6..50c00c9532 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 72b345874f..47ef37a19e 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 e3612dd8e3..ed375738b4 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 8b10d71dcb..7dda04eda2 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.")))
@@ -3486,14 +3486,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 1ff0828e85..67a0217a13 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' will decode events via `input-decode-map' that
+`read-event' will not.  On a terminal this includes function keys such
+as <F7> and <RIGHT>, or 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


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: Additional cleanup around xterm-mouse
  2021-01-09 23:01       ` Jared Finder via Emacs development discussions.
@ 2021-01-15 11:54         ` Eli Zaretskii
  0 siblings, 0 replies; 41+ messages in thread
From: Eli Zaretskii @ 2021-01-15 11:54 UTC (permalink / raw)
  To: Jared Finder; +Cc: monnier, emacs-devel

> Date: Sat, 09 Jan 2021 15:01:16 -0800
> From: Jared Finder <jared@finder.org>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> I've addressed the feedback you provided.  Updated patch attached.

Thanks, I've now installed this on the master branch.



^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2021-01-15 11:54 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-26 23:49 Additional cleanup around xterm-mouse Jared Finder via Emacs development discussions.
2020-12-27 15:36 ` Stefan Monnier
2020-12-27 16:30   ` Jared Finder via Emacs development discussions.
2020-12-27 17:10     ` Stefan Monnier
2020-12-28  0:22       ` Jared Finder via Emacs development discussions.
2021-01-02  8:17 ` Eli Zaretskii
2021-01-02 22:20   ` Jared Finder via Emacs development discussions.
2021-01-09 12:27     ` Eli Zaretskii
2021-01-09 23:01       ` Jared Finder via Emacs development discussions.
2021-01-15 11:54         ` Eli Zaretskii
  -- strict thread matches above, loose matches on Subject: below --
2020-11-16  6:29 Jared Finder via Emacs development discussions.
2020-11-16 17:30 ` Jared Finder via Emacs development discussions.
2020-11-18 17:40 ` Eli Zaretskii
2020-11-19  8:03   ` Jared Finder via Emacs development discussions.
2020-11-21  9:31     ` Eli Zaretskii
2020-11-22 23:56       ` Jared Finder via Emacs development discussions.
2020-11-28 16:36         ` Eli Zaretskii
2020-12-01  7:36           ` Jared Finder via Emacs development discussions.
2020-12-01 15:21             ` Stefan Monnier
2020-12-01 18:23             ` Eli Zaretskii
2020-12-02  6:45               ` Jared Finder via Emacs development discussions.
2020-12-02 16:53                 ` Stefan Monnier
2020-12-03  5:46                   ` Jared Finder via Emacs development discussions.
2020-12-03 14:45                     ` Stefan Monnier
2020-12-03 17:31                       ` Jared Finder via Emacs development discussions.
2020-12-14  0:54                         ` Jared Finder via Emacs development discussions.
2020-12-14 15:32                           ` Eli Zaretskii
2020-12-16  5:30                             ` Jared Finder via Emacs development discussions.
2020-12-19 18:32                               ` Eli Zaretskii
2020-12-19 22:50                                 ` Stefan Monnier
2020-12-20  7:26                                   ` Jared Finder via Emacs development discussions.
2020-12-20 14:07                                     ` Stefan Monnier
2020-12-20 23:27                                       ` Jared Finder via Emacs development discussions.
2020-12-23 16:52                                         ` Eli Zaretskii
2020-12-23 17:21                                           ` Jared Finder via Emacs development discussions.
2020-12-24 18:43                                             ` Eli Zaretskii
2020-12-14  0:36               ` Jared Finder via Emacs development discussions.
2020-11-21 17:00     ` Stefan Monnier
2020-11-21  8:23   ` Eli Zaretskii
2020-11-15  8:49 Jared Finder via Emacs development discussions.
2020-11-15 18:11 ` Eli Zaretskii

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.