bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[JD Smith]

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

repeat-mode is excellent for informal and very lightweight modal interfaces (great article <https://karthinks.com/software/it-bears-repeating/>).  One limitation it has compared to other such interfaces is it only mentions the keys which can be repeated, without providing hints about what they do.  

A simple patch to `repeat-echo-message-strings' to support displaying a string for keymap definitions of form (STRING . DEFN) is below.  It works nicely to provide "hints" after each repeat key, if the keymap includes them.

Example usage:

(defvar-keymap expreg-repeat-map
    :doc "Repeat map for `expreg' actions."
    :repeat t
    :name "expreg"
    "\\" (cons "expand" 'expreg-expand)
    "|"  (cons "contract"  'expreg-contract))

Example prompt (after one invocation of `expreg-expand'): 

 Repeat with \:expand, |:contract

It might also be useful to provide a custom variable by which a user can disable hint strings, for the experts/minimalists..

Patch:

--- repeat_old.el	2024-04-25 18:22:59
+++ repeat.el	2024-04-25 18:09:11
@@ -553,12 +553,20 @@
 (defun repeat-echo-message-string (keymap)
   "Return a string with the list of repeating keys in KEYMAP."
   (let (keys)
-    (map-keymap (lambda (key cmd) (and cmd (push key keys))) keymap)
+    (map-keymap (lambda (key cmd)
+		  (if (consp cmd)
+		      (push (cons (car cmd) key) keys)
+		    (when cmd (push key keys))))
+		keymap)
     (format-message "Repeat with %s%s"
                     (mapconcat (lambda (key)
-                                 (substitute-command-keys
-                                  (format "\\`%s'"
-                                          (key-description (vector key)))))
+				 (let* ((cmd (when (consp key) (format ":%s" (car key)))))
+				   (if (consp key) (setq key (cdr key)))
+				   (concat
+				    (substitute-command-keys
+				     (format "\\`%s'"
+					     (key-description (vector key))))
+				    cmd)))
                                keys ", ")
                     (if repeat-exit-key
                         (substitute-command-keys


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

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[Juri Linkov]

> Example usage:
>
> (defvar-keymap expreg-repeat-map
>     :doc "Repeat map for `expreg' actions."
>     :repeat t
>     :name "expreg"
>     "\\" (cons "expand" 'expreg-expand)
>     "|"  (cons "contract"  'expreg-contract))

I feel uneasy about reusing the form (STRING . DEFN)
that has another meaning:

 a cons (STRING . DEFN), meaning that DEFN is the definition
    (DEFN should be a valid definition in its own right) and
    STRING is the menu item name (which is used only if the containing
    keymap has been created with a menu name, see make-keymap),

Can the same instead be achieved by using symbol properties?
For example:

(defvar-keymap expreg-repeat-map
    :doc "Repeat map for `expreg' actions."
    :repeat t
    :name "expreg"
    "\\" 'expreg-expand
    "|"  'expreg-contract)

(put 'expreg-expand 'repeat-hint "expand")
(put 'expreg-contract 'repeat-hint "contract")

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[JD Smith]

> On Apr 26, 2024, at 2:06 AM, Juri Linkov <juri@linkov.net> wrote:
> 
> I feel uneasy about reusing the form (STRING . DEFN)
> that has another meaning:
> 
> a cons (STRING . DEFN), meaning that DEFN is the definition
>    (DEFN should be a valid definition in its own right) and
>    STRING is the menu item name (which is used only if the containing
>    keymap has been created with a menu name, see make-keymap),
> 
> Can the same instead be achieved by using symbol properties?
> For example:

> (defvar-keymap expreg-repeat-map
>    :doc "Repeat map for `expreg' actions."
>    :repeat t
>    "\\" 'expreg-expand
>    "|"  'expreg-contract)
> 
> (put 'expreg-expand 'repeat-hint "expand")
> (put 'expreg-contract 'repeat-hint "contract")


Thanks for taking a look.  A property on the command could work (and then you need no :name).  It would be somewhat harder to maintain the structure for long keymaps, e.g. if a command changes.  I do note that menu-item with :filter is commonly recommended for non-menu dynamic bindings, so there is some precedent of reusing menu functionality in other contexts.  Of course a new wrapper macro could also be developed to ease this.  Do you anticipate any specific issues?

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[Juri Linkov]

>> (defvar-keymap expreg-repeat-map
>>    :doc "Repeat map for `expreg' actions."
>>    :repeat t
>>    "\\" 'expreg-expand
>>    "|"  'expreg-contract)
>>
>> (put 'expreg-expand 'repeat-hint "expand")
>> (put 'expreg-contract 'repeat-hint "contract")
>
> Thanks for taking a look.  A property on the command could work (and then
> you need no :name).  It would be somewhat harder to maintain the structure
> for long keymaps, e.g. if a command changes.  I do note that menu-item with
> :filter is commonly recommended for non-menu dynamic bindings, so there is
> some precedent of reusing menu functionality in other contexts.

As a more complicated feature, usually :filter is added by developers for
specific functionality, it's not intended to be used by users in their normal
customization of keymaps, like users will configure hints.

> Of course a new wrapper macro could also be developed to ease this.
> Do you anticipate any specific issues?

Here is what I recommend to do to simplify the definition of hints for users.

Like there can be a list of enter and exit commands in 'defvar-keymap':

  :repeat ‘(:enter (commands ...) :exit (commands ...))’

the same list could be used for hints:

  :repeat ‘(:enter (commands ...) :exit (commands ...) :hints ((command . hint) ...))’

This should be easier to document and to understand by users than
a special syntax of cons for binding and the requirement to add :name.

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[JD Smith]

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



> On Apr 28, 2024, at 2:58 AM, Juri Linkov <juri@linkov.net> wrote
>> Of course a new wrapper macro could also be developed to ease this.
>> Do you anticipate any specific issues?
> 
> Here is what I recommend to do to simplify the definition of hints for users.
> 
> Like there can be a list of enter and exit commands in 'defvar-keymap':
> 
>  :repeat ‘(:enter (commands ...) :exit (commands ...))’
> 
> the same list could be used for hints:
> 
>  :repeat ‘(:enter (commands ...) :exit (commands ...) :hints ((command . hint) ...))’
> 
> This should be easier to document and to understand by users than
> a special syntax of cons for binding and the requirement to add :name.

Though it will require repetition and thus be subject to changed-here-but-not-there errors, this does look good and nicely groups the information under :repeat.  So this should work for all commands in the keymap:

> (defvar-keymap expreg-repeat-map
>    :doc "Repeat map for `expreg' actions."
>    :repeat '(:hints ((expreg-expand . "expand") (expreg-contract . "contract")))
>    "\\" 'expreg-expand
>    "|"  'expreg-contract)


If a hint is missing for a command, it should just have its key mentioned.  It looks like your idea would require changes to defvar-keymap.  Do you want to propose a patch?  We'd need some way to pass the hints in; perhaps the macro could set a property on the command symbol as you initially proposed.

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

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[Juri Linkov]

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

>> (defvar-keymap expreg-repeat-map
>>    :doc "Repeat map for `expreg' actions."
>>    :repeat '(:hints ((expreg-expand . "expand") (expreg-contract . "contract")))
>>    "\\" 'expreg-expand
>>    "|"  'expreg-contract)
>
> If a hint is missing for a command, it should just have its key mentioned.
> It looks like your idea would require changes to defvar-keymap.  Do you
> want to propose a patch?  We'd need some way to pass the hints in; perhaps
> the macro could set a property on the command symbol as you initially
> proposed.

Alright, let's add this to defvar-keymap.  Please try the following patch.

PS: Do you still think a new custom variable should be added to be able to
disable hint strings?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: repeat-hint.patch --]
[-- Type: text/x-diff, Size: 3706 bytes --]

diff --git a/lisp/keymap.el b/lisp/keymap.el
index b2b475c7d71..cbd26f1060e 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -603,10 +603,11 @@ defvar-keymap
 symbol property.
 
 More control is available over which commands are repeatable; the
-value can also be a property list with properties `:enter' and
-`:exit', for example:
+value can also be a property list with properties `:enter',
+`:exit' and `:hints', for example:
 
-     :repeat (:enter (commands ...) :exit (commands ...))
+     :repeat (:enter (commands ...) :exit (commands ...)
+              :hints ((command . \"hint\") ...))
 
 `:enter' specifies the list of additional commands that only
 enter `repeat-mode'.  When the list is empty, then only the
@@ -621,6 +622,10 @@ defvar-keymap
 in this specific map, but should not have the `repeat-map' symbol
 property.
 
+`:hints' is a list of cons pairs where car is a command and
+cdr is a string that is displayed alongside of the repeatable key
+in the echo area.
+
 \(fn VARIABLE-NAME &key DOC FULL PARENT SUPPRESS NAME PREFIX KEYMAP REPEAT &rest [KEY DEFINITION]...)"
   (declare (indent 1))
   (let ((opts nil)
@@ -660,7 +665,9 @@ defvar-keymap
           (setq def (pop defs))
           (when (and (memq (car def) '(function quote))
                      (not (memq (cadr def) (plist-get repeat :exit))))
-            (push `(put ,def 'repeat-map ',variable-name) props)))))
+            (push `(put ,def 'repeat-map ',variable-name) props)))
+        (dolist (def (plist-get repeat :hints))
+          (push `(put ',(car def) 'repeat-hint ',(cdr def)) props))))
 
     (let ((defvar-form
            `(defvar ,variable-name
diff --git a/lisp/repeat.el b/lisp/repeat.el
index 0a59494c097..b9c96fa4e12 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -553,20 +553,27 @@ repeat--clear-prev
 (defun repeat-echo-message-string (keymap)
   "Return a string with the list of repeating keys in KEYMAP."
   (let (keys)
-    (map-keymap (lambda (key cmd) (and cmd (push key keys))) keymap)
-    (format-message "Repeat with %s%s"
-                    (mapconcat (lambda (key)
-                                 (substitute-command-keys
-                                  (format "\\`%s'"
-                                          (key-description (vector key)))))
-                               keys ", ")
-                    (if repeat-exit-key
-                        (substitute-command-keys
-                         (format ", or exit with \\`%s'"
-                                 (if (key-valid-p repeat-exit-key)
-                                     repeat-exit-key
-                                   (key-description repeat-exit-key))))
-                      ""))))
+    (map-keymap (lambda (key cmd) (and cmd (push (cons key cmd) keys)))
+                keymap)
+    (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) "")))))
+                keys ", ")
+     (if repeat-exit-key
+         (substitute-command-keys
+          (format ", or exit with \\`%s'"
+                  (if (key-valid-p repeat-exit-key)
+                      repeat-exit-key
+                    (key-description repeat-exit-key))))
+       ""))))
 
 (defun repeat-echo-message (keymap)
   "Display in the echo area the repeating keys defined by KEYMAP.

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[Juri Linkov]

close 70576 30.0.50
thanks

>>> (defvar-keymap expreg-repeat-map
>>>    :doc "Repeat map for `expreg' actions."
>>>    :repeat '(:hints ((expreg-expand . "expand") (expreg-contract . "contract")))
>>>    "\\" 'expreg-expand
>>>    "|"  'expreg-contract)
>>
>> If a hint is missing for a command, it should just have its key mentioned.
>> It looks like your idea would require changes to defvar-keymap.  Do you
>> want to propose a patch?  We'd need some way to pass the hints in; perhaps
>> the macro could set a property on the command symbol as you initially
>> proposed.
>
> Alright, let's add this to defvar-keymap.  Please try the following patch.

Thanks for the feature request.  This is pushed now.

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[Eli Zaretskii]

> Cc: 70576@debbugs.gnu.org
> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 02 May 2024 09:48:06 +0300
> 
> close 70576 30.0.50
> thanks
> 
> >>> (defvar-keymap expreg-repeat-map
> >>>    :doc "Repeat map for `expreg' actions."
> >>>    :repeat '(:hints ((expreg-expand . "expand") (expreg-contract . "contract")))
> >>>    "\\" 'expreg-expand
> >>>    "|"  'expreg-contract)
> >>
> >> If a hint is missing for a command, it should just have its key mentioned.
> >> It looks like your idea would require changes to defvar-keymap.  Do you
> >> want to propose a patch?  We'd need some way to pass the hints in; perhaps
> >> the macro could set a property on the command symbol as you initially
> >> proposed.
> >
> > Alright, let's add this to defvar-keymap.  Please try the following patch.
> 
> Thanks for the feature request.  This is pushed now.

Thanks, but please also update the ELisp manual and add an entry to
NEWS about this change.

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[JD Smith]

Thanks for working on this.  I'm sorry I didn't have a chance to test the patch, it looks good.  I don't think a disable variable is necessary for now, since most repeat maps are self-defined.

> On May 2, 2024, at 3:16 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> Cc: 70576@debbugs.gnu.org
>> From: Juri Linkov <juri@linkov.net>
>> Date: Thu, 02 May 2024 09:48:06 +0300
>> 
>> close 70576 30.0.50
>> thanks
>> 
>>>>> (defvar-keymap expreg-repeat-map
>>>>>   :doc "Repeat map for `expreg' actions."
>>>>>   :repeat '(:hints ((expreg-expand . "expand") (expreg-contract . "contract")))
>>>>>   "\\" 'expreg-expand
>>>>>   "|"  'expreg-contract)
>>>> 
>>>> If a hint is missing for a command, it should just have its key mentioned.
>>>> It looks like your idea would require changes to defvar-keymap.  Do you
>>>> want to propose a patch?  We'd need some way to pass the hints in; perhaps
>>>> the macro could set a property on the command symbol as you initially
>>>> proposed.
>>> 
>>> Alright, let's add this to defvar-keymap.  Please try the following patch.
>> 
>> Thanks for the feature request.  This is pushed now.
> 
> Thanks, but please also update the ELisp manual and add an entry to
> NEWS about this change.

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[Juri Linkov]

>> >>> (defvar-keymap expreg-repeat-map
>> >>>    :doc "Repeat map for `expreg' actions."
>> >>>    :repeat '(:hints ((expreg-expand . "expand") (expreg-contract . "contract")))
>> >>>    "\\" 'expreg-expand
>> >>>    "|"  'expreg-contract)
>> >>
>> >> If a hint is missing for a command, it should just have its key mentioned.
>> >> It looks like your idea would require changes to defvar-keymap.  Do you
>> >> want to propose a patch?  We'd need some way to pass the hints in; perhaps
>> >> the macro could set a property on the command symbol as you initially
>> >> proposed.
>> >
>> > Alright, let's add this to defvar-keymap.  Please try the following patch.
>>
>> Thanks for the feature request.  This is pushed now.
>
> Thanks, but please also update the ELisp manual and add an entry to
> NEWS about this change.

A NEWS entry was already added with "---" since I didn't expect that
such a small feature that is not used anywhere needs to grow
the size of the manual.

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: jdtsmith@gmail.com,  70576@debbugs.gnu.org
> Date: Thu, 02 May 2024 19:16:06 +0300
> 
> >> >>> (defvar-keymap expreg-repeat-map
> >> >>>    :doc "Repeat map for `expreg' actions."
> >> >>>    :repeat '(:hints ((expreg-expand . "expand") (expreg-contract . "contract")))
> >> >>>    "\\" 'expreg-expand
> >> >>>    "|"  'expreg-contract)
> >> >>
> >> >> If a hint is missing for a command, it should just have its key mentioned.
> >> >> It looks like your idea would require changes to defvar-keymap.  Do you
> >> >> want to propose a patch?  We'd need some way to pass the hints in; perhaps
> >> >> the macro could set a property on the command symbol as you initially
> >> >> proposed.
> >> >
> >> > Alright, let's add this to defvar-keymap.  Please try the following patch.
> >>
> >> Thanks for the feature request.  This is pushed now.
> >
> > Thanks, but please also update the ELisp manual and add an entry to
> > NEWS about this change.
> 
> A NEWS entry was already added with "---" since I didn't expect that
> such a small feature that is not used anywhere needs to grow
> the size of the manual.

I don't see how we can avoid updating the ELisp manual, when the other
values of :repeat are already documented there, see the node "Creating
Keymaps".

So please add the description of the new value to those already
documented there.

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[Juri Linkov]

>> > Thanks, but please also update the ELisp manual and add an entry to
>> > NEWS about this change.
>>
>> A NEWS entry was already added with "---" since I didn't expect that
>> such a small feature that is not used anywhere needs to grow
>> the size of the manual.
>
> I don't see how we can avoid updating the ELisp manual, when the other
> values of :repeat are already documented there, see the node "Creating
> Keymaps".
>
> So please add the description of the new value to those already
> documented there.

Indeed, without this the manual would be incomplete, so now added as well.

bug#70576: [PATCH] `repeat-echo-message-string': support repeat keymap "hints"

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: jdtsmith@gmail.com,  70576@debbugs.gnu.org
> Date: Fri, 03 May 2024 09:23:01 +0300
> 
> >> > Thanks, but please also update the ELisp manual and add an entry to
> >> > NEWS about this change.
> >>
> >> A NEWS entry was already added with "---" since I didn't expect that
> >> such a small feature that is not used anywhere needs to grow
> >> the size of the manual.
> >
> > I don't see how we can avoid updating the ELisp manual, when the other
> > values of :repeat are already documented there, see the node "Creating
> > Keymaps".
> >
> > So please add the description of the new value to those already
> > documented there.
> 
> Indeed, without this the manual would be incomplete, so now added as well.

Thanks.