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