bug#74111: [PATCH] New `tmm-menubar' shortcut display style

[Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: 74111@debbugs.gnu.org
>> Date: Thu, 31 Oct 2024 12:48:46 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Are we sure users will understand what "marked letters" mean in that
>> > case?
>> 
>> Maybe we could change it to "highlighted letters (or digits)".  But
>> FWIW, "marked letters" was not really more clear before this patch.  So
>> I think we could try to rewrite tmm-completion-prompt with something
>> clearer that work for both options.  WDYT?
>
> I agree, please suggest a better text that caters to both cases.

What do you think of this new version of the patch?  I removed the
"PageUp" comment as it does not seem to work (while 'M-v' is) and I make
the help adapt whether `tmm-shortcut-inside-entry' is on or not.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-New-tmm-menubar-shortcut-display-style.patch --]
[-- Type: text/x-patch, Size: 7285 bytes --]

From 87908786843fb9ce7f4ef150bee4c687a8f1f848 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Wed, 30 Oct 2024 17:24:24 +0100
Subject: [PATCH] New `tmm-menubar' shortcut display style

Bug#74111

* lisp/tmm.el (tmm-shortcut-inside-entry): New user option to
control if the shortcut character is placed inside the menu
entry's string.
(tmm-add-one-shortcut): Compute the new shortcut style.
(tmm-prompt): Adapt the prompt regexp.
(tmm-shortcut): Simplify and make it work with this change.
* etc/NEWS: Announce the option.
---
 etc/NEWS    |  6 +++++
 lisp/tmm.el | 70 ++++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d1c7303f976..c3fc8d46062 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -584,6 +584,12 @@ instead.
 *** A new shortcut to navigate to previous menu.
 The hardcoded '^' shortcut gets you back to the previous menu.
 
+---
+*** New user option 'tmm-shortcut-inside-entry'.
+When non-nil, insert the character shortcut of a menu entry inside its
+string.  This character is highlighted with a different face to help
+identifying it.
+
 ** Foldout
 
 ---
diff --git a/lisp/tmm.el b/lisp/tmm.el
index 252c1e45afa..3b94656a657 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -83,11 +83,10 @@ tmm-mid-prompt
                  string))
 
 (defcustom tmm-completion-prompt
-  "Press PageUp key to reach this buffer from the minibuffer.
-Alternatively, you can use Up/Down keys (or your History keys) to change
+  "You can use Up/Down keys (or your History keys) to change
 the item in the minibuffer, and press RET when you are done, or press
-the marked letters to pick up your choice.  Type ^ to go to the parent
-menu.  Type C-g or ESC ESC ESC to cancel.
+the %s to pick up your choice.
+Type ^ to go to the parent menu.  Type C-g or ESC ESC ESC to cancel.
 "
   "Help text to insert on the top of the completion buffer.
 To save space, you can set this to nil,
@@ -108,6 +107,13 @@ tmm-shortcut-words
 specify nil for this variable."
   :type '(choice integer (const nil)))
 
+(defcustom tmm-shortcut-inside-entry nil
+  "Place the shortcut character into the menu entry's string.
+When non-nil, the shortcut character will be inserted into its menu
+entry string with a different face to help identifying it.  The
+`tmm-mid-prompt' string is not used then."
+  :type 'boolean)
+
 (defface tmm-inactive
   '((t :inherit shadow))
   "Face used for inactive menu items.")
@@ -198,7 +204,8 @@ tmm-prompt
 		     (setq tail (cdr tail)))))
              (let ((prompt
                     (concat "^"
-                            (if (stringp tmm-mid-prompt)
+                            (if (and (stringp tmm-mid-prompt)
+                                     (not tmm-shortcut-inside-entry))
                                 (concat "."
                                         (regexp-quote tmm-mid-prompt))))))
                (setq tmm--history
@@ -224,7 +231,7 @@ tmm-prompt
                        ;; reversed copy of the list.
                        (completing-read-default
                         (concat gl-str
-                                " (up/down to change, PgUp to menu): ")
+                                " (up/down to change): ")
                         (tmm--completion-table tmm-km-list) nil t nil
                         'tmm--history (reverse tmm--history)))))))
       (if (and (stringp out) (string= "^" out))
@@ -283,7 +290,7 @@ tmm-add-one-shortcut
    (t
     (let* ((str (car elt))
            (paren (string-search "(" str))
-           (pos 0) (word 0) char)
+           (word 0) pos char)
       (catch 'done                             ; ??? is this slow?
         (while (and (or (not tmm-shortcut-words)   ; no limit on words
                         (< word tmm-shortcut-words)) ; try n words
@@ -299,17 +306,32 @@ tmm-add-one-shortcut
                 (if (not (memq char tmm-short-cuts)) (throw 'done char))))
           (setq word (1+ word))
           (setq pos (match-end 0)))
+        ;; A nil value for pos means that the shortcut is not inside the
+        ;; string of the menu entry
+        (setq pos nil)
         (while (<= tmm-next-shortcut-digit ?9) ; no letter shortcut, pick a digit
           (setq char tmm-next-shortcut-digit)
           (setq tmm-next-shortcut-digit (1+ tmm-next-shortcut-digit))
           (if (not (memq char tmm-short-cuts)) (throw 'done char)))
         (setq char nil))
       (if char (setq tmm-short-cuts (cons char tmm-short-cuts)))
-      (cons (concat (if char (concat (char-to-string char) tmm-mid-prompt)
-                      ;; keep them lined up in columns
-                      (make-string (1+ (length tmm-mid-prompt)) ?\s))
-                    str)
-            (cdr elt))))))
+      (cons
+       (if tmm-shortcut-inside-entry
+           (if char
+               (if pos
+                   ;; A character inside the menu entry
+                   (let ((res (copy-sequence str)))
+                     (aset res pos char)
+                     (add-text-properties pos (1+ pos) '(face highlight) res)
+                     res)
+                 ;; A fallback digit character: place it in front of the menu entry
+                 (concat (propertize (char-to-string char) 'face 'highlight) " " str))
+             (make-string 2 ?\s))
+         (concat (if char (concat (char-to-string char) tmm-mid-prompt)
+                   ;; keep them lined up in columns
+                   (make-string (1+ (length tmm-mid-prompt)) ?\s))
+                 str))
+       (cdr elt))))))
 
 ;; This returns the old map.
 (defun tmm-define-keys (minibuffer)
@@ -384,7 +406,12 @@ tmm-add-prompt
       (let ((inhibit-read-only t)
 	    (window (get-buffer-window "*Completions*")))
 	(goto-char (point-min))
-	(insert tmm-completion-prompt)
+	(insert
+         (if tmm-shortcut-inside-entry
+             (format tmm-completion-prompt
+                     (concat (propertize "highlighted" 'face 'highlight) " character"))
+           (format tmm-completion-prompt
+                   (concat "character right before '" tmm-mid-prompt "' "))))
 	(when window
 	  ;; Try to show everything just inserted and preserve height of
 	  ;; *Completions* window.  This should fix a behavior described
@@ -406,13 +433,16 @@ tmm-shortcut
 	      (choose-completion))
 	  ;; In minibuffer
 	  (delete-region (minibuffer-prompt-end) (point-max))
-	  (dolist (elt tmm-km-list)
-            (if (string=
-                 (substring (car elt) 0
-                            (min (1+ (length tmm-mid-prompt))
-                                 (length (car elt))))
-                 (concat (char-to-string c) tmm-mid-prompt))
-                (setq s (car elt))))
+          (dolist (elt tmm-km-list)
+            (let ((str (car elt))
+                  (index 0))
+              (when tmm-shortcut-inside-entry
+                (if (get-char-property 0 'face str)
+                    (setq index 0)
+                  (let ((next (next-single-char-property-change 0 'face str)))
+                    (setq index (if (= (length str) next) 0 next)))))
+              (if (= (aref str index) c)
+                  (setq s str))))
 	  (insert s)
 	  (exit-minibuffer)))))
 
-- 
2.47.0


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud