bug#62626: 29.0.60; describe-key errors on Edit → Paste from Kill Ring → [any item]

[Eli Zaretskii <eliz@gnu.org>]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Sun, 02 Apr 2023 14:49:41 -0400
> 
> 
> 1. emacs -Q
> 2. C-h k and in the menu bar, click on "Edit", then "Paste from Kill
> Menu", then "(any string)".
> 3. See error printed in *Messages*:
> event-basic-type: Wrong type argument: integer-or-marker-p, "(any string)"
> 4. Type "foobar" and kill it
> 5. C-h k and in the menu bar, click on "Edit", then "Paste from Kill
> Menu", then "foobar".
> 6. See error printed in *Messages*:
> event-basic-type: Wrong type argument: integer-or-marker-p, #("foobar" 0 6 (fontified t))
> 
> This happens on both Emacs 28 and Emacs 29.

AFAICT, this has been broken since Emacs 27.

> It looks like this is happening because help--read-key-sequence calls
> event-basic-type on the last element of the key sequence returned from
> read-key-sequence, which in this case is a string.

Yes.

> The following patch fixes it (that is, makes describe-key complete
> successfully and show appropriate help), but I'm not sure if it's right.
> 
> diff --git a/lisp/help.el b/lisp/help.el
> index 83be85b1ee4..73ac793f4aa 100644
> --- a/lisp/help.el
> +++ b/lisp/help.el
> @@ -1021,8 +1021,8 @@ help--read-key-sequence
>                     (raw-seq (this-single-command-raw-keys))
>                     (keyn (when (> (length seq) 0)
>                             (aref seq (1- (length seq)))))
> -                   (base (event-basic-type keyn))
> -                   (modifiers (event-modifiers keyn)))
> +                   (base (and (integer-or-marker-p keyn) (event-basic-type keyn)))
> +                   (modifiers (and (integer-or-marker-p keyn) (event-modifiers keyn))))
>                (cond
>                 ((zerop (length seq)))   ;FIXME: Can this happen?
>                 ((and no-mouse-movement (eq base 'mouse-movement)) nil)

Doesn't the above break "C-h c" and "C-h k" for mouse click events?
They yield a symbol like down-mouse-1 as KEYN, so are rejected by your
proposed condition.

I suggest the below instead.  I'm just not sure about what to do with
RAW-SEQ (which is returned as the cdr of the cons cell value of this
function).  Emacs 26 returned nil there, so we are "bug-compatible"
with it after applying the below.  But since this is a general-purpose
utility function, perhaps we should return the same value as SEQ
there?  I mean, what does "untranslated" mean for such "events"?

An alternative solution for the original problem would be to teach
event-basic-type and event-modifiers about "events" that happen to be
strings.

Stefan, any comments or better ideas?

diff --git a/lisp/help.el b/lisp/help.el
index 6eac037..299042f 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1012,7 +1012,7 @@ help--read-key-sequence
     (unwind-protect
         (let (last-modifiers key-list)
           ;; If yank-menu is empty, populate it temporarily, so that
-          ;; "Select and Paste" menu can generate a complete event.
+          ;; "Paste from Kill Menu" menu can generate a complete event.
           (when (null (cdr yank-menu))
             (setq saved-yank-menu (copy-sequence yank-menu))
             (menu-bar-update-yank-menu "(any string)" nil))
@@ -1031,8 +1031,14 @@ help--read-key-sequence
                    (raw-seq (this-single-command-raw-keys))
                    (keyn (when (> (length seq) 0)
                            (aref seq (1- (length seq)))))
-                   (base (event-basic-type keyn))
-                   (modifiers (event-modifiers keyn)))
+                   ;; The "Paste from Kill Menu" menu-bar item has the
+                   ;; text-to-yank, a string, as its last "event"
+                   ;; component, and those are not supported by
+                   ;; 'event-basic-type' and 'event-modifiers'.
+                   (str-from-menu (stringp keyn))
+                   (base (and (not str-from-menu) (event-basic-type keyn)))
+                   (modifiers (and (not str-from-menu)
+                                   (event-modifiers keyn))))
               (cond
                ((zerop (length seq)))   ;FIXME: Can this happen?
                ((and no-mouse-movement (eq base 'mouse-movement)) nil)