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

From: Jared Finder <[email-masked]>
Date: Wed, 27 Mar 2024 14:47:27 -0700

On 2024-03-26 16:50, Olaf Rogalsky wrote:
> Hi Jared,
> 
>> >> <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:
... elided comments ...
>> > 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:
... elided results ...
> 
> 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:
... elided results ...
> 
> 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.

Sadly, read_key_sequence in C code is quite a beast.

One last experiment is worth trying here.  If this doesn't work out, I 
think a FIXME will be sufficient.  Instead of returning the 
<select-window> event, try pushing the <select-window> event onto 
unread-command-events.  My thought is that this will let native code 
dequeue and handle the event normally, including taking 
can_return_switch_frame into account.  Can you please try this?

>> > 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'm not sure what the right way to proceed here is then.  Eli, can you 
give advice?

Looking at different OS files that handle mouse_autoselect_window, I see 
the following state for checks if the selected window is a minibuffer 
with MINI_WINDOW_P:

pgtkterm.c: checks
w32term.c: does NOT check
w32inevt.c: does NOT check
nsterm.m: checks
xterm.c: checks
msdos.c: does NOT check
haikuterm.c: checks
androidterm.c: checks
term.c: no support for mouse-autoselect-window. :(

My gut is to assume that the X and GTK behavior is most likely to be 
better tested and more correct, but I defer to Eli here.

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

I think the documentation is incorrect and this commented out case 
should be removed.  Local testing on PGTK and Mac shows that the mouse 
pointer can be over the window dividers, widget scroll bars, or fringes 
and still have autoselect behavior activate.

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

Agreed.  I think it should be added to Focus Events in commands.texi.

> Please find below the next iteration of the patch.

Assuming above comments are addressed, this looks good to me.  I imagine 
there will be minor comment reformatting to avoid going beyond 80 
columns.

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