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

[Olaf Rogalsky]

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