Subject: Re: bug#60867: 29.0.60; keymap-set-after does not accept the AFTER=t argument

From: Robert Pluim <[email-masked]>
Date: Tue, 17 Jan 2023 18:13:33 +0100

>>>>> On Mon, 16 Jan 2023 19:20:24 +0100, Daniel Mendler <[email-masked]> said:

    Daniel> The docstring of `keymap-set-after' says that AFTER=t should be
    Daniel> accepted, but it fails because of the `keymap--check'.

True. But the problems go deeper than that. `keymap-set-after' does
this:

  (define-key-after keymap (key-parse key) definition
    (and after (key-parse after))))

So passing eg "C-a" as `after' results in

(key-parse "C-a") => [1]

being passed to `define-key-after'. But the events in keymaps for keys
are not vectors, theyʼre integers, so this can never actually work
unless we do something like this:

diff --git c/lisp/keymap.el i/lisp/keymap.el
index 315eaab7560..a0575cd1b9f 100644
--- c/lisp/keymap.el
+++ i/lisp/keymap.el
@@ -186,10 +186,10 @@ keymap-set-after
   (declare (indent defun)
            (compiler-macro (lambda (form) (keymap--compile-check key) form)))
   (keymap--check key)
-  (when after
-    (keymap--check after))
-  (define-key-after keymap (key-parse key) definition
-    (and after (key-parse after))))
+  (when (and after (not (eq after t)))
+    (keymap--check after)
+    (setq after (aref (key-parse after) 0)))
+  (define-key-after keymap (key-parse key) definition after))
 
 (defun key-parse (keys)
   "Convert KEYS to the internal Emacs key representation.

but that makes me wonder what else Iʼm missing, since that feels kind
of wrong. Hmm, maybe we should be taking the last element there, not
the first?

    Daniel> Then I've observed that `keymap-lookup' uses `kbd'. Shouldn't
    Daniel> `key-parse' be used instead?

`kbd' supports a slightly less strict syntax than `key-parse' for
historical reasons. Itʼs not something I think we should change
without good reason. Certainly not in emacs-29

Robert
--