Re: Additional cleanup around xterm-mouse

[Stefan Monnier <monnier@iro.umontreal.ca>]

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