emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Additional xterm-mouse cleanup


From: Jared Finder
Subject: Re: Additional xterm-mouse cleanup
Date: Sun, 14 Feb 2021 17:02:19 -0800
User-agent: Roundcube Webmail/1.3.16

Looks like this email got missed.  Thoughts on the updated patch?

I'd like to get the two remaining straightforward patches (make-help-screen, mouse-auto-select-window) done so that I can focus on the echo area improvements, which I think will be significantly more complex, as discussed below.

  -- MJF

On 2021-02-03 10:54 pm, Jared Finder wrote:
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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]