Re: Additional xterm-mouse cleanup

[Jared Finder]

On 2021-02-02 6:37 am, Stefan Monnier wrote:
> > > > Subject: [PATCH 3/4] Support 'mouse-autoselect-window' for GPM and 
> > > > xterm
> > > > mouse
> > > I think this should be consolidated with the *very* similar code in
> > > `src/msdos.c` and `src/w32inevt.c`.
> > > [ Ideally it should also be consolidated with the
> > > `Vmouse_autoselect_window`
> > >   support in `src/xterm.c`, `src/nsterm.m`, and `src/w32term.c`.  ]
> >
> > I completely agree.  However, I don't have access to any of those 
> > platforms
> > to test against, so I do not feel comfortable making any changes.
> > Instead I added a fixme to all the different locations handling mouse
> > movement.  Updated patch attached.
>
> I thought the parts in `src/msdos.c` and `src/w32inevt.c` were
> sufficiently similar that the change can be done without much risk of
> introducing a regression.

Ah, got it. I agree, it is mostly straightforward. To do this properly 
required making an assumption that .timestamp=0 for SELECT_WINDOW_EVENT 
is ok. Looking through the C code, I don't see any location that reads 
.timestamp for the SELECT_WINDOW_EVENT, so I make it uniformly 0 
throughout.  Updated patch attached.

> > > [ Oh ... how I hate that echo area code.  ]
>
> I have the impression that a whole lot of code can run between the
> `clear_message` and your code, so I don't immediately see why we can be
> sure that `echo_area_buffer[1]` indeed always contains the thing before
> the `clear_message`.  And if it doesn't, then maybe we shouldn't try to
> revert the echo message.

Good point. I will update my patch to have a copy of the echo area made 
inside read_key_sequence.

> I agree it doesn't seem easy, but in my experience "do and later undo"
> sooner or later leads to extra difficulties so I tend to prefer "delay
> the do until we're sure we want to perform it".

I completely agree with the sentiment, but I do not think it is the 
right tradeoff. To delay until we're sure, we'd need to have some sort 
of assumption of how terminal escape sequences are received that normal 
humans would never do. Consider that the following key sequence is a 
mouse movement escape sequence but is completely possible for a human to 
type slowly:

ESC [ < 3 5 ; 1 9 ; 3 4 m

What should the echo area display if it has read "ESC ["? At this point, 
input-decode-map still doesn't know if this is a xterm escape sequence 
or not. This could just be part way through the above sequence, in which 
case nothing should be displayed. Alternatively, a user could have "ESC 
[ a" bound to an arbitrary command and they're part of the way through 
triggering that command, in which sase "ESC [-" should be displayed. Any 
change here is going to be *BIG*, which feels like a high risk change 
for a not-yet-demonstrated bug.

Or to use a metaphor: this feels like you're asking for heart bypass 
surgery before putting a bandage over a cut on Mr. Echo Area's elbow. I 
don't disagree that Mr. Echo Area would be able to be much more active 
after the surgery (and all that smoking raised his blood pressure a 
lot), but right now all he really wants is the bandage.

I do completely agree with adding the unit tests to ensure I don't make 
things worse.

  -- MJF

0002-Support-mouse-autoselect-window-for-GPM-and-xterm-mo.patch
Description: Text Data