bug#30309: Surprising behaviour of insert-kbd-macro

bug#30309: Surprising behaviour of insert-kbd-macro

[Robert Pluim]

#emacs-26
emacs -Q
C-u M-x insert-kbd-macro RET RET
=>
(setq last-kbd-macro
   nil)
(global-set-key [C-down-mouse-2 s2] 'last-kbd-macro)
(global-set-key [C-down-mouse-2 s1] 'last-kbd-macro)
(global-set-key [menu-bar help-menu sep1] 'last-kbd-macro)
(global-set-key [menu-bar help-menu sep2] 'last-kbd-macro)
(global-set-key [menu-bar help-menu sep4] 'last-kbd-macro)
(global-set-key [menu-bar file separator-save] 'last-kbd-macro)
(global-set-key [menu-bar file separator-print] 'last-kbd-macro)
(global-set-key [menu-bar file separator-window] 'last-kbd-macro)
(global-set-key [menu-bar file separator-frame] 'last-kbd-macro)
(global-set-key [menu-bar file separator-exit] 'last-kbd-macro)
(global-set-key [menu-bar edit separator-search] 'last-kbd-macro)
(global-set-key [menu-bar edit separator-bookmark] 'last-kbd-macro)
(global-set-key [menu-bar options highlight-separator] 'last-kbd-macro)
(global-set-key [menu-bar options edit-options-separator] 'last-kbd-macro)
(global-set-key [menu-bar options cursor-separator] 'last-kbd-macro)
(global-set-key [menu-bar options debugger-separator] 'last-kbd-macro)
(global-set-key [menu-bar options mule-separator] 'last-kbd-macro)
(global-set-key [menu-bar options showhide-separator] 'last-kbd-macro)
(global-set-key [menu-bar options custom-separator] 'last-kbd-macro)
(global-set-key [menu-bar buffer command-separator] 'last-kbd-macro)
(global-set-key [menu-bar tools separator-prog] 'last-kbd-macro)
(global-set-key [menu-bar tools separator-spell] 'last-kbd-macro)
(global-set-key [menu-bar tools separator-compare] 'last-kbd-macro)
(global-set-key [menu-bar tools separator-vc] 'last-kbd-macro)
(global-set-key [menu-bar tools separator-net] 'last-kbd-macro)
(global-set-key [menu-bar tools separator-encryption-decryption] 'last-kbd-macro)
(global-set-key [menu-bar tools separator-games] 'last-kbd-macro)
(global-set-key [menu-bar help-menu search-documentation sep1] 'last-kbd-macro)
(global-set-key [menu-bar help-menu describe separator-desc-mule] 'last-kbd-macro)
(global-set-key [menu-bar help-menu more-manuals sep2] 'last-kbd-macro)
(global-set-key [menu-bar edit search separator-repeat-search] 'last-kbd-macro)
(global-set-key [menu-bar edit search separator-tag-search] 'last-kbd-macro)
(global-set-key [menu-bar edit search separator-tag-isearch] 'last-kbd-macro)
(global-set-key [menu-bar edit replace separator-replace-tags] 'last-kbd-macro)
(global-set-key [menu-bar edit goto separator-xref] 'last-kbd-macro)
(global-set-key [menu-bar edit goto separator-tag-file] 'last-kbd-macro)
(global-set-key [menu-bar edit props s2] 'last-kbd-macro)
(global-set-key [menu-bar edit props s1] 'last-kbd-macro)
(global-set-key [menu-bar options search-options custom-separator] 'last-kbd-macro)
(global-set-key [menu-bar options mule separator-mule] 'last-kbd-macro)
(global-set-key [menu-bar options mule separator-input-method] 'last-kbd-macro)
(global-set-key [menu-bar options mule separator-coding-system] 'last-kbd-macro)
(global-set-key [menu-bar options showhide datetime-separator] 'last-kbd-macro)
(global-set-key [menu-bar options showhide linecolumn-separator] 'last-kbd-macro)
(global-set-key [menu-bar options customize separator-3] 'last-kbd-macro)
(global-set-key [menu-bar options customize separator-2] 'last-kbd-macro)
(global-set-key [menu-bar options customize separator-1] 'last-kbd-macro)
(global-set-key [menu-bar tools compare separator-ediff-files] 'last-kbd-macro)
(global-set-key [menu-bar tools compare separator-ediff-directories] 'last-kbd-macro)
(global-set-key [menu-bar tools compare separator-ediff-regions] 'last-kbd-macro)
(global-set-key [menu-bar tools compare separator-ediff-windows] 'last-kbd-macro)
(global-set-key [menu-bar tools compare separator-ediff-misc] 'last-kbd-macro)
(global-set-key [menu-bar tools ediff-merge separator-ediff-merge-dirs] 'last-kbd-macro)
(global-set-key [menu-bar tools ediff-merge separator-ediff-merge] 'last-kbd-macro)
(global-set-key [menu-bar tools vc separator2] 'last-kbd-macro)
(global-set-key [menu-bar tools vc separator1] 'last-kbd-macro)
(global-set-key [menu-bar tools directory-search separator-eudc-query] 'last-kbd-macro)
(global-set-key [menu-bar tools directory-search separator-eudc-email] 'last-kbd-macro)
(global-set-key [menu-bar tools encryption-decryption separator-file] 'last-kbd-macro)
(global-set-key [menu-bar tools encryption-decryption separator-keys] 'last-kbd-macro)
(global-set-key [menu-bar options mule set-various-coding-system separator-1] 'last-kbd-macro)
(global-set-key [menu-bar options mule set-various-coding-system separator-2] 'last-kbd-macro)
(global-set-key [menu-bar options mule set-various-coding-system separator-3] 'last-kbd-macro)
(global-set-key [menu-bar options showhide showhide-scroll-bar scrollbar-separator] 'last-kbd-macro)

bug#30309: Surprising behaviour of insert-kbd-macro

[Noam Postavsky]

tags 30309 + confirmed easy
retitle 30309 C-u M-x insert-kbd-macro RET RET inserts many unrelated "bindings" (menu separators?)
quit

Robert Pluim <rpluim@gmail.com> writes:

> #emacs-26
> emacs -Q
> C-u M-x insert-kbd-macro RET RET
> =>
> (setq last-kbd-macro
>    nil)
> (global-set-key [C-down-mouse-2 s2] 'last-kbd-macro)
> (global-set-key [C-down-mouse-2 s1] 'last-kbd-macro)
> (global-set-key [menu-bar help-menu sep1] 'last-kbd-macro)
[...]

Yeah, I guess no keybindings should be inserted in that case.

bug#30309: Surprising behaviour of insert-kbd-macro

[Robert Pluim]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> tags 30309 + confirmed easy
> retitle 30309 C-u M-x insert-kbd-macro RET RET inserts many unrelated "bindings" (menu separators?)
> quit
>
> Robert Pluim <rpluim@gmail.com> writes:
>
>> #emacs-26
>> emacs -Q
>> C-u M-x insert-kbd-macro RET RET
>> =>
>> (setq last-kbd-macro
>>    nil)
>> (global-set-key [C-down-mouse-2 s2] 'last-kbd-macro)
>> (global-set-key [C-down-mouse-2 s1] 'last-kbd-macro)
>> (global-set-key [menu-bar help-menu sep1] 'last-kbd-macro)
> [...]
>
> Yeah, I guess no keybindings should be inserted in that case.

So I guess just checking for last-kbd-macro should be enough, since
that should never have a legitimate key binding.

diff --git i/lisp/macros.el w/lisp/macros.el
index 4078b983ec..53a688a367 100644
--- i/lisp/macros.el
+++ w/lisp/macros.el
@@ -125,7 +125,7 @@ insert-kbd-macro
         ;; (kmacro-create [<keys>] 0 "%d").
 	(prin1 definition (current-buffer))))
     (insert ")\n")
-    (if keys
+    (and keys (not (eq macroname 'last-kbd-macro))
         (let ((keys (or (where-is-internal (symbol-function macroname)
                                            '(keymap))
                         (where-is-internal macroname '(keymap)))))

bug#30309: Surprising behaviour of insert-kbd-macro

[Noam Postavsky]

On Fri, 30 Nov 2018 at 03:37, Robert Pluim <rpluim@gmail.com> wrote:

> So I guess just checking for last-kbd-macro should be enough, since
> that should never have a legitimate key binding.

I think avoiding passing nil to where-is-internal would be cleaner (or
else stop where-is-internal from looking for bindings of nil). And it
would handle the non-interactive case of

(insert-kbd-macro 'something-not-fbound t)

bug#30309: Surprising behaviour of insert-kbd-macro

[Robert Pluim]

>>>>> On Fri, 29 Mar 2019 10:16:50 -0400, Noam Postavsky <npostavs@users.sourceforge.net> said:

    Noam> On Fri, 30 Nov 2018 at 03:37, Robert Pluim <rpluim@gmail.com> wrote:
    >> So I guess just checking for last-kbd-macro should be enough,
    >> since that should never have a legitimate key binding.

    Noam> I think avoiding passing nil to where-is-internal would be
    Noam> cleaner (or else stop where-is-internal from looking for
    Noam> bindings of nil). And it would handle the non-interactive
    Noam> case of

    Noam> (insert-kbd-macro 'something-not-fbound t)

That would work as well. Something like this (utterly untested beyond
'insert-kbd-macro'):

diff --git a/src/keymap.c b/src/keymap.c
index 2ac3d33460..2e9f3d5a96 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -2489,6 +2489,7 @@ where_is_internal (Lisp_Object definition, Lisp_Object keymaps,
 
 DEFUN ("where-is-internal", Fwhere_is_internal, Swhere_is_internal, 1, 5, 0,
        doc: /* Return list of keys that invoke DEFINITION.
+If DEFINITION is nil, return nil.
 If KEYMAP is a keymap, search only KEYMAP and the global keymap.
 If KEYMAP is nil, search all the currently active keymaps, except
  for `overriding-local-map' (which is ignored).
@@ -2536,6 +2537,9 @@ The optional 5th arg NO-REMAP alters how command remapping is handled:
   bool remapped = 0;
   Lisp_Object tem = Qnil;
 
+  if (NILP (definition))
+    return Qnil;
+
   /* Refresh the C version of the modifier preference.  */
   where_is_preferred_modifier
     = parse_solitary_modifier (Vwhere_is_preferred_modifier);

bug#30309: Surprising behaviour of insert-kbd-macro

[Noam Postavsky]

Robert Pluim <rpluim@gmail.com> writes:

> That would work as well. Something like this (utterly untested beyond
> 'insert-kbd-macro'):

>  DEFUN ("where-is-internal", Fwhere_is_internal, Swhere_is_internal, 1, 5, 0,
>         doc: /* Return list of keys that invoke DEFINITION.
> +If DEFINITION is nil, return nil.

> +  if (NILP (definition))
> +    return Qnil;
> +

Hmm, on the one hand no keys invoke 'nil', so returning nil in that
situation is already covered by the current docstring (and doing
something else could be considered a plain bug).  On the other hand, I
see it returns keys for which (define-key global-map KEY nil) was
called, which is potentially useful.  I don't know if anything uses
that, but I hesitate to remove the capability.

By the way, I noticed the diff below also fixes this bug, although I
don't quite understand why.

--- i/lisp/macros.el
+++ w/lisp/macros.el
@@ -127,7 +127,7 @@ (defun insert-kbd-macro (macroname &optional keys)
     (insert ")\n")
     (if keys
         (let ((keys (or (where-is-internal (symbol-function macroname)
-                                           '(keymap))
+                                           '(keymap) nil t)
                         (where-is-internal macroname '(keymap)))))
 	  (while keys
 	    (insert "(global-set-key ")

bug#30309: Surprising behaviour of insert-kbd-macro

[Robert Pluim]

>>>>> On Mon, 15 Apr 2019 20:23:19 -0400, Noam Postavsky <npostavs@gmail.com> said:

    Noam> Robert Pluim <rpluim@gmail.com> writes:
    >> That would work as well. Something like this (utterly untested
    >> beyond 'insert-kbd-macro'):

    >> DEFUN ("where-is-internal", Fwhere_is_internal,
    >> Swhere_is_internal, 1, 5, 0, doc: /* Return list of keys that
    >> invoke DEFINITION.  +If DEFINITION is nil, return nil.

    >> + if (NILP (definition)) + return Qnil; +

    Noam> Hmm, on the one hand no keys invoke 'nil', so returning nil
    Noam> in that situation is already covered by the current
    Noam> docstring (and doing something else could be considered a
    Noam> plain bug).  On the other hand, I see it returns keys for
    Noam> which (define-key global-map KEY nil) was called, which is
    Noam> potentially useful.  I don't know if anything uses that, but
    Noam> I hesitate to remove the capability.

I donʼt know either, so best leave it alone.

    Noam> By the way, I noticed the diff below also fixes this bug,
    Noam> although I don't quite understand why.

That results in

    (setq last-kbd-macro
       nil)
    (global-set-key [S-down-mouse-1] 'last-kbd-macro)
    (global-set-key [horizontal-scroll-bar drag-mouse-1] 'last-kbd-macro)
    (global-set-key [horizontal-scroll-bar mouse-1] 'last-kbd-macro)
    (global-set-key [vertical-scroll-bar drag-mouse-1] 'last-kbd-macro)
    (global-set-key [vertical-scroll-bar mouse-1] 'last-kbd-macro)

which I guess is better, but not perfect.

Robert

bug#30309: C-u M-x insert-kbd-macro RET RET inserts many unrelated "bindings" (menu separators?)

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> #emacs-26
> emacs -Q
> C-u M-x insert-kbd-macro RET RET
> =>
> (setq last-kbd-macro
>    nil)
> (global-set-key [C-down-mouse-2 s2] 'last-kbd-macro)
> (global-set-key [C-down-mouse-2 s1] 'last-kbd-macro)
> (global-set-key [menu-bar help-menu sep1] 'last-kbd-macro)

I've now fixed this in Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no