bug#74140: [PATCH] Add :continue-only directive for repeat maps in bind-keys, use-package

[Juri Linkov <juri@linkov.net>]

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

>> Thanks for pointing out the case when the command is bound to the same key
>> globally and in the repeat map.  So checking for a key can't help here.
>>
>> Therefore, I implemented another solution in repeat.el.  There is now
>> a new symbol property 'repeat-continue-only'.  And a command with this
>> symbol property will not activate the repeat map, but will only continue
>> the already activated repeating sequence.   This is implemented by
>> a simple change:
>>
>>       (when (and (repeat-check-map map)
>>                  (or (null (repeat--command-property 'repeat-continue-only))
>>                      was-in-progress))
>>
>
> Thanks, I took a look.  One disadvantage of this approach is that it
> does not allow the same command to enter one repeat map and continue
> another.  I'll confess that I'm not aware of any such examples in my
> config, but it still seems like an undesirable "non-local" effect.

Other existing properties such as 'repeat-exit-timeout' or 'repeat-check-key'
don't accept a list of repeat maps, but only a non-nil value (including the
special symbol :no).  However, this can be changed gradually by adding
support for the list to them.

So below is a patch that adds support for a list of maps.
And users don't have to populate the list manually because
'defvar-keymap' does this automatically with :continue-only
in the same patch.

> This issue motivated my suggestion that the symbol property should be a
> list of repeat maps that the command continues, although I haven't
> considered the details (e.g., concerning the map vs. the symbol that
> points to it) - maybe you have a clearer picture of those.

Using a variable value for the map property is supported but not encouraged.
There are too many problems when not using a symbol.  So let's support
only symbols in the list of repeat maps for :continue-only.

>> BTW, while writing the 'bind-keys' test, I noticed there is no way
>> to specify a command that only activates, but not continues
>> (the same as :entry in 'defvar-keymap').  Is it true that 'bind-keys'
>> has no such keyword, so there is a need to do this explicitly with:
>>
>>   (put 'repeat-tests-bind-call-a 'repeat-map 'repeat-tests-bind-keys-repeat-map)
>
> Yes, that's also my understanding (but it's not clear to me that it
> requires such a keyword if its purpose is to bind keys in a map).

My goal was to write the same test for 'bind-keys'
as for 'defvar-keymap' in test/lisp/repeat-tests.el:

  (bind-keys
   :map repeat-tests-bind-keys-map
   ("C-M-a" . repeat-tests-bind-call-a)
   ("C-M-o" . repeat-tests-bind-call-o)
   :repeat-map repeat-tests-bind-keys-repeat-map
   :continue
   ("c"     . repeat-tests-bind-call-c)
   :continue-only
   ("C-M-o" . repeat-tests-bind-call-o)
   :exit
   ("q"     . repeat-tests-bind-call-q))

But I can't find a way to do the same what the :enter key
does in 'defvar-keymap', so needed to set the symbol explicitly:

  (put 'repeat-tests-bind-call-a 'repeat-map 'repeat-tests-bind-keys-repeat-map)


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

diff --git a/lisp/keymap.el b/lisp/keymap.el
index 9b133e1ca82..43c8d918ba7 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -687,6 +687,7 @@ defvar-keymap
 `:exit' and `:hints', for example:
 
      :repeat (:enter (commands ...) :exit (commands ...)
+              :continue-only (commands ...)
               :hints ((command . \"hint\") ...))
 
 `:enter' specifies the list of additional commands that only
@@ -702,6 +703,10 @@ defvar-keymap
 in this specific map, but should not have the `repeat-map' symbol
 property.
 
+`:continue-only' specifies the list of commands that should not
+enter `repeat-mode'.  These command should only continue the
+already activated repeating sequence.
+
 `: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.
@@ -740,6 +745,10 @@ defvar-keymap
             def)
         (dolist (def (plist-get repeat :enter))
           (push `(put ',def 'repeat-map ',variable-name) props))
+        (dolist (def (plist-get repeat :continue-only))
+          (push `(put ',def 'repeat-continue-only
+                      (cons ',variable-name (get ',def 'repeat-continue-only)))
+                props))
         (while defs
           (pop defs)
           (setq def (pop defs))
diff --git a/lisp/repeat.el b/lisp/repeat.el
index 11d26a477b6..45888d9db08 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -505,8 +505,12 @@ repeat-post-hook
     (setq repeat-in-progress nil)
     (let ((map (repeat-get-map)))
       (when (and (repeat-check-map map)
-                 (or (null (repeat--command-property 'repeat-continue-only))
-                     was-in-progress))
+                 (let ((continue-only (repeat--command-property 'repeat-continue-only)))
+                   (or (null continue-only)
+                       (and (or (not (consp continue-only))
+                                (memq (repeat--command-property 'repeat-map)
+                                      continue-only))
+                            was-in-progress))))
         ;; Messaging
         (funcall repeat-echo-function map)
 
diff --git a/test/lisp/repeat-tests.el b/test/lisp/repeat-tests.el
index c560a283039..527963e3ef8 100644
--- a/test/lisp/repeat-tests.el
+++ b/test/lisp/repeat-tests.el
@@ -63,17 +63,15 @@ repeat-tests-map
 
 (defvar-keymap repeat-tests-repeat-map
   :doc "Keymap for repeating sequences."
-  :repeat ( :enter (repeat-tests-call-a)
-            :exit  (repeat-tests-call-q))
+  :repeat ( :enter         (repeat-tests-call-a)
+            :continue-only (repeat-tests-call-o)
+            :exit          (repeat-tests-call-q))
   "a"     'ignore ;; for non-nil repeat-check-key only
   "c"     'repeat-tests-call-c
   "d"     'repeat-tests-call-d
   "C-M-o" 'repeat-tests-call-o
   "q"     'repeat-tests-call-q)
 
-;; TODO: add new keyword ':continue-only (repeat-tests-call-o)'
-(put 'repeat-tests-call-o 'repeat-continue-only t)
-
 ;; Test using a variable instead of the symbol:
 (put 'repeat-tests-call-b 'repeat-map repeat-tests-repeat-map)