bug#29478: [Patch] bug#29478: 26.0.90; `C-h k' followed by mouse clicks no longer shows down event

[Stefan Monnier]

>> +(defun help--first-event (keyseq)
>> +  (when (> (length keyseq) 0)
>> +    (aref keyseq (if (and (symbolp (aref keyseq 0))
>> +                      (> (length keyseq) 1)
>> +                      (consp (aref keyseq 1)))
>> +                     ;; Look at the second event when the first
>> +                     ;; is a pseudo-event like `mode-line' of `left-fringe'.
>> +                 1
>> +               0))))
>
> Should we perhaps test explicitly for pseudo-events, in case some day
> we have events we don't want to skip?  Or maybe have an assertion
> here, for that future day?

[ This code was moved from help--analyze-key, so improving it is
  orthogonal to the patch under discussion.  Especially now that I see
  that the reason why I extracted this function (to use it in
  help-read-key-sequence) doesn't apply any more.  ]

You mean we should check that (aref keyseq 0) is within
(mode-line left-fringe right-fringe ...)?

I guess we could but what could we do with the result of the check?
Signaling an error would not be very useful to the user.

Note that the output of this function is only used to extract the
modifiers of the event to then decide whether we add the text "at that
spot" or not, so it's really not important.

>> -(defun describe-key-briefly (&optional key insert untranslated)
>> -  "Print the name of the function KEY invokes.  KEY is a string.
>> +(defun describe-key-briefly (key-list &optional insert)
>> +  "Print the name of the functions KEY-LIST invokes.
>
> Is it a good idea to change the signature of this function?

It's indeed the main issue I see with the patch: it completely changes
the signature of several functions.

> I'm sure it's used in many places, including outside Emacs.

I couldn't find any use of those functions outside of Emacs (I did find
uses as commands, but not as functions).  But indeed I now see that
there are some uses within Emacs, which I'd need to fix.

I'll change the new code to accept the old calling convention.

>>  (defun help-read-key-sequence (&optional no-mouse-movement)
>> -  "Reads a key sequence from the user.
>> -Returns a list of the form (KEY UP-EVENT), where KEY is the key
>> -sequence, and UP-EVENT is the up-event that was discarded by
>> -reading KEY, or nil.
>> +  "Read \"a\" key sequence from the user.
>
> Why "a" in quotes?

Because it can also return several key sequences.  But now that you
mention it, it's better not to put those quotes and simply explain it
better in the subsequent text.

>> +Return a list of elements of the form (SEQ . RAW-SEQ), where SEQ is a key
>> +sequence, and RAW-SEQ is its untranslated form.
> This also changes the format of the return value in incompatible way,
> doesn't it?

Yes.

> Is this incompatible change justified?

Yes: we can't recover all the RAW-SEQs after the call (only the
RAW-SEQ of the last element which can still be obtained from
this-single-command-raw-keys), so the old return value format throws
away information we need to emit complete information.

In any case, this function is (or at least should be) internal, so I now
renamed it with a "help--" prefix.  If external callers show up, we may
have to revisit this, but I couldn't find any.

>> -(defun describe-key (&optional key untranslated up-event)
>> -  "Display documentation of the function invoked by KEY.
>> -KEY can be any kind of a key sequence; it can include keyboard events,
>> +(defun describe-key (key-list &optional buffer)
>> +  "Display documentation of the function invoked by KEY-LIST.
>> +KEY-LIST can be any kind of a key sequence; it can include keyboard events,
>>  mouse events, and/or menu events.  When calling from a program,
> Same issue here.

I'll also add compatibility to the old calling convention.

> > We can probably come up with some heuristic to keep "at least one line
> > of output" or something, but I think it's more useful for the user to
> > report all the events and their binding or lack thereof since we don't
> > really know what the user is looking for.

> I'd prefer the "undefined" lines to appear only if none of the
> possible interpretations is bound to a command, yes.

Not sure what this means.


        Stefan