From 06953fc8e1d70397f421e1c6efc327af8e0bad6c Mon Sep 17 00:00:00 2001 From: Robert Pluim Date: Thu, 19 Jan 2023 16:05:45 +0100 Subject: [PATCH] Make `keymap-set-after' work for menus It still doesn't work for an AFTER that's a key, though, since `key-parse' produces vectors, and keymaps contain integers. * lisp/keymap.el (keymap-set-after): Only parse AFTER as a key if it's a string. For consistency, use `key-parse' on the definition if it's a string, just like `keymap-set'. * test/src/keymap-tests.el (keymap-tests--command-3): New dummy command. (keymap-set-after-menus): New test. Check that we can insert a menu item after a specific entry. --- lisp/keymap.el | 12 +++++++++--- test/src/keymap-tests.el | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lisp/keymap.el b/lisp/keymap.el index 89461416a9c..791221f2459 100644 --- a/lisp/keymap.el +++ b/lisp/keymap.el @@ -187,10 +187,16 @@ a menu, so this function is not useful for non-menu keymaps." (compiler-macro (lambda (form) (keymap--compile-check key) form))) (keymap--check key) (when (eq after t) (setq after nil)) ; nil and t are treated the same - (when after - (keymap--check after)) + (when (stringp after) + (keymap--check after) + (setq after (key-parse after))) + ;; If we're binding this key to another key, then parse that other + ;; key, too. + (when (stringp definition) + (keymap--check definition) + (setq definition (key-parse definition))) (define-key-after keymap (key-parse key) definition - (and after (key-parse after)))) + after)) (defun key-parse (keys) "Convert KEYS to the internal Emacs key representation. diff --git a/test/src/keymap-tests.el b/test/src/keymap-tests.el index d7be2cac53a..aa710519825 100644 --- a/test/src/keymap-tests.el +++ b/test/src/keymap-tests.el @@ -226,6 +226,7 @@ commit 86c19714b097aa477d339ed99ffb5136c755a046." (defun keymap-tests--command-1 () (interactive) nil) (defun keymap-tests--command-2 () (interactive) nil) +(defun keymap-tests--command-3 () (interactive) nil) (put 'keymap-tests--command-1 :advertised-binding [?y]) (ert-deftest keymap-where-is-internal () @@ -446,6 +447,22 @@ g .. h foo (should-not (keymap-set-after k "f" "f" "a")) (should (equal (keymap-lookup k "f") (key-parse "f"))))) +(ert-deftest keymap-set-after-menus () + (let ((map (make-sparse-keymap))) + (keymap-set map "" + '(menu-item "Run Command 1" keymap-tests--command-1 + :help "Command 1 Help")) + (keymap-set-after map "" + '(menu-item "Run Command 2" keymap-tests--command-2 + :help "Command 2 Help")) + (keymap-set-after map "" + '(menu-item "Run Command 3" keymap-tests--command-3 + :help "Command 3 Help") + 'cmd1) + (should (equal (caadr map) 'cmd1)) + (should (equal (caaddr map) 'cmd3)) + (should (equal (caar (last map)) 'cmd2)))) + (ert-deftest keymap-test-duplicate-definitions () "Check that defvar-keymap rejects duplicate key definitions." (should-error -- 2.39.2