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

[Paul Nelson <ultrono@gmail.com>]

Juri Linkov <juri@linkov.net> writes:

>> I think the repeat-map property is fine as is, but the logic should be a
>> bit different:
>>
>> - If a command is called while repeat map MAP is active, then:
>>
>>   - If the command's repeat-continue-only property contains MAP, then
>>     MAP remains active (i.e., that map "continues").  [Maybe a better
>>     name for the property would be simply "repeat-continue".]
>
> So the property is renamed now to 'repeat-continue'.
>
>>   - Otherwise, if the command has a repeat-map property, then that
>>     becomes the new repeat map.
>>
>>   - Otherwise, the repeat map deactivates.
>>
>> - If a command is called while no repeat map is active, then we proceed
>>   as before.
>>
>> Do you agree that this is the desired behavior?  The difference is that
>> with the current implementation, only the repeat-map property is
>> relevant for determining which repeat map a command activates or
>> continues.
>
> Thanks for the clear explanation.  Now this is completely implemented
> with tests.

Thanks Juri, I tried it out and have noticed an issue.  Consider as
before the example:

(defvar-keymap repeat-list-map
  :doc "Keymap for repeating sequences."
  :repeat ( :continue (yank undo))
  "n" 'forward-list
  "p" 'backward-list
  "C-/" 'undo
  "y" 'yank)

(defvar-keymap repeat-paragraph-map
  :doc "Keymap for repeating sequences."
  :repeat ( :continue (yank undo))
  "]" 'forward-paragraph
  "}" 'forward-paragraph
  "[" 'backward-paragraph
  "{" 'backward-paragraph
  "C-/" 'undo
  "y" 'yank)

If we now do forward-list followed by two repeat-map yanks (i.e., C-M-n
y y), then we wind up in repeat-paragraph-map (but we should have never
left repeat-list-map).

To diagnose, let's look at (symbol-plist #'yank):

(... repeat-continue (repeat-paragraph-map repeat-list-map)
     repeat-map repeat-paragraph-map)

Here the precise value of the repeat-map property of yank should not
affect behavior (and so that property should not exist), but it does,
due to the way repeat-post-hook is currently coded.  What happens is
that when we are continuing a repeat map via repeat-continue, the value
of map-sym is discarded in the evaluation of

  (repeat-get-map map-sym)

but gets used later in

  (setq repeat-in-progress map-sym)

In such cases, we should instead (somehow) first modify map-sym so that
it is the symbol describing the "continue" map whose value is ultimately
returned via (repeat-get-map map-sym).  I'm not sure how best to do so.

Hope that makes sense; happy to clarify or think further if it'd help.

Paul