bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not `smartrep'

[Juri Linkov]

>> (defun hl-test ()
>>   (interactive)
>>   (message "OK")
>>   (message "result: %s"
>>            (y-or-n-p "Yes? ")))
>>
>> (defvar hl-repeat-map
>>   (let ((map (make-sparse-keymap)))
>>     (define-key map (kbd "n") 'hl-test) ; Note the changed key binding.
>>     map)
>>   "Keymap to repeat hl-test.")
>>
>> (put 'hl-test 'repeat-map 'hl-repeat-map)
>>
>> To try it:
>>
>> 1. M-x hl-test.
>> 2. Press 'n' a few times.
>>
>> Expected behavior:
>>
>> It alternates between the prompt "Yes? " and message "result: nil".
>>
>> Actual behavior:
>>
>> It enters some sort of recursive state, only showing the prompt. I have to
>> press 'y' a bunch of times to get out of it.
>
> Thanks for the detailed test case.  Now it's fixed in 580c4c6510.

This was a pretty bad fix.  It broke a lot of useful workflows.
For example, when the minibuffer is active, it's not possible anymore
to switch from the minibuffer to the original buffer and back using 'C-x o o'.
Also multiple undo with 'C-x u u u ...' is not available in the
minibuffer anymore.  I'm trying to find a proper fix.

The first thing how I tried to handle the above test case is to
disable repeat-mode only in the minibuffer activated from y-or-n-p:

diff --git a/lisp/subr.el b/lisp/subr.el
index 8ff403e113..de5a512946 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3324,9 +3324,12 @@ y-or-n-p
                        map))
              ;; Protect this-command when called from pre-command-hook 
(bug#45029)
              (this-command this-command)
-             (str (read-from-minibuffer
-                   prompt nil keymap nil
-                   (or y-or-n-p-history-variable 'empty-history))))
+             (str (minibuffer-with-setup-hook
+                      (lambda ()
+                        (setq-local repeat-mode nil))
+                    (read-from-minibuffer
+                     prompt nil keymap nil
+                     (or y-or-n-p-history-variable 'empty-history)))))
         (setq answer (if (member str '("y" "Y")) 'act 'skip)))))
     (let ((ret (eq answer 'act)))
       (unless noninteractive

But it doesn't seems appropriate to mention repeat-mode in y-or-n-p.

The next thing tried was to detect the y-or-n-p minibuffer in repeat-mode:

diff --git a/lisp/repeat.el b/lisp/repeat.el
index ac08952eaa..53046714bd 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -417,7 +419,7 @@ repeat-post-hook
 
             ;; Exit when the last char is not among repeatable keys,
             ;; so e.g. `C-x u u' repeats undo, whereas `C-/ u' doesn't.
-            (when (and (zerop (minibuffer-depth)) ; avoid remapping in prompts
+            (when (and (not (eq (key-binding "n") 'y-or-n-p-insert-n))
                        (or (lookup-key map (this-command-keys-vector))
                            prefix-arg))
 
But special handling of y-or-n in repeat-mode seems inappropriate too.

What would be a general rule when repeating should be disabled?
Looks like the most relevant event is when the minibuffer pops up
in the middle of repeating sequence.  But minibuffer-depth can't be used
to detect changes in the minibuffer presence.  Because there are cases
when minibuffer-depth is not changed when the minibuffer is activated.

For example, with the above test case: after typing `M-x hl-test RET',
minibuffer-depth is 1 in pre-command-hook after typing RET.  Then the
next command activates own minibuffer with y-or-n-p, and again
minibuffer-depth is 1 in post-command-hook.

What I'm going to try is a combination of minibuffer-depth and
current-minibuffer-command.  Then in pre-command-hook,
current-minibuffer-command is 'execute-extended-command',
but in post-command-hook it's 'hl-test'.

Also worth to note that 'hl-test' that uses 'y-or-n-p' (and 'y-or-n-p'
preserves the original 'this-command') can be repeatable only with
the following patch.  Some time ago 'this-command' was replaced with
'real-this-command' on the request in bug#47688, but actually
need to check both 'this-command' and 'real-this-command':

diff --git a/lisp/repeat.el b/lisp/repeat.el
index ac08952eaa..c136b0cee4 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -408,6 +411,8 @@ repeat-post-hook
     (setq repeat-in-progress nil)
     (when repeat-mode
       (let ((rep-map (or repeat-map
+                         (and (symbolp this-command)
+                              (get this-command 'repeat-map))
                          (and (symbolp real-this-command)
                               (get real-this-command 'repeat-map)))))
         (when rep-map