Subject: bug#69915: 30.0.50; mouse-autoselect-window has no effect in terminal

From: Olaf Rogalsky <[email-masked]>
Date: Tue, 26 Mar 2024 23:50:45 +0000

Hi Jared,

 > On 2024-03-24 12:27, Olaf Rogalsky wrote:
 > > Hi Jared,
 > > thanks for your feedback (answering this from my gmail account and
 > > hope this doesn't mess up the debbugs history).
 > > >> Are you certain you need the change to window.el as well? I'd be 
very
 > >> surprised if it is necessary to change
 > > ...
 > >> Is your setup is different somehow?
 > > No, but I forgot to mention, that the "nil <select-window> is 
undefined" error
 > > only occurred, iff mouse-autoselect-window had a numeric value.
 > > With my new patch, the error disappeared. I don't know why, but a
 > > change to window.el
 > > isn't necessary anymore. I still think, that the proposed change would
 > > be correct.
 > 
 > I'll defer to Eli here, but my general sentiment would be to leave 
window.el untouched unless a change is needed.  The event list returned was 
last changed in 2006 so it's reasonably stable.

I can't reproduce the error anymore, so I would leave it as is.

 > >> <select-window> events shouldn't be generated while the mouse is 
being
 > >> dragged. This probably is reflected in fully by track-mouse, but I'd
 > >> suggest looking at the native code that generates the event to 
confirm.
 > > Truly understanding xterm.c unfortunately is beyond my expertise. 
Nevertheless I
 > > tested, the behavior. Dragging the mouse from one window to the next
 > > (while passing over
 > > the modeline) gives the following sequence of events:
 > >  ESC [ < 3 5 ; 5 5 ; 4 1 M ESC [ < 0 ; 5 5 ; 4 1 M ;; 
mouse-drag-region
 > >  ESC [ < 3 2 ; 5 5 ; 4 2 M             ;; anonymous-command
 > >  ESC [ < 3 2 ; 5 5 ; 4 3 M             ;; anonymous-command
 > >  <help-echo> ESC [ < 3 2 ; 5 5 ; 4 4 M ;; ignore
 > >  ESC [ < 3 2 ; 5 6 ; 4 4 M             ;; anonymous-command
 > >  ESC [ < 0 ; 5 6 ; 4 4 m               ;; anonymous-command
 > >  <drag-mouse-1>                        ;; mouse-set-region
 > > So indeed, no select-window event is generated. As a result, dragging
 > > the mouse over the
 > > borders of the window results in a scrolling of the window. This
 > > matches the behavior of the
 > > X11 backend.
 > 
 > Thank you.  There's one remaining difference to handle that I highlight 
in the diff below.
 >
 > >> If there is a case where two events should be generated (not sure if
 > >> this case exists depending on above), we'd want to return both, but 
you
 > >> can only return a single key sequence from the translate function. I
 > >> think this case deserves a FIXME note.
 > > Can't follow you here. At which occasion two events might be generated
 > > and which ones?
 > 
 > I was thinking that if both a mouse movement and select window event 
should both be returned, like if track-mouse is non-nil and you switch 
windows.  Can you test this case?

Test case 1:
(track-mouse
   (setq mouse-autoselect-window t)

   (let (e)
     (while (not (eq e ?q))
       (setq e (read-key))
       (when (and (consp e) (symbolp (car e)))
           (message "%s to %s at posn %s" (car e) (caadr e) (posn-x-y (cadr 
e)))))))

Result with patched terminal:

mouse-movement to #<window 1 on *scratch*> at posn (65 . 40)
mouse-movement to #<window 1 on *scratch*> at posn (65 . 41)
select-window to #<window 4 on *scratch*> at posn (65 . 0)
mouse-movement to #<window 4 on *scratch*> at posn (64 . 0)
mouse-movement to #<window 4 on *scratch*> at posn (64 . 1)
=> select-window event is reported, but at a different posn as the next or
previous mouse-movement. So no simultaneous events for mouse-movement
and select-window.

Result for X11 backend

mouse-movement to #<window 3 on *scratch*> at posn (433 . 983)
mouse-movement to #<window 3 on *scratch*> at posn (432 . 1009)
mouse-movement to #<window 15 on *scratch*> at posn (423 . 0)
mouse-movement to #<window 15 on *scratch*> at posn (420 . 24)
=> no select-window event is reported to read-key at all!

For the second test case I use read-key-sequence instead of read-key

Test case 2:

(track-mouse
   (let ((can-return-switch-frame t) e)
     (while (not (equal e "q"))
       (setq e (read-key-sequence nil nil t can-return-switch-frame nil))
       (when (not (stringp e))
           (setq e (seq-elt e 0))
           (message "%s to %s at posn %s" (car e) (caadr e) (posn-x-y (cadr 
e)))))))

Result with patched terminal:

mouse-movement to #<window 1 on *scratch*> at posn (65 . 40)
mouse-movement to #<window 1 on *scratch*> at posn (65 . 41)
select-window to #<window 4 on *scratch*> at posn (65 . 0)
mouse-movement to #<window 4 on *scratch*> at posn (64 . 0)
mouse-movement to #<window 4 on *scratch*> at posn (64 . 1)
=> same as result for test case 1

Result for X11 backend:

mouse-movement to #<window 1 on *scratch*> at posn (43 . 39)
mouse-movement to #<window 1 on *scratch*> at posn (43 . 40)
select-window to #<window 4 on *scratch*> at posn (43 . 0)
mouse-movement to #<window 4 on *scratch*> at posn (43 . 1)
mouse-movement to #<window 4 on *scratch*> at posn (43 . 2)
=> Now a select window event is reported as in the terminal. And also
for X11: no simultaneous events for mouse-movement and select-window.

So, read-key behaves differently in the terminal compared to X11:
In the terminal, the can-return-switch-frame parameter of read-key-sequence
has no effect and select-window events are always generated.

Fyi, the can-return-switch-frame parameter is handled at +10838 keyboard.c
in the monstrous over 1000 lines long function read_key_sequence:

	  if (EVENT_HAS_PARAMETERS (key)
	      /* Either a `switch-frame' or a `select-window' event.  */
	      && EQ (EVENT_HEAD_KIND (EVENT_HEAD (key)), Qswitch_frame))
	    {
	      /* If we're at the beginning of a key sequence, and the caller
		 says it's okay, go ahead and return this event.  If we're
		 in the midst of a key sequence, delay it until the end.  */
	      if (t > 0 || !can_return_switch_frame)
		{
		  delayed_switch_frame = key;
		  goto replay_key;
		}
	    }
But apparently these lines are never executed in the case of the terminal 
input.

 > >> Did you try out switching frames? I'm not certain if <select-window> 
is
 > >> supposed to be generated when the frame is switched.
 > > Switching frames is not handled in xt-mouse.el. However, if you change
 > > the focus from another
 > > X11 window to the title bar of the terminal or wise-versa, no
 > > switch-frame event is generated. Instead,
 > > xterm-translate-focus-in/xterm-translate-focus-out are called via a
 > > binding in xterm-rxvt-function-map.
 > > These functions toggle the terminal parameter tty-focus-state between
 > > focused and defocused and then
 > > call after-focus-change-function, which also does not generate a
 > > switch-frame event.
 > > As far as I could find out, the X11 backend of emacs doesn't generate
 > > switch-frame events, either.
 > 
 > I was actually referring to using C-x 5 2 and C-x 5 o within a single 
terminal.  I tested this locally and it works fine.

Good!

 > > New patch:
 > > ... other parts of patch look good to me :) ...
 > > @@ -84,10 +89,19 @@ xterm-mouse-translate-1
 > >         vec)
 > >         (is-move
 > >          (xterm-mouse--handle-mouse-movement)
 > > -        (if track-mouse vec
 > > -          ;; Mouse movement events are currently supposed to be
 > > -          ;; suppressed.  Return no event.
 > > -          []))
 > > +       (if (and mouse-autoselect-window ; after mouse movement
 > 
 > Style nit: Can you please do this as a cond instead of a nested (if x y 
(if z u v))?

yes, of cause

 > > autoselect the mouse window, but ...
 > > +                (windowp ev-window) ; ignore modeline, tab-bar,
 > > menu-bar and so forth ...
 > > +                ;;(not (posn-area (event-start event))) ; also
 > > ignore, if not inside of text area of window ...
 > > +                (not (eq ev-window last-window)) ; but only, if mouse
 > > is over new window ...
 > > +                (not (eq ev-window (selected-window)))) ; which is
 > > different from the selected window
 > 
 > Looking at xterm.c, I think you also want a test against 
window-minibuffer-p.

On the other hand, looking at msdos.c, there is no test against the 
minibuffer. I believed, that
the selection of the minibuffer is taken care of at +10638 of window.el. In 
my tests the patch
behaves exactly like the documentation, quote: "Mouse auto-selection 
selects the minibuffer window
only if it is active, and never deselects the active minibuffer window."
I added the test, but commented it out.

I also commented out a condition, which ensures that the selection of a 
window can only occur, if 
the mouse is in the text area of the window. This matches the following 
sentence of the documentation:
"In either case, the mouse pointer must enter the text area of a window in 
order to trigger its selection."
But I found no situation, where it did matter and msdos.c didn't have that 
test, either. WDYT?

 > > +           (progn
 > > +             (put 'select-window 'event-kind 'switch-frame)
 > > +             (setf (car event) 'select-window)
 > > +             vec)
 > 
 > I think this should be an event that's just select-window and the 
window, to line up with what the select-window event looks like on other 
platforms (I tested PGTK and Windows terminal).  You can verify this by 
running M-x trace-function RET handle-select-window RET.
 > 
 > That would instead be something like:
 > 
 > (progn
 >   (put 'select-window 'event-kind 'switch-frame)
 >   (vector `(select-window (,ev-window)))

I can't find the documentation of the format of the select-window event. 
Maybe its a good idea to add it.

Please find below the next iteration of the patch.
Olaf

diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index 081b8f32456..88c6c28b293 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -60,7 +60,9 @@ xterm-mouse-translate-1
      (let* ((event (xterm-mouse-event extension))
            (ev-command (nth 0 event))
            (ev-data    (nth 1 event))
+          (ev-window  (nth 0 ev-data))
            (ev-where   (nth 1 ev-data))
+          (last-window (terminal-parameter nil 'xterm-mouse-last-window))
            (vec (vector event))
            (is-move (eq 'mouse-movement ev-command))
            (is-down (string-match "down-" (symbol-name ev-command))))
@@ -73,6 +75,9 @@ xterm-mouse-translate-1
                                  'mouse-movement
                                'mouse-click)))

+      ;; remember window of current mouse position
+      (set-terminal-parameter nil 'xterm-mouse-last-window ev-window)
+
        (cond
         ((null event) nil)              ;Unknown/bogus byte sequence!
         (is-down
@@ -84,10 +89,16 @@ xterm-mouse-translate-1
         vec)
         (is-move
          (xterm-mouse--handle-mouse-movement)
-        (if track-mouse vec
-          ;; Mouse movement events are currently supposed to be
-          ;; suppressed.  Return no event.
-          []))
+       (cond ((and mouse-autoselect-window ; after mouse movement 
autoselect the mouse window, but ...
+                   (windowp ev-window) ; ignore modeline, tab-bar, 
menu-bar and so forth ...
+                   ;;(not (posn-area (event-start event))) ; also ignore, 
if not inside of text area of window ...
+                    ;;(not (window-minibuffer-p (selected-window))) ; and 
don't deselect the minibuffer
+                    (not (eq ev-window last-window)) ; and select only, if 
mouse is over a new window ...
+                   (not (eq ev-window (selected-window)))) ; which is 
different from the selected window
+              (put 'select-window 'event-kind 'switch-frame)
+              (vector `(select-window (,ev-window))))
+              (track-mouse vec)
+              (t [])))
         (t
         (let* ((down (terminal-parameter nil 'xterm-mouse-last-down))
                (down-data (nth 1 down))