Re: Additional cleanup around xterm-mouse

[Jared Finder]

On 2020-11-21 1:31 am, Eli Zaretskii wrote:
> > Date: Thu, 19 Nov 2020 00:03:30 -0800
> > From: Jared Finder <jared@finder.org>
> > Cc: emacs-devel@gnu.org
> >
> > > I don't think I follow.  All the places where you need changes are
> > > related to handling mouse events, so why cannot it be specific to
> > > xt-mouse?
> >
> > There is a bug in widget-key-sequence-read-event's usage of keyboard
> > events specifically.
> >
> > The function widget-key-sequence-read-event currently does not 
> > correctly
> > translate function keys for the terminal.  It has code that attempts 
> > to
> > apply function-key-map, but does not apply input-decode-map, so it can
> > not read function keys.  (Additionally, it should be using
> > local-function-key-map now.)
>
> OK, but that means widget-key-sequence-read-event has a bug that needs
> to be fixed regardless.
>
> So I'm still asking why can't we do something like
>
>   (if xterm-mouse-mode
>       (read-key)
>     (read-event))
>
> in all the affected places that currently call read-event?

This can be done in all places except widget-key-sequence-read-event 
(see below for an explanation).  Though it feels like a bad solution as 
the info pages clearly state right now that if you want to read 
translated events, you should use read-key, not read-event.  Mouse 
events always could be translated because of xterm-mouse-mode, so isn't 
this just a bug?

I completely agree with the goal to make sure to preserve backward 
compatibility issues, but I think a better approach would be to add the 
additional functionality to read-key to make it a safe replacement for 
read-event.  (This seems aligned with how I read Stefan's recent comment 
on this thread as well.)


By the way, the info text I'm referring to is the following excerpt from 
(elisp)Top > Command Loop > Reading Input > Reading One Event.

   We emphasize that, unlike ‘read-key-sequence’, the functions
‘read-event’, ‘read-char’, and ‘read-char-exclusive’ do not perform the
translations described in Translation Keymaps.  If you wish to
read a single key taking these translations into account, use the
function ‘read-key’:

> So I'm still wary of making such a
> significant change, even though we are talking about a small number of
> relatively minor feature (with the sole exception of Ediff, perhaps).

Note: the wid-edit.el change is also pretty major and where I first 
noticed this.  Without some fix there, interaction with widgets in 
Customize buffers with an xterm mouse will cause infinite loops that 
only C-g can escape.

> >    (let ((ev2 (and (memq 'down (event-modifiers ev))
> > -                 (read-event)))
> > -       (tr (and (keymapp function-key-map)
> > -                (lookup-key function-key-map (vector ev)))))
> > +                 (read-key)))
> > +        ;; This is actually a separate bug-fix.  `function-key-map'
> > +        ;; does not contain any term-specific function key mappings
> > +        ;; like f13 --> S-f1.
> > +        (tr (and (keymapp local-function-key-map)
> > +                (lookup-key local-function-key-map (vector ev)))))
>
> Let's fix this part separately.

The underlying issue with function keys is the same as the xterm-mouse 
issue (failure to apply input-decode-map), so the solutions may be 
intertwined.  Terminal function keys and xterm mouse events both get 
decoded through input-decode-map.

Note that because function keys are also affected, for this function the 
check whether to use read-key or read-event would need to be something 
like:

(if (eq window-system nil) ; This is correct on Linux and macOS, not 
sure
                           ; if accurate for other platforms
    (read-key)
  (read-event))

Without the change from read-event to read-key throughout this function, 
'ev' will never contain an event that would be translated by 
local-function-key-map or function-key-map from what I've seen.  It may 
be a better path to convert this function to just call (read-key 
"Prompt" t) like the rest of the modifications.  If this is done, then 
local-function-key-map is already applied.  I realize that is a behavior 
change, but given how fragile reconstructing the way translation maps 
already is, I think it is worth considering.

  -- MJF