Re: Additional cleanup around xterm-mouse

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

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