From 33676820bdb0fac8ebd43ab098ad13d8098f3586 Mon Sep 17 00:00:00 2001
From: Phil Sainty <psainty@orcon.net.nz>
Date: Sat, 16 Nov 2019 15:48:44 +1300
Subject: [PATCH] lisp/so-long.el: Refactor menu action commands

* lisp/so-long.el (so-long-menu): Call `so-long' with an ACTION
argument instead of using `so-long-menu-item-replace-action'.
(so-long-menu-item-replace-action): Remove the deprecated function.

* test/lisp/so-long-tests/so-long-tests.el
(so-long-tests-so-long-menu-item-replace-action): Update the test.
---
 lisp/so-long.el                          | 35 +++++++++---------------
 test/lisp/so-long-tests/so-long-tests.el | 14 ++++++----
 2 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/lisp/so-long.el b/lisp/so-long.el
index e80abc76a53..33b7155d044 100644
--- a/lisp/so-long.el
+++ b/lisp/so-long.el
@@ -902,14 +902,19 @@ If RESET is non-nil, remove any existing values before storing the new ones."
           `(menu-item
             ,label
             ,(let ((sym (make-symbol "so-long-menu-item-replace-action")))
-               ;; Using a symbol here, so that `describe-key' on the menu item
-               ;; produces the `so-long-menu-item-replace-action' documentation.
-               (defalias sym
-                 (apply-partially #'so-long-menu-item-replace-action item)
-                 (documentation #'so-long-menu-item-replace-action))
-               (put sym 'interactive-form '(interactive "@"))
-               ;; We use "@" as commands in the mode-line menu may be
-               ;; triggered by mouse when some other window is selected.
+               ;; We make a symbol so that `describe-key' on the menu item
+               ;; produces something more descriptive than byte code.  There is
+               ;; no interned `so-long-menu-item-replace-action' which might
+               ;; make this slightly confusing -- but only in the rare situation
+               ;; when someone uses `describe-key' on one of these menu items,
+               ;; and then wants to find more information.  We mitigate this by
+               ;; making the following docstring very clear.
+               (defalias sym (lambda () (interactive "@") (so-long key))
+                 ;; We use "@" as commands in the mode-line menu may be
+                 ;; triggered by mouse when some other window is selected.
+                 "Revert the current action and invoke the chosen replacement.
+
+This commmand calls `so-long' with the selected action as an argument.")
                sym)
             :enable (not (and so-long--active
                               (eq ',actionfunc so-long-function)
@@ -925,20 +930,6 @@ If RESET is non-nil, remove any existing values before storing the new ones."
       '(menu-item "Customize" so-long-customize))
     map))
 
-(defun so-long-menu-item-replace-action (replacement)
-  "Revert the current action and invoke the specified replacement.
-
-REPLACEMENT is a `so-long-action-alist' item."
-  (interactive)
-  (when so-long--active
-    (so-long-revert))
-  (cl-destructuring-bind (_key _label actionfunc revertfunc)
-      replacement
-    (setq so-long-function actionfunc)
-    (setq so-long-revert-function revertfunc)
-    (setq this-command 'so-long)
-    (so-long)))
-
 ;;;###autoload
 (defun so-long-commentary ()
   "View the so-long documentation in `outline-mode'."
diff --git a/test/lisp/so-long-tests/so-long-tests.el b/test/lisp/so-long-tests/so-long-tests.el
index 5c885c4fd09..ae834421667 100644
--- a/test/lisp/so-long-tests/so-long-tests.el
+++ b/test/lisp/so-long-tests/so-long-tests.el
@@ -259,22 +259,24 @@
       (so-long-tests-assert-and-revert (or action 'so-long-mode)))))
 
 (ert-deftest so-long-tests-so-long-menu-item-replace-action ()
-  "Test using the `so-long-menu-item-replace-action' menu item."
+  "Test using the `so-long-menu' action commands."
   (with-temp-buffer
     (insert "#!emacs\n")
     (normal-mode)
     (so-long-tests-remember)
     (insert (make-string (1+ so-long-threshold) ?x))
-    (let (action)
+    (let ((menu (so-long-menu))
+          action
+          command)
       (dolist (item so-long-action-alist)
-        ;; n.b. Any existing action is first reverted.
-        (so-long-menu-item-replace-action item)
-        (setq action (car item))
+        (setq action (car item)
+              command (lookup-key menu (vector action)))
+        (funcall command)
         (so-long-tests-assert-active action))
       ;; After all actions have been used, revert to normal and assert
       ;; that the most recent action to have been applied is the one
       ;; we have just reverted.
-      (so-long-revert)
+      (funcall (lookup-key menu [so-long-revert]))
       (so-long-tests-assert-reverted action))))
 
 (ert-deftest so-long-tests-major-mode ()
-- 
2.39.5