Re: master 0023891e842: Support hints in the :repeat keyword of defvar-keymap for repeat-mode

Re: master 0023891e842: Support hints in the :repeat keyword of defvar-keymap for repeat-mode

[Eshel Yaron]

Hi,

Juri Linkov <juri@jurta.org> writes:

> branch: master
> commit 0023891e84285c2ea2a8029f479135f792c9d1e9
> Author: Juri Linkov <juri@linkov.net>
> Commit: Juri Linkov <juri@linkov.net>
>
>     Support hints in the :repeat keyword of defvar-keymap for repeat-mode
>
>     * lisp/keymap.el (defvar-keymap): Add :hints to the :repeat keyword.
>     Put the property 'repeat-hint' on the command symbol.
>
>     * lisp/repeat.el (repeat-echo-message-string): Show hint strings
>     defined with the property 'repeat-hint' on the command symbol (bug#70576).

Thank you for this nice addition.  A couple of thoughts I had after
trying it out:

The need to repeat the name of the command in order to specify its label
("hint") makes the keymap definition twice as long and prone to errors.
For example, after adding "hints" to one of my keymaps I got:

--8<---------------cut here---------------start------------->8---
(defvar-keymap esy/emms-map
  :doc "My keymap for EMMS commands."
  :prefix 'esy/emms-map
  :repeat (:hints ((emms-next          . "Next")
                   (emms-previous      . "Previous")
                   (emms-stop          . "Stop")
                   (emms-seek-backward . "backward")
                   (emms-seek-forward  . "forward")
                   (emms-seek          . "seek")
                   (emms-show          . "show")
                   (emms-pause         . "pause")
                   (emms-start         . "start")
                   (emms-seek-to       . "to")))
  "N" #'emms-next
  "P" #'emms-previous
  "S" #'emms-stop
  "b" #'emms-seek-backward
  "f" #'emms-seek-forward
  "k" #'emms-seek
  "m" #'emms-show
  "p" #'emms-pause
  "s" #'emms-start
  "t" #'emms-seek-to)
--8<---------------cut here---------------end--------------->8---

I don't have a great idea for an alternative interface off the top of my
head, but if there's a way to avoid this duplication I think it would be
a significant improvement.

Lastly, it could be nice to make the appearance of the prompt more in
line with the read-multiple-choice prompt, which is conceptually very
similar.  Crucially, if the key is a character that occurs in the label,
it's better, IMO, to highlight the first occurrence of that character in
the label (as r-m-c does) than to prefix the label with that character.
We can reuse rmc--add-key-description for that purpose, as follows:

diff --git a/lisp/repeat.el b/lisp/repeat.el
index 412afc35ba7..cd70c1a7f2d 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -562,10 +562,7 @@ repeat-echo-message-string
                          (cmd (cdr key-cmd))
                          (hint (when (symbolp cmd)
                                  (get cmd 'repeat-hint))))
-                    (substitute-command-keys
-                     (format "\\`%s'%s"
-                             (key-description (vector key))
-                             (if hint (format ":%s" hint) "")))))
+                    (cdr (rmc--add-key-description (list key (or hint ""))))))
                 keys ", ")
      (if repeat-exit-key
          (substitute-command-keys

Re: master 0023891e842: Support hints in the :repeat keyword of defvar-keymap for repeat-mode

[Juri Linkov]

> The need to repeat the name of the command in order to specify its label
> ("hint") makes the keymap definition twice as long and prone to errors.
> For example, after adding "hints" to one of my keymaps I got:
> [...]
> I don't have a great idea for an alternative interface off the top of my
> head, but if there's a way to avoid this duplication I think it would be
> a significant improvement.

Unfortunately, duplication can be removed only at the cost of
complicating the definitions of bindings that would be more
hard to learn for users, and therefore will remain prone to errors.

> Lastly, it could be nice to make the appearance of the prompt more in
> line with the read-multiple-choice prompt, which is conceptually very
> similar.  Crucially, if the key is a character that occurs in the label,
> it's better, IMO, to highlight the first occurrence of that character in
> the label (as r-m-c does) than to prefix the label with that character.
> We can reuse rmc--add-key-description for that purpose, as follows:

Nice idea.  A small problem is that when hints are not used,
it inserts an extra space between the character and comma.
But this is so minor that you could push the patch
if you think it's not worth making code more complex
by using rmc only when hints are specified.

Re: master 0023891e842: Support hints in the :repeat keyword of defvar-keymap for repeat-mode

[JD Smith]

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



> On May 7, 2024, at 2:27 AM, Juri Linkov <juri@linkov.net> wrote:
> 
>> The need to repeat the name of the command in order to specify its label
>> ("hint") makes the keymap definition twice as long and prone to errors.
>> For example, after adding "hints" to one of my keymaps I got:
>> [...]
>> I don't have a great idea for an alternative interface off the top of my
>> head, but if there's a way to avoid this duplication I think it would be
>> a significant improvement.

The original patch <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=70576> re-used the (STRING . DEFN) menu form of key bindings to avoid such duplication, but was deemed too confusing for users.

> Unfortunately, duplication can be removed only at the cost of
> complicating the definitions of bindings that would be more
> hard to learn for users, and therefore will remain prone to errors.
> 
>> Lastly, it could be nice to make the appearance of the prompt more in
>> line with the read-multiple-choice prompt, which is conceptually very
>> similar.  Crucially, if the key is a character that occurs in the label,
>> it's better, IMO, to highlight the first occurrence of that character in
>> the label (as r-m-c does) than to prefix the label with that character.
>> We can reuse rmc--add-key-description for that purpose, as follows:
> 
> Nice idea.  A small problem is that when hints are not used,
> it inserts an extra space between the character and comma.
> But this is so minor that you could push the patch
> if you think it's not worth making code more complex
> by using rmc only when hints are specified.

That will save some space.  Remember though that many times the key will not appear in the hint. In that case it would be nice to have a connector character of some kind, e.g. `|':expreg.


[-- Attachment #2: Type: text/html, Size: 2379 bytes --]

Re: master 0023891e842: Support hints in the :repeat keyword of defvar-keymap for repeat-mode

[Eshel Yaron]

JD Smith <jdtsmith@gmail.com> writes:

>>>  Lastly, it could be nice to make the appearance of the prompt more in
>>>  line with the read-multiple-choice prompt, which is conceptually very
>>>  similar.  Crucially, if the key is a character that occurs in the label,
>>>  it's better, IMO, to highlight the first occurrence of that character in
>>>  the label (as r-m-c does) than to prefix the label with that character.
>>>  We can reuse rmc--add-key-description for that purpose, as follows:
>>
>>  Nice idea.  A small problem is that when hints are not used,
>>  it inserts an extra space between the character and comma.

Well spotted.  How about something like the diff below?

> That will save some space.  Remember though that many times the key
> will not appear in the hint. In that case it would be nice to have a
> connector character of some kind, e.g. `|':expreg.

IIUC, the "connector character" in this example is the colon, right?  If
so, in read-multiple-choice we use a space as the connector character,
do you think a colon is preferable?


diff --git a/lisp/repeat.el b/lisp/repeat.el
index 412afc35ba7..efca965a533 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -558,14 +558,13 @@ repeat-echo-message-string
     (format-message
      "Repeat with %s%s"
      (mapconcat (lambda (key-cmd)
-                  (let* ((key (car key-cmd))
-                         (cmd (cdr key-cmd))
-                         (hint (when (symbolp cmd)
-                                 (get cmd 'repeat-hint))))
-                    (substitute-command-keys
-                     (format "\\`%s'%s"
-                             (key-description (vector key))
-                             (if hint (format ":%s" hint) "")))))
+                  (let ((key (car key-cmd))
+                        (cmd (cdr key-cmd)))
+                    (if-let ((hint (and (symbolp cmd)
+                                        (get cmd 'repeat-hint))))
+                        (cdr (rmc--add-key-description (list key hint)))
+                      (propertize (key-description (vector key))
+                                  'face 'read-multiple-choice-face))))
                 keys ", ")
      (if repeat-exit-key
          (substitute-command-keys

Re: master 0023891e842: Support hints in the :repeat keyword of defvar-keymap for repeat-mode

[Juri Linkov]

>>>>  Lastly, it could be nice to make the appearance of the prompt more in
>>>>  line with the read-multiple-choice prompt, which is conceptually very
>>>>  similar.  Crucially, if the key is a character that occurs in the label,
>>>>  it's better, IMO, to highlight the first occurrence of that character in
>>>>  the label (as r-m-c does) than to prefix the label with that character.
>>>>  We can reuse rmc--add-key-description for that purpose, as follows:
>>>
>>>  Nice idea.  A small problem is that when hints are not used,
>>>  it inserts an extra space between the character and comma.
>
> Well spotted.  How about something like the diff below?

This looks better, thanks.

Re: master 0023891e842: Support hints in the :repeat keyword of defvar-keymap for repeat-mode

[Eshel Yaron]

Juri Linkov <juri@linkov.net> writes:

>>>>>  Lastly, it could be nice to make the appearance of the prompt more in
>>>>>  line with the read-multiple-choice prompt, which is conceptually very
>>>>>  similar.  Crucially, if the key is a character that occurs in the label,
>>>>>  it's better, IMO, to highlight the first occurrence of that character in
>>>>>  the label (as r-m-c does) than to prefix the label with that character.
>>>>>  We can reuse rmc--add-key-description for that purpose, as follows:
>>>>
>>>>  Nice idea.  A small problem is that when hints are not used,
>>>>  it inserts an extra space between the character and comma.
>>
>> Well spotted.  How about something like the diff below?
>
> This looks better, thanks.

Great, pushed to master (commit 840c33070dc).