kmacro: Represent it as an OClosure

Merge the old lambda+list into a single OClosure object which plays
both roles at the same time.  Take advantage of it to provide a
`cl-print-object` method so kmacro objects print nicely using the
`key-parse` syntax.
Also replace the old `kmacro-lambda-form` with a new `kmacro` constructor
which takes a `key-parse` syntax, so that the code inserted with
`insert-kbd-macro` is now more readable.

* lisp/kmacro.el (kmacro): New OClosure type.
(kmacro-ring-head): Use `kmacro` constructor.
(kmacro-push-ring): Convert `elt` from old representation if needed.
(kmacro-split-ring-element, kmacro-view-ring-2nd, kmacro-view-macro):
Adapt to new representation.
(kmacro-exec-ring-item): Turn into obsolete alias.
(kmacro-call-ring-2nd, kmacro-end-or-call-macro): Adjust accordingly.
(kmacro-start-macro): Simplify call to `kmacro-push-ring`.
(kmacro): New constructor function.  Replaces `kmacro-lambda-form`.
(kmacro-lambda-form): Use it and declare obsolete.
(kmacro-extract-lambda): Rewrite and declare obsolete.
(kmacro-p): Rewrite.
(cl-print-object): New method.
(kmacro-bind-to-key, kmacro-name-last-macro): Simplify.

* lisp/macros.el (macro--string-to-vector): New function.
(insert-kbd-macro): Use it.  Generate code using the `kmacro` constructor.

* test/lisp/kmacro-tests.el (kmacro-tests-kmacro-bind-to-single-key):
Silence warning.
(kmacro-tests-name-last-macro-bind-and-rebind): Strengthen the test a bit.
(kmacro-tests--cl-print): New test.

1 files changed, 33 insertions, 58 deletions

diff --git a/lisp/macros.el b/lisp/macros.el
index 35d34d2e337..0baf3804332 100644
--- a/lisp/macros.el
+++ b/lisp/macros.el
@@ -46,6 +46,16 @@
                      " ")
           ?\]))
 
+(defun macro--string-to-vector (str)
+  "Convert an old-style string key sequence to the vector form."
+  (let ((vec (string-to-vector str)))
+    (unless (multibyte-string-p str)
+      (dotimes (i (length vec))
+	(let ((k (aref vec i)))
+	  (when (> k 127)
+	    (setf (aref vec i) (+ k ?\M-\C-@ -128))))))
+    vec))
+
 ;;;###autoload
 (defun insert-kbd-macro (macroname &optional keys)
   "Insert in buffer the definition of kbd macro MACRONAME, as Lisp code.
@@ -72,66 +82,31 @@ use this command, and then save the file."
 	  (setq macroname 'last-kbd-macro definition last-kbd-macro)
 	  (insert "(setq "))
       (setq definition (symbol-function macroname))
-      (insert "(fset '"))
+      ;; Prefer `defalias' over `fset' since it additionally keeps
+      ;; track of the file where the users added it, and it interacts
+      ;; better with `advice-add' (and hence things like ELP).
+      (insert "(defalias '"))
     (prin1 macroname (current-buffer))
     (insert "\n   ")
-    (if (stringp definition)
-	(let ((beg (point)) end)
-	  (prin1 definition (current-buffer))
-	  (setq end (point-marker))
-	  (goto-char beg)
-	  (while (< (point) end)
-	    (let ((char (following-char)))
-	      (cond ((= char 0)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\C-@"))
-		    ((< char 27)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\C-" (+ 96 char)))
-		    ((= char ?\C-\\)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\C-\\\\"))
-		    ((< char 32)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\C-" (+ 64 char)))
-		    ((< char 127)
-		     (forward-char 1))
-		    ((= char 127)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\C-?"))
-		    ((= char 128)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\M-\\C-@"))
-		    ((= char (aref "\M-\C-\\" 0))
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\M-\\C-\\\\"))
-		    ((< char 155)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\M-\\C-" (- char 32)))
-		    ((< char 160)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\M-\\C-" (- char 64)))
-		    ((= char (aref "\M-\\" 0))
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\M-\\\\"))
-		    ((< char 255)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\M-" (- char 128)))
-		    ((= char 255)
-		     (delete-region (point) (1+ (point)))
-		     (insert "\\M-\\C-?"))))))
-      (if (vectorp definition)
-          (macros--insert-vector-macro definition)
-        (pcase (kmacro-extract-lambda definition)
-          (`(,vecdef ,counter ,format)
-           (insert "(kmacro-lambda-form ")
-           (macros--insert-vector-macro vecdef)
-           (insert " ")
-           (prin1 counter (current-buffer))
-           (insert " ")
-           (prin1 format (current-buffer))
-           (insert ")"))
-          (_ (prin1 definition (current-buffer))))))
+    (when (stringp definition)
+      (setq definition (macro--string-to-vector definition)))
+    (if (vectorp definition)
+        (setq definition (kmacro definition)))
+    (if (kmacro-p definition)
+        (let ((vecdef  (kmacro--keys     definition))
+              (counter (kmacro--counter definition))
+              (format  (kmacro--format  definition)))
+          (insert "(kmacro ")
+          (prin1 (key-description vecdef) (current-buffer))
+          ;; FIXME: Do we really want to store the counter?
+          (unless (and (equal counter 0) (equal format "%d"))
+            (insert " ")
+            (prin1 counter (current-buffer))
+            (insert " ")
+            (prin1 format (current-buffer)))
+          (insert ")"))
+      ;; FIXME: Shouldn't this signal an error?
+      (prin1 definition (current-buffer)))
     (insert ")\n")
     (if keys
         (let ((keys (or (and (symbol-function macroname)