* lisp/macros.el (insert-kbd-macro): Recognize kmacros reliably

* lisp/macros.el (name-last-kbd-macro): Make it an alias of
kmacro-name-last-macro.
(insert-kbd-macro): Use kmacro-keyboard-macro-p so it also works for
kmacros that fail to have a `kmacro' property.
(see https://emacs.stackexchange.com/questions/37065, for an example).

* lisp/kmacro.el: Remove redundant :group args.
(kmacro-insert-counter): Simplify.
(kmacro-extract-lambda): Use car/cdr-safe.
(kmacro-p, kmacro-keyboard-macro-p): New functions.
(kmacro-name-last-macro): Use them.

2 files changed, 46 insertions, 57 deletions

diff --git a/lisp/kmacro.el b/lisp/kmacro.el
index 5729f2fc8d3..da02ab5aca4 100644
--- a/lisp/kmacro.el
+++ b/lisp/kmacro.el
@@ -124,13 +124,11 @@
 (defcustom kmacro-call-mouse-event 'S-mouse-3
   "The mouse event used by kmacro to call a macro.
 Set to nil if no mouse binding is desired."
-  :type 'symbol
-  :group 'kmacro)
+  :type 'symbol)
 
 (defcustom kmacro-ring-max 8
   "Maximum number of keyboard macros to save in macro ring."
-  :type 'integer
-  :group 'kmacro)
+  :type 'integer)
 
 
 (defcustom kmacro-execute-before-append t
@@ -141,32 +139,27 @@ execute the macro.
 Otherwise, a single \\[universal-argument] prefix does not execute the
 macro, while more than one \\[universal-argument] prefix causes the
 macro to be executed before appending to it."
-  :type 'boolean
-  :group 'kmacro)
+  :type 'boolean)
 
 
 (defcustom kmacro-repeat-no-prefix t
   "Allow repeating certain macro commands without entering the C-x C-k prefix."
-  :type 'boolean
-  :group 'kmacro)
+  :type 'boolean)
 
 (defcustom kmacro-call-repeat-key t
   "Allow repeating macro call using last key or a specific key."
   :type '(choice (const :tag "Disabled" nil)
                  (const :tag "Last key" t)
                  (character :tag "Character" :value ?e)
-                 (symbol :tag "Key symbol" :value RET))
-  :group 'kmacro)
+                 (symbol :tag "Key symbol" :value RET)))
 
 (defcustom kmacro-call-repeat-with-arg nil
   "Repeat macro call with original arg when non-nil; repeat once if nil."
-  :type 'boolean
-  :group 'kmacro)
+  :type 'boolean)
 
 (defcustom kmacro-step-edit-mini-window-height 0.75
   "Override `max-mini-window-height' when step edit keyboard macro."
-  :type 'number
-  :group 'kmacro)
+  :type 'number)
 
 ;; Keymap
 
@@ -261,7 +254,7 @@ previous `kmacro-counter', and do not modify counter."
   (if kmacro-initial-counter-value
       (setq kmacro-counter kmacro-initial-counter-value
             kmacro-initial-counter-value nil))
-  (if (and arg (listp arg))
+  (if (consp arg)
       (insert (format kmacro-counter-format kmacro-last-counter))
     (insert (format kmacro-counter-format kmacro-counter))
     (kmacro-add-counter (prefix-numeric-value arg))))
@@ -280,8 +273,8 @@ previous `kmacro-counter', and do not modify counter."
 (defun kmacro-display-counter (&optional value)
   "Display current counter value."
   (unless value (setq value kmacro-counter))
-  (message "New macro counter value: %s (%d)" (format kmacro-counter-format value) value))
-
+  (message "New macro counter value: %s (%d)"
+           (format kmacro-counter-format value) value))
 
 (defun kmacro-set-counter (arg)
   "Set `kmacro-counter' to ARG or prompt if missing.
@@ -774,19 +767,18 @@ If kbd macro currently being defined end it before activating it."
 
 (defun kmacro-extract-lambda (mac)
   "Extract kmacro from a kmacro lambda form."
-  (and (consp mac)
-       (eq (car mac) 'lambda)
+  (and (eq (car-safe mac) 'lambda)
        (setq mac (assoc 'kmacro-exec-ring-item mac))
-       (consp (cdr mac))
-       (consp (car (cdr mac)))
-       (consp (cdr (car (cdr mac))))
-       (setq mac (car (cdr (car (cdr mac)))))
+       (setq mac (car-safe (cdr-safe (car-safe (cdr-safe mac)))))
        (listp mac)
        (= (length mac) 3)
        (arrayp (car mac))
        mac))
 
 
+(defalias 'kmacro-p #'kmacro-extract-lambda
+  "Return non-nil if MAC is a kmacro keyboard macro.")
+
 (defun kmacro-bind-to-key (_arg)
   "When not defining or executing a macro, offer to bind last macro to a key.
 The key sequences [C-x C-k 0] through [C-x C-k 9] and [C-x C-k A]
@@ -827,6 +819,13 @@ The ARG parameter is unused."
           (kmacro-lambda-form (kmacro-ring-head)))
         (message "Keyboard macro bound to %s" (format-kbd-macro key-seq))))))
 
+(defun kmacro-keyboard-macro-p (symbol)
+  "Return non-nil if SYMBOL is the name of some sort of keyboard macro."
+  (let ((f (symbol-function symbol)))
+    (when f
+      (or (stringp f)
+          (vectorp f)
+          (kmacro-p f)))))
 
 (defun kmacro-name-last-macro (symbol)
   "Assign a name to the last keyboard macro defined.
@@ -837,14 +836,18 @@ Such a \"function\" cannot be called from Lisp, but it is a valid editor command
   (or last-kbd-macro
       (error "No keyboard macro defined"))
   (and (fboundp symbol)
-       (not (get symbol 'kmacro))
-       (not (stringp (symbol-function symbol)))
-       (not (vectorp (symbol-function symbol)))
+       (not (kmacro-keyboard-macro-p symbol))
        (error "Function %s is already defined and not a keyboard macro"
               symbol))
   (if (string-equal symbol "")
       (error "No command name given"))
+  ;; FIXME: Use plain old `last-kbd-macro' for kmacros where it doesn't
+  ;; make a difference?
   (fset symbol (kmacro-lambda-form (kmacro-ring-head)))
+  ;; This used to be used to detect when a symbol corresponds to a kmacro.
+  ;; Nowadays it's unused because we used `kmacro-p' instead to see if the
+  ;; symbol's function definition matches that of a kmacro, which is more
+  ;; reliable.
   (put symbol 'kmacro t))
 
 
@@ -1203,7 +1206,7 @@ following additional answers: `insert', `insert-1', `replace', `replace-1',
     (setq kmacro-step-edit-key-index next-index)))
 
 (defun kmacro-step-edit-pre-command ()
-  (remove-hook 'post-command-hook 'kmacro-step-edit-post-command)
+  (remove-hook 'post-command-hook #'kmacro-step-edit-post-command)
   (when kmacro-step-edit-active
     (cond
      ((eq kmacro-step-edit-active 'ignore)
@@ -1223,17 +1226,17 @@ following additional answers: `insert', `insert-1', `replace', `replace-1',
           (setq kmacro-step-edit-appending nil
                 kmacro-step-edit-active 'ignore)))))
   (when (eq kmacro-step-edit-active t)
-    (add-hook 'post-command-hook 'kmacro-step-edit-post-command t)))
+    (add-hook 'post-command-hook #'kmacro-step-edit-post-command t)))
 
 (defun kmacro-step-edit-minibuf-setup ()
-  (remove-hook 'pre-command-hook 'kmacro-step-edit-pre-command t)
+  (remove-hook 'pre-command-hook #'kmacro-step-edit-pre-command t)
   (when kmacro-step-edit-active
-    (add-hook 'pre-command-hook 'kmacro-step-edit-pre-command nil t)))
+    (add-hook 'pre-command-hook #'kmacro-step-edit-pre-command nil t)))
 
 (defun kmacro-step-edit-post-command ()
-  (remove-hook 'pre-command-hook 'kmacro-step-edit-pre-command)
+  (remove-hook 'pre-command-hook #'kmacro-step-edit-pre-command)
   (when kmacro-step-edit-active
-    (add-hook 'pre-command-hook 'kmacro-step-edit-pre-command nil nil)
+    (add-hook 'pre-command-hook #'kmacro-step-edit-pre-command nil nil)
     (if kmacro-step-edit-key-index
         (setq executing-kbd-macro-index kmacro-step-edit-key-index)
       (setq kmacro-step-edit-key-index executing-kbd-macro-index))))
@@ -1256,9 +1259,9 @@ To customize possible responses, change the \"bindings\" in `kmacro-step-edit-ma
         (pre-command-hook pre-command-hook)
         (post-command-hook post-command-hook)
         (minibuffer-setup-hook minibuffer-setup-hook))
-    (add-hook 'pre-command-hook 'kmacro-step-edit-pre-command nil)
-    (add-hook 'post-command-hook 'kmacro-step-edit-post-command t)
-    (add-hook 'minibuffer-setup-hook 'kmacro-step-edit-minibuf-setup t)
+    (add-hook 'pre-command-hook #'kmacro-step-edit-pre-command nil)
+    (add-hook 'post-command-hook #'kmacro-step-edit-post-command t)
+    (add-hook 'minibuffer-setup-hook #'kmacro-step-edit-minibuf-setup t)
     (call-last-kbd-macro nil nil)
     (when (and kmacro-step-edit-replace
                kmacro-step-edit-new-macro
diff --git a/lisp/macros.el b/lisp/macros.el
index 34e81f693f5..5583c02f68b 100644
--- a/lisp/macros.el
+++ b/lisp/macros.el
@@ -1,4 +1,4 @@
-;;; macros.el --- non-primitive commands for keyboard macros
+;;; macros.el --- non-primitive commands for keyboard macros -*- lexical-binding:t -*-
 
 ;; Copyright (C) 1985-1987, 1992, 1994-1995, 2001-2017 Free Software
 ;; Foundation, Inc.
@@ -31,23 +31,10 @@
 
 ;;; Code:
 
+(require 'kmacro)
+
 ;;;###autoload
-(defun name-last-kbd-macro (symbol)
-  "Assign a name to the last keyboard macro defined.
-Argument SYMBOL is the name to define.
-The symbol's function definition becomes the keyboard macro string.
-Such a \"function\" cannot be called from Lisp, but it is a valid editor command."
-  (interactive "SName for last kbd macro: ")
-  (or last-kbd-macro
-      (user-error "No keyboard macro defined"))
-  (and (fboundp symbol)
-       (not (stringp (symbol-function symbol)))
-       (not (vectorp (symbol-function symbol)))
-       (user-error "Function %s is already defined and not a keyboard macro"
-              symbol))
-  (if (string-equal symbol "")
-      (user-error "No command name given"))
-  (fset symbol last-kbd-macro))
+(defalias 'name-last-kbd-macro #'kmacro-name-last-macro)
 
 ;;;###autoload
 (defun insert-kbd-macro (macroname &optional keys)
@@ -66,11 +53,7 @@ To save a kbd macro, visit a file of Lisp code such as your `~/.emacs',
 use this command, and then save the file."
   (interactive (list (intern (completing-read "Insert kbd macro (name): "
                                               obarray
-                                              (lambda (elt)
-                                                (and (fboundp elt)
-                                                     (or (stringp (symbol-function elt))
-                                                         (vectorp (symbol-function elt))
-                                                         (get elt 'kmacro))))
+                                              #'kmacro-keyboard-macro-p
                                               t))
                      current-prefix-arg))
   (let (definition)
@@ -137,6 +120,9 @@ use this command, and then save the file."
                   (prin1 char (current-buffer))
                 (princ (prin1-char char) (current-buffer))))
             (insert ?\]))
+        ;; FIXME: For kmacros, we shouldn't write the (lambda ...)
+        ;; gunk but instead we should write something more abstract like
+        ;; (kmacro-create [<keys>] 0 "%d").
         (prin1 definition (current-buffer))))
     (insert ")\n")
     (if keys