bug#61700: 30.0.50; insert-kbd-macro fails for named macro but not last macro

bug#61700: 30.0.50; insert-kbd-macro fails for named macro but not last macro

[Josh Moller-Mara]

Using “M-x insert-kbd-macro” can insert an incorrect kmacro form for a
named macro, while still correctly inserting the “last-kbd-macro”.

To see this, we’ll define a macro that simply inserts the HTML string
“<b>hello</b>” (press F3, type the previous string, then press F4).
We’ll name this macro “bold-hello” using “C-x C-k n”.

Inserting the definition to the last macro (using “M-x insert-kbd-macro”
and leaving the prompt blank) returns:

(setq last-kbd-macro
   (kmacro "< b > H e l l o < / b >"))

Inserting the definition to “bold-hello” (using “M-x insert-kbd-macro”
and typing “bold-hello” at the prompt) returns:

(defalias 'bold-hello
   (kmacro "<b> H e l l o < / b >"))

Notice the difference in “< b >” vs “<b>”.  The “bold-hello” command
doesn’t work, printing the message:

“After 0 kbd macro iterations: Keyboard macro terminated by a command ringing the bell”

I think this may have to do with the use of “key-parse” in “kmacro” (see
“(find-function 'kmacro)”).

(key-parse "<b>hello</b>") returns
“[b 104 101 108 108 111 60 47 98 62]”
but
(macro--string-to-vector "<b>hello</b>") returns
“[60 98 62 104 101 108 108 111 60 47 98 62]”

The latter form is used in “insert-kbd-macro” when inserting
“last-kbd-macro”.


You can also quickly replicate the issue by evaluating the following
form:

(progn
  (setq last-kbd-macro "<b>hello</b>")
  (kmacro-name-last-macro 'bold-hello)
  (let ((temp-buffer (generate-new-buffer "*macros*")))
    (with-current-buffer temp-buffer
      (insert-kbd-macro (intern ""))
      (insert-kbd-macro 'bold-hello))
    (display-buffer temp-buffer)))




In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.35, cairo version 1.16.0)
Repository revision: 2c7e87c73a90effab66fa7c783855199880315d3
Repository branch: master
System Description: NixOS 22.11 (Raccoon)

Configured using:
 'configure
 --prefix=/nix/store/bdg6d4gm3ma2hja63hbxwrkaap04j2c6-emacs-pgtk-20230217.0
 --disable-build-details --with-modules --with-pgtk
 --with-native-compilation'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER
PGTK PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP XIM GTK3 ZLIB

bug#61700: 30.0.50; insert-kbd-macro fails for named macro but not last macro

[Robert Pluim]

>>>>> On Tue, 21 Feb 2023 17:18:16 -0800, Josh Moller-Mara <jmm@cns.nyu.edu> said:

    Josh> Using “M-x insert-kbd-macro” can insert an incorrect kmacro form for a
    Josh> named macro, while still correctly inserting the “last-kbd-macro”.

    Josh> To see this, we’ll define a macro that simply inserts the HTML string
    Josh> “<b>hello</b>” (press F3, type the previous string, then press F4).
    Josh> We’ll name this macro “bold-hello” using “C-x C-k n”.

    Josh> Inserting the definition to the last macro (using “M-x insert-kbd-macro”
    Josh> and leaving the prompt blank) returns:

    Josh> (setq last-kbd-macro
    Josh>    (kmacro "< b > H e l l o < / b >"))

    Josh> Inserting the definition to “bold-hello” (using “M-x insert-kbd-macro”
    Josh> and typing “bold-hello” at the prompt) returns:

    Josh> (defalias 'bold-hello
    Josh>    (kmacro "<b> H e l l o < / b >"))

    Josh> Notice the difference in “< b >” vs “<b>”.  The “bold-hello” command
    Josh> doesn’t work, printing the message:

    Josh> “After 0 kbd macro iterations: Keyboard macro terminated by a command ringing the bell”

    Josh> I think this may have to do with the use of “key-parse” in “kmacro” (see
    Josh> “(find-function 'kmacro)”).

    Josh> (key-parse "<b>hello</b>") returns
    Josh> “[b 104 101 108 108 111 60 47 98 62]”
    Josh> but
    Josh> (macro--string-to-vector "<b>hello</b>") returns
    Josh> “[60 98 62 104 101 108 108 111 60 47 98 62]”

    Josh> The latter form is used in “insert-kbd-macro” when inserting
    Josh> “last-kbd-macro”.

This fails in emacs-29 as well. Stefan, you rewrote this code to
use oclosures, any ideas? I guess we could just use
macro--string-to-vector in both cases, but youʼre the expert here :-)

    Josh> You can also quickly replicate the issue by evaluating the following
    Josh> form:

    Josh> (progn
    Josh>   (setq last-kbd-macro "<b>hello</b>")
    Josh>   (kmacro-name-last-macro 'bold-hello)
    Josh>   (let ((temp-buffer (generate-new-buffer "*macros*")))
    Josh>     (with-current-buffer temp-buffer
    Josh>       (insert-kbd-macro (intern ""))
    Josh>       (insert-kbd-macro 'bold-hello))
    Josh>     (display-buffer temp-buffer)))

Robert
-- 

bug#61700: 30.0.50; insert-kbd-macro fails for named macro but not last macro

[Robert Pluim]

>>>>> On Thu, 23 Feb 2023 10:21:05 +0100, Robert Pluim <rpluim@gmail.com> said:

    Robert> This fails in emacs-29 as well. Stefan, you rewrote this code to
    Robert> use oclosures, any ideas? I guess we could just use
    Robert> macro--string-to-vector in both cases, but youʼre the expert here :-)

The following passes 'make bootstrap' and 'make check'.

Iʼve probably committed a heinous crime by autoloading an internal
function, but that seemed cleaner than "(require 'macros)" in two
places.

Robert
-- 

diff --git c/lisp/kmacro.el i/lisp/kmacro.el
index 94d8794bd23..411f981d47b 100644
--- c/lisp/kmacro.el
+++ i/lisp/kmacro.el
@@ -380,7 +380,10 @@ kmacro-view-item-no
 (defun kmacro-ring-head ()
   "Return pseudo head element in macro ring."
   (and last-kbd-macro
-       (kmacro last-kbd-macro kmacro-counter kmacro-counter-format-start)))
+       (kmacro (if (stringp last-kbd-macro)
+                   (macro--string-to-vector last-kbd-macro)
+                 last-kbd-macro)
+               kmacro-counter kmacro-counter-format-start)))
 
 
 (defun kmacro-push-ring (&optional elt)
@@ -841,8 +844,6 @@ kmacro-lambda-form
       (setq mac     (nth 0 mac)))
     (when (stringp mac)
       ;; `kmacro' interprets a string according to `key-parse'.
-      (require 'macros)
-      (declare-function macro--string-to-vector "macros")
       (setq mac (macro--string-to-vector mac)))
     (kmacro mac counter format)))
 
diff --git c/lisp/macros.el i/lisp/macros.el
index 59c7796551f..e00aedc82b0 100644
--- c/lisp/macros.el
+++ i/lisp/macros.el
@@ -46,6 +46,7 @@ macros--insert-vector-macro
                      " ")
           ?\]))
 
+;;;###autoload
 (defun macro--string-to-vector (str)
   "Convert an old-style string key sequence to the vector form."
   (let ((vec (string-to-vector str)))
diff --git c/test/lisp/kmacro-tests.el i/test/lisp/kmacro-tests.el
index 551fd8b60fc..a325220e8d9 100644
--- c/test/lisp/kmacro-tests.el
+++ i/test/lisp/kmacro-tests.el
@@ -614,6 +614,20 @@ kmacro-tests-name-last-macro-bind-and-rebind
   (kmacro-tests-should-insert "bb"
     (kmacro-tests-simulate-command '(kmacro-tests-symbol-for-test))))
 
+;; Bug#61700 inserting named macro when the definition contains things
+;; that `key-parse' thinks are named keys
+(kmacro-tests-deftest kmacro-tests-name-last-macro-key-parse-syntax ()
+  "Name last macro can rebind a symbol it binds."
+  ;; Make sure our symbol is unbound.
+  (when (fboundp 'kmacro-tests-symbol-for-test)
+    (fmakunbound 'kmacro-tests-symbol-for-test))
+  (setplist 'kmacro-tests-symbol-for-test nil)
+  (kmacro-tests-define-macro "<b> hello </>")
+  (kmacro-name-last-macro 'kmacro-tests-symbol-for-test)
+  ;; Now run the function bound to the symbol.
+  (kmacro-tests-should-insert "<b> hello </>"
+    (kmacro-tests-simulate-command '(kmacro-tests-symbol-for-test))))
+
 (kmacro-tests-deftest kmacro-tests-store-in-register ()
   "Macro can be stored in and retrieved from a register."
   (use-local-map kmacro-tests-keymap)

bug#61700: 30.0.50; insert-kbd-macro fails for named macro but not last macro

[Eli Zaretskii]

> Cc: 61700@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
> From: Robert Pluim <rpluim@gmail.com>
> Date: Thu, 23 Feb 2023 13:59:40 +0100
> 
> Iʼve probably committed a heinous crime by autoloading an internal
> function, but that seemed cleaner than "(require 'macros)" in two
> places.

Why not use an explicit autoload form in kmacro.el and kmacro-tests.el
instead?

bug#61700: 30.0.50; insert-kbd-macro fails for named macro but not last macro

[Robert Pluim]

>>>>> On Thu, 23 Feb 2023 18:00:39 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> Cc: 61700@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
    >> From: Robert Pluim <rpluim@gmail.com>
    >> Date: Thu, 23 Feb 2023 13:59:40 +0100
    >> 
    >> Iʼve probably committed a heinous crime by autoloading an internal
    >> function, but that seemed cleaner than "(require 'macros)" in two
    >> places.

    Eli> Why not use an explicit autoload form in kmacro.el and kmacro-tests.el
    Eli> instead?

Sure, that works as well (although I donʼt understand why kmacro-tests
would need an autoload if thereʼs one in kmacro.el).

Robert
-- 

bug#61700: 30.0.50; insert-kbd-macro fails for named macro but not last macro

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: jmm@cns.nyu.edu,  61700@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Thu, 23 Feb 2023 17:30:07 +0100
> 
> >>>>> On Thu, 23 Feb 2023 18:00:39 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> Cc: 61700@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
>     >> From: Robert Pluim <rpluim@gmail.com>
>     >> Date: Thu, 23 Feb 2023 13:59:40 +0100
>     >> 
>     >> Iʼve probably committed a heinous crime by autoloading an internal
>     >> function, but that seemed cleaner than "(require 'macros)" in two
>     >> places.
> 
>     Eli> Why not use an explicit autoload form in kmacro.el and kmacro-tests.el
>     Eli> instead?
> 
> Sure, that works as well (although I donʼt understand why kmacro-tests
> would need an autoload if thereʼs one in kmacro.el).

Maybe it doesn't I just assumed it might.

bug#61700: 30.0.50; insert-kbd-macro fails for named macro but not last macro

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> The following passes 'make bootstrap' and 'make check'.

Looks right to me, thank you.


        Stefan

bug#61700: 30.0.50; insert-kbd-macro fails for named macro but not last macro

[Robert Pluim]

tags 61700 fixed
close 61700 29.1
quit

>>>>> On Thu, 23 Feb 2023 13:28:58 -0500, Stefan Monnier <monnier@iro.umontreal.ca> said:

    >> The following passes 'make bootstrap' and 'make check'.
    Stefan> Looks right to me, thank you.

Closing.
Committed as 573d9675fd7

Robert
--