Re: Additional cleanup around xterm-mouse

[Jared Finder via "Emacs development discussions." <emacs-devel@gnu.org>]

Any updates here?

I do not know what the etiquette is here in terms of replies, so I'd 
like to apologize in advance if I'm just being impatient.

And FYI, I have five other patches around the xterm-mouse that I'd like 
to also submit but have been holding back to avoid creating too much 
noise.  These just address issues I've noticed locally when running with 
my changes.

Two of the changes are straightforward:

* Make command `help-for-help` use input-decode-map (for term function 
keys and xterm mouse) and allow mouse wheel scrolling.
* Clicking tab-line-mode's "+" pops up TTY menu or TMM based on 
tty-menu-open-use-tmm.

Three will likely merit some discussion:

* Make menus popped up on a down mouse event not trigger on the first up 
event unless the mouse has moved. (This is due to [mode-line mouse-1] 
being newly defined to `tty-menu-select` in `tty-menu-navigation-map` in 
0695c9e8.)
* Do not clear echo area when input-decode-map returns an empty vector 
(e.g. when the event is mouse movement but track-mouse is nil)
* Make menu-bar-at-x properly handle gud-gdb's "toolbar"-like menubar 
and undefined menu items.

Let me know how you'd ideally like me to share these.  My initial 
thought was to avoid dumping a bunch of changes all at once and share 
these sequentially.  Thanks.

   -- MJF

On 2020-12-03 9:31 am, Jared Finder wrote:
> On 2020-12-03 6:45 am, Stefan Monnier wrote:
>> Jared Finder [2020-12-02 21:46:53] wrote:
>> 
>>> On 2020-12-02 8:53 am, Stefan Monnier wrote:
>>>>> Subject: [PATCH] Make libraries work with xterm-mouse-mode.
>>>> Could you explain (at least in the code, and ideally here as well) 
>>>> why
>>>> we need this new `all-mouse-events` behavior?
>>> 
>>> I updated the function locally to look like as follows.  Let me know 
>>> if you
>>> have further questions.
>>> 
>>> (defun read-potential-mouse-event ()
>>>     "Read an event that might be a mouse event.
>>> 
>>> This function exists for backward compatibility in code packaged
>>> with Emacs.  Do not call it directly in your own packages."
>>>     ;; `xterm-mouse-mode' events must go through `read-key' as they
>>>     ;; are decoded via `input-decode-map'.
>>>     (if xterm-mouse-mode
>>>         (read-key nil
>>>                   ;; Normally `read-key' discards all mouse button
>>>                   ;; down events.  However, we want them here.
>>>                   t)
>>>       (read-event)))
>> 
>> That doesn't say what this function should do with non-mouse events, 
>> so
>> it makes it hard to decide what its behavior should be.
>> 
>> OK, so what you specifically need is for down events not to be
>> dropped, right?
> 
> I want no mouse event to get dropped (drag events also get dropped,
> right?) and no mouse events to get degraded to simpler forms.  In
> summary, I want mouse events to get returned unprocessed, as if from
> read-event, but with input-decode-map applied.
> 
> The alternative I presented further up in the thread was to apply
> input-decode-map manually to successive calls to read-event.  I got
> this working, though it sounded like modifying read-key was preferred
> earlier in this thread.
> 
> For context, here's the alternative:
> 
> (defun read-decoded-key ()
>   "Read a single key, as if by `read-key'.
> 
> Unlike `read-key', this does not call `read-key-sequence' and
> instead has a bare-bones implementation of its functionality.  In
> particular, it applies `input-decode-map' but does not apply
> `local-function-key-map' or `input-translation-map'."
>   (let* ((keys '())
>          (decoder input-decode-map)
>          (echo-keystrokes 0)
>          (timer (run-with-idle-timer
>                  read-key-delay t
>                  (lambda ()
>                    (unless (null keys)
>                      (throw 'read-decoded-key nil)))))
>          result)
>     (catch 'read-decoded-key
>       (unwind-protect
>           (while t
>             (let ((next-key (read-event)))
>               (push next-key keys)
>               (setq decoder (lookup-key decoder (vector next-key))))
>             (when (pcase decoder
>                     ;; A direct decoding (common for function keys)
>                     ((pred arrayp)
>                      (setq result decoder))
> 
>                     ;; A function that does decoding (like for
>                     ;; `xterm-mouse-mode')
>                     ((pred functionp)
>                      (setq result (funcall decoder nil))))
>               (if (zerop (length result))
>                   ;; The decoding is an empty vector to say "continue
>                   ;; reading".  This happens when the key would be
>                   ;; mouse-movement but `track-mouse' is nil.
>                   (setq keys '()
>                         decoder input-decode-map)
>                 (throw 'read-decoded-key nil))))
>         (cancel-timer timer)))
> 
>     ;; If no decoding, the accumulated keys are the result.
>     (or result (vconcat (nreverse keys)))))
> 
>>>> `function-key-map` has very similar effects (and to a large extent, 
>>>> the
>>>> downgrading of mouse-down events controlled by `all-mouse-events` 
>>>> could
>>>> (I'd even say should) be implemented in `function-key-map` rather 
>>>> than
>>>> via the current ad-hoc code in read-key-sequence), so I'm not very
>>>> comfortable with treating these mouse-event-rewritings differently 
>>>> from
>>>> other rewritings.
>>> Just a few comments:
>>> Wouldn't that require binding 2^6 * 3 * 3 * 5 = 2880 events in
>>> function-key-map?
>> 
>> Yes, but that's only because of the limited form available in keymaps.
>> To make it practical, we'd need to add "computed keymaps".  This is
>> a long-standing desire of mine, which would be useful in various
>> circumstances (and should make it possible to remove a lot of ad-hoc
>> rewritings in read_key_sequence).
> 
> Makes sense.  I think the easiest way to do this would be to allow
> keymaps to have multiple conditional fallbacks.  Perhaps allow a
> binding in a keymap to be a hook that runs via
> `run-hook-with-args-until-success'?  It's a slight generalization of
> the current logic which allows functions.
> 
> Technically this can be done now with just a layer of indirection on
> the [t] binding in a keymap.
> 
>>> And such behavior would want a special variable (as the code is 
>>> currently in
>>> my patch) to disable it to avoid copying all of function-key-map in
>>> read-key.  So I think it is fully independent of my current patch.
>> 
>> Yes.  My point is just that a functionally "don't discard 
>> mouse-events"
>> is weird in a function which is not specifically about mouse events.
>> It naturally leads to "don't down case events", "don't remap `tab` to
>> TAB", etc...
>> There has to be a more general underlying principle.
>> 
>> Maybe we could (re)use the DONT-DOWNCASE-LAST arg of 
>> `read-key-sequence`
>> for that?  This would mean no change in `read-key` but a change in
>> `read-key-sequence` instead (and hence further-reaching consequences).
>> 
>> Or maybe an option to `read-key` to disable all
>> function-key-map-like remappings (i.e. remappings which are only 
>> applied
>> if there's no binding for that key-sequence)?
> 
> One way to do this would be to add new behavior if DONT-DOWNCASE-LAST
> is a list.  In other words, if DONT-DOWNCASE-LAST is non-nil and not a
> list, existing behavior.  But if DONT-DOWNCASE-LAST is a list, we can
> look at the members of the list to determine what to do.  This is a
> slight change in existing behavior, but it is very unlikely to break
> anything.
> 
>   -- MJF