Re: Additional xterm-mouse cleanup

[Stefan Monnier]

>> LGTM, but it would be good to improve the code so it determine this set
>> of events automatically, e.g. by looking up local-map or something.
>
> I just went ahead and did a cleanup of the logic around here.  Now there's
> a keymap for all the navigation commands.  Also a simple cache of all help
> characters.  Updated patch attached.

LGTM.

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

>> [ Oh ... how I hate that echo area code.  ]
>> I think this needs a reproducible test case, ideally one we can run in
>> batch as part of our test suite.
>
> I can do this, but I'm not sure what is the right way to test here.
> Best I can see is to use current-message to read the current message string,
> set unread-command-events to simulate key presses, and then call
> read-key-sequence with a timeotu (like how read-key works).  Does this
> sound right?

It does sound right, but I wouldn't be surprised if you encounter some
difficulty because the code behaves differently in batch mode or because
that message somehow gets erased just before your Elisp code gets to run.

We don't yet have much experience testing these "interactive" aspects of
Emacs, so it still frequently requires further changes to the code to
make such testing possible.  That's another reason it'd be good to have
a test case here (it's a good opportunity to improve our testing
facilities).

>> Also, I'm curious about a few things:
>> - How do we know that the echo area was cleared by calls to `read_char`?
>>   [ Do we even know for sure that `read_char` always clears it?  ]
>
> read_char clears the echo area by calling "clear_message (1, 0)" if it
> encounters any event other than help-echo, switch-frame, or select-window in
> normal flow.  See the comment "Now wipe the echo area, except for" in
> read_char.
>
>> - How do we know that this extra line has the effect of restoring it to
>>   its pristine state?
>
> Isn't that what echo_area_buffer[1] is for?

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.

>> I think I'd like it better if we could make those things clear in
>> the code.  E.g. by having the "code that clears" set some variable
>> indicating that it's indeed been cleared and maybe also how it's been
>> cleared, and which we can then use here.
>> Or alternatively, change the "code that clears" so that it doesn't
>> actually clear when called from `read_key_sequence` and let
>> `read_key_sequence` take care of clearing the echo area once it has
>> *really* read some input?
>
> I don't think these are feasible as read_char's modifications of the echo
> area can be dependent on waiting for user input.  Specifically, it echos the
> dash (via echo_dash) while waiting for input after an idle timer activates.
> Pulling out read_char's existing code around idle timers to control all this
> will be a beast to pull out.  Additionally, I don't see how
> read_key_sequence could know any better what to do as the translation maps
> are just translating individual character events that if typed slowly
> *should* be echoed.

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".


        Stefan