Re: master a6a5d6a: Move 'revert-buffer' global binding to 'C-x g g'

[Stefan Kangas <stefan@marxist.se>]

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

Stefan Kangas <stefan@marxist.se> writes:

> I don't know what notation to use here though.  Maybe something like:
>
>     \\[cmd]  -> works as today
>     \\[=C-h] -> just adds the formatting to the exact string
>     \\[+cmd] -> adds both the key binding and its M-x form

Introducing the \\[=C-h] substitution (patch 1) does seem to bring
useful simplifications (patch 2).

[-- Attachment #2: 0001-Add-new-format-for-literal-keybindings-to-substitute.patch --]
[-- Type: text/x-diff, Size: 4480 bytes --]

From e427472e6f79007996afd47fd87d1d41bf1dec62 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Thu, 11 Mar 2021 05:28:58 +0100
Subject: [PATCH 1/2] Add new format for literal keybindings to
 substitute-command-keys

* lisp/help.el (substitute-command-keys): Add new format for
literal keybindings.
* test/lisp/help-tests.el
(help-tests-substitute-command-keys/literal-keybinding):
(help-tests-substitute-key-bindings/face-help-key-binding): New tests.
---
 lisp/help.el            | 34 ++++++++++++++++++++++++----------
 test/lisp/help-tests.el | 14 ++++++++++++++
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/lisp/help.el b/lisp/help.el
index 79d8296cfe..d1a817a59d 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -984,6 +984,9 @@ substitute-command-keys
 keystroke sequence that invokes COMMAND, or \"M-x COMMAND\" if COMMAND
 is not on any keys.  Keybindings will use the face `help-key-binding'.
 
+Each substring of the form \\\\=[=KEYBINDING] will be replaced by
+KEYBINDING and use the `help-key-binding' face.
+
 Each substring of the form \\\\={MAPVAR} is replaced by a summary of
 the value of MAPVAR as a keymap.  This summary is similar to the one
 produced by ‘describe-bindings’.  The summary ends in two newlines
@@ -1054,18 +1057,29 @@ substitute-command-keys
                     (setq fun (aref key 1))
                     (setq key (with-current-buffer orig-buf
                                 (where-is-internal fun keymap t))))
-                  (if (not key)
-                      ;; Function is not on any key.
-                      (let ((op (point)))
-                        (insert "M-x ")
-                        (goto-char (+ end-point 3))
-                        (add-text-properties op (point)
-                                             '( face help-key-binding
-                                                font-lock-face help-key-binding))
-                        (delete-char 1))
+                  (cond
+                   ((looking-at "=")
+                    ;; Literal replacement.
+                    (let ((op (point)))
+                      (delete-char 1)
+                      (goto-char (- end-point 2))
+                      (delete-char 1)
+                      (add-text-properties op (point)
+                                           '( face help-key-binding
+                                              font-lock-face help-key-binding))))
+                   ((not key)
+                    ;; Function is not on any key.
+                    (let ((op (point)))
+                      (insert "M-x ")
+                      (goto-char (+ end-point 3))
+                      (add-text-properties op (point)
+                                           '( face help-key-binding
+                                              font-lock-face help-key-binding))
+                      (delete-char 1)))
+                   (t
                     ;; Function is on a key.
                     (delete-char (- end-point (point)))
-                    (insert (help--key-description-fontified key)))))
+                    (insert (help--key-description-fontified key))))))
                ;; 1D. \{foo} is replaced with a summary of the keymap
                ;;            (symbol-value foo).
                ;;     \<foo> just sets the keymap used for \[cmd].
diff --git a/test/lisp/help-tests.el b/test/lisp/help-tests.el
index b2fec5c1bd..09ac01ee64 100644
--- a/test/lisp/help-tests.el
+++ b/test/lisp/help-tests.el
@@ -88,6 +88,20 @@ help-tests-substitute-command-keys/commands
    (test "\\[emacs-version]\\[next-line]" "M-x emacs-versionC-n")
    (test-re "\\[emacs-version]`foo'" "M-x emacs-version[`'‘]foo['’]")))
 
+(ert-deftest help-tests-substitute-command-keys/literal-keybinding ()
+  "Literal replacement."
+  (with-substitute-command-keys-test
+   (test "\\[=C-m]" "C-m")
+   (test "\\[=C-m]\\[=C-j]" "C-mC-j")
+   (test "foo\\[=C-m]bar\\[=C-j]baz" "fooC-mbarC-jbaz")))
+
+(ert-deftest help-tests-substitute-key-bindings/face-help-key-binding ()
+  (should (eq (get-text-property 0 'face (substitute-command-keys "\\[next-line]"))
+              'help-key-binding))
+  (should (eq (get-text-property 0 'face (substitute-command-keys "\\[=f]"))
+              'help-key-binding)))
+
+
 (ert-deftest help-tests-substitute-command-keys/keymaps ()
   (with-substitute-command-keys-test
    (test "\\{minibuffer-local-must-match-map}"
-- 
2.30.1


[-- Attachment #3: 0002-Use-substitute-command-keys-in-userlock.el.patch --]
[-- Type: text/x-diff, Size: 5642 bytes --]

From f5b32d46ce9b29d65a8bf5fee58eca5d65ec4354 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Thu, 11 Mar 2021 05:32:29 +0100
Subject: [PATCH 2/2] Use substitute-command-keys in userlock.el

* lisp/userlock.el (userlock--fontify-key): Remove function.
(ask-user-about-lock, ask-user-about-lock-help)
(ask-user-about-supersession-threat)
(ask-user-about-supersession-help): Use substitute-command-keys.
---
 lisp/userlock.el | 65 +++++++++++++++++-------------------------------
 1 file changed, 23 insertions(+), 42 deletions(-)

diff --git a/lisp/userlock.el b/lisp/userlock.el
index 57311ac99c..f0dbbe7b4c 100644
--- a/lisp/userlock.el
+++ b/lisp/userlock.el
@@ -39,10 +39,6 @@
 
 (define-error 'file-locked "File is locked" 'file-error)
 
-(defun userlock--fontify-key (key)
-  "Add the `help-key-binding' face to string KEY."
-  (propertize key 'face 'help-key-binding))
-
 ;;;###autoload
 (defun ask-user-about-lock (file opponent)
   "Ask user what to do when he wants to edit FILE but it is locked by OPPONENT.
@@ -68,12 +64,9 @@ ask-user-about-lock
 			  (match-string 0 opponent)))
 	      opponent))
       (while (null answer)
-        (message "%s locked by %s: (%s, %s, %s, %s)? "
-                 short-file short-opponent
-                 (userlock--fontify-key "s")
-                 (userlock--fontify-key "q")
-                 (userlock--fontify-key "p")
-                 (userlock--fontify-key "?"))
+        (message (substitute-command-keys
+                  "%s locked by %s: (\\[=s], \\[=q], \\[=p], \\[=?])? ")
+                 short-file short-opponent)
 	(if noninteractive (error "Cannot resolve lock conflict in batch mode"))
 	(let ((tem (let ((inhibit-quit t)
 			 (cursor-in-echo-area t))
@@ -88,12 +81,9 @@ ask-user-about-lock
 				      (?? . help))))
 	    (cond ((null answer)
 		   (beep)
-                   (message "Please type %s, %s, or %s; or %s for help"
-                            (userlock--fontify-key "q")
-                            (userlock--fontify-key "s")
-                            (userlock--fontify-key "p")
-                            ;; FIXME: Why do we use "?" here and "C-h" below?
-                            (userlock--fontify-key "?"))
+                   ;; FIXME: Why do we use "?" here and "C-h" below?
+                   (message (substitute-command-keys
+                             "Please type \\[=q], \\[=s], or \\[=p]; or \\[=?] for help"))
 		   (sit-for 3))
 		  ((eq (cdr answer) 'help)
 		   (ask-user-about-lock-help)
@@ -106,17 +96,14 @@ ask-user-about-lock-help
   (with-output-to-temp-buffer "*Help*"
     (with-current-buffer standard-output
       (insert
-       (format
+       (substitute-command-keys
         "It has been detected that you want to modify a file that someone else has
 already started modifying in Emacs.
 
-You can <%s>teal the file; the other user becomes the
+You can <\\[=s]>teal the file; the other user becomes the
   intruder if (s)he ever unmodifies the file and then changes it again.
-You can <%s>roceed; you edit at your own (and the other user's) risk.
-You can <%s>uit; don't modify this file."
-        (userlock--fontify-key "s")
-        (userlock--fontify-key "p")
-        (userlock--fontify-key "q")))
+You can <\\[=p]>roceed; you edit at your own (and the other user's) risk.
+You can <\\[=q]>uit; don't modify this file."))
       (help-mode))))
 
 (define-error 'file-supersession nil 'file-error)
@@ -168,14 +155,11 @@ ask-user-about-supersession-threat
   (discard-input)
   (save-window-excursion
     (let ((prompt
-	   (format "%s changed on disk; \
-really edit the buffer? (%s, %s, %s or %s) "
-                   (file-name-nondirectory filename)
-                   (userlock--fontify-key "y")
-                   (userlock--fontify-key "n")
-                   (userlock--fontify-key "r")
-                   ;; FIXME: Why do we use "C-h" here and "?" above?
-                   (userlock--fontify-key "C-h")))
+           ;; FIXME: Why do we use "C-h" here and "?" above?
+           (format (substitute-command-keys
+                    "%s changed on disk; \
+really edit the buffer? (\\[=y], \\[=n], \\[=r] or \\[=C-h]) ")
+                   (file-name-nondirectory filename)))
 	  (choices '(?y ?n ?r ?? ?\C-h))
 	  answer)
       (when noninteractive
@@ -206,21 +190,18 @@ ask-user-about-supersession-help
       (with-current-buffer standard-output
         (insert
          (format
-          "You want to modify a buffer whose disk file has changed
+          (substitute-command-keys
+           "You want to modify a buffer whose disk file has changed
 since you last read it in or saved it with this buffer.
 
-If you say %s to go ahead and modify this buffer,
+If you say \\[=y] to go ahead and modify this buffer,
 you risk ruining the work of whoever rewrote the file.
-If you say %s to revert, the contents of the buffer are refreshed
+If you say \\[=r] to revert, the contents of the buffer are refreshed
 from the file on disk.
-If you say %s, the change you started to make will be aborted.
-
-Usually, you should type %s and then %s,
-to get the latest version of the file, then make the change again."
-          (userlock--fontify-key "y")
-          (userlock--fontify-key "r")
-          (userlock--fontify-key "n")
-          (userlock--fontify-key "n")
+If you say \\[=n], the change you started to make will be aborted.
+
+Usually, you should type \\[=n] and then \\[=M-x revert-buffer] (or %s),
+to get the latest version of the file, then make the change again.")
           revert-buffer-binding))
         (help-mode)))))
 
-- 
2.30.1