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

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

[Ramesh Nedunchezian]

28.0.50; diff-hl should use `repeat-mode' ... and not `smartrep'.

Commentary in `diff-hl' says

    ;; `diff-hl-mode' highlights uncommitted changes on the side of the
    ;; window (using the fringe, by default), allows you to jump between
    ;; the hunks and revert them selectively.

    ;; Provided commands:
    ;;
    ;; `diff-hl-diff-goto-hunk'  C-x v =
    ;; `diff-hl-revert-hunk'     C-x v n
    ;; `diff-hl-previous-hunk'   C-x v [
    ;; `diff-hl-next-hunk'       C-x v ]
    ;;
    ;; The mode takes advantage of `smartrep' if it is installed.

FWIW, `smartrep' is not part of GNU Emacs or GNU ELPA but available
elsewhere.  (I install it via MELPA)

Now, that `repeat-mode' is part of GNU Emacs, I think this dependency
on `smartrep' can be removed.

FWIW, this is what I have in my `.emacs'.

    (progn
      (defvar diff-hl--repeat-map
	(let
	    ((map
	      (make-sparse-keymap)))
	  map))
      (message "Installed %s" 'diff-hl--repeat-map)
      (cl-loop for
	       (cmd . key)
	       in
	       '((diff-hl-diff-goto-hunk . "=")
		 (diff-hl-revert-hunk . "n")
		 (diff-hl-previous-hunk . "[")
		 (diff-hl-next-hunk . "]"))
	       do
	       (define-key diff-hl--repeat-map key cmd)
	       (put cmd 'repeat-map 'diff-hl--repeat-map)))

----------------

In GNU Emacs 28.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-03-20 built on debian
Repository revision: 31544bc908d35bff513450bc4bea1d0283a7ddb0
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Debian GNU/Linux bullseye/sid

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

[Dmitry Gutov]

Hi!

On 02.04.2021 08:40, Ramesh Nedunchezian wrote:
> FWIW, `smartrep' is not part of GNU Emacs or GNU ELPA but available
> elsewhere.  (I install it via MELPA)
> 
> Now, that `repeat-mode' is part of GNU Emacs, I think this dependency
> on `smartrep' can be removed.

I'd be happy to migrate away from smartrep, as soon as the functionality 
is equal, and all supported Emacs versions have repeat-mode.

> FWIW, this is what I have in my `.emacs'.
> 
>      (progn
>        (defvar diff-hl--repeat-map
> 	(let
> 	    ((map
> 	      (make-sparse-keymap)))
> 	  map))
>        (message "Installed %s" 'diff-hl--repeat-map)
>        (cl-loop for
> 	       (cmd . key)
> 	       in
> 	       '((diff-hl-diff-goto-hunk . "=")
> 		 (diff-hl-revert-hunk . "n")
> 		 (diff-hl-previous-hunk . "[")
> 		 (diff-hl-next-hunk . "]"))
> 	       do
> 	       (define-key diff-hl--repeat-map key cmd)
> 	       (put cmd 'repeat-map 'diff-hl--repeat-map)))

Try this alternative too, seems shorter:

(map-keymap
  (lambda (_key cmd)
    (put cmd 'repeat-map 'diff-hl-command-map))
  diff-hl-command-map)

But either version (together with enabling repeat-mode) seems to break 
diff-hl-revert-hunk: as soon as it reaches the the y-or-n prompt, and I 
try to answer 'n', it enters the repeat loop again, instead of sending 
the result to the command.

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

[Juri Linkov]

>>        (cl-loop for
>> 	       (cmd . key)
>> 	       in
>> 	       '((diff-hl-diff-goto-hunk . "=")
>> 		 (diff-hl-revert-hunk . "n")
>> 		 (diff-hl-previous-hunk . "[")
>> 		 (diff-hl-next-hunk . "]"))
>> 	       do
>> 	       (define-key diff-hl--repeat-map key cmd)
>> 	       (put cmd 'repeat-map 'diff-hl--repeat-map)))
>
> Try this alternative too, seems shorter:
>
> (map-keymap
>  (lambda (_key cmd)
>    (put cmd 'repeat-map 'diff-hl-command-map))
>  diff-hl-command-map)
>
> But either version (together with enabling repeat-mode) seems to break
> diff-hl-revert-hunk: as soon as it reaches the the y-or-n prompt, and I try
> to answer 'n', it enters the repeat loop again, instead of sending the
> result to the command.

Could you provide a minimal test case?  I tried with:

#+begin_src emacs-lisp
(defun hl-test ()
  (interactive)
  (message "OK")
  (y-or-n-p "Yes? "))

(defvar hl-repeat-map
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "r") 'hl-test)
    map)
  "Keymap to repeat hl-test.")
(put 'hl-test 'repeat-map 'hl-repeat-map)
#+end_src

Then typing `M-x hl-test RET', and `y r y r y r ...' keeps the loop.

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

[Dmitry Gutov]

Hi Juri,

On 05.04.2021 23:43, Juri Linkov wrote:
> Could you provide a minimal test case?  I tried with:
> 
> #+begin_src emacs-lisp
> (defun hl-test ()
>    (interactive)
>    (message "OK")
>    (y-or-n-p "Yes? "))
> 
> (defvar hl-repeat-map
>    (let ((map (make-sparse-keymap)))
>      (define-key map (kbd "r") 'hl-test)
>      map)
>    "Keymap to repeat hl-test.")
> (put 'hl-test 'repeat-map 'hl-repeat-map)
> #+end_src
> 
> Then typing `M-x hl-test RET', and `y r y r y r ...' keeps the loop.

Here you go:

(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.

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.

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

[Dmitry Gutov]

On 08.04.2021 21:57, Juri Linkov wrote:
> Thanks for the detailed test case.  Now it's fixed in 580c4c6510.

Thanks! repeat-mode is looking good now (*).

I've added integration for it to diff-hl master, and with that we can 
close this issue.

Thanks all.

(*) Though the first impression, in comparison, was that it is too 
chatty. The hints are definitely helpful for discovery at first, though.

Maybe something like this would be an improvement? Experimental code 
warning.

diff --git a/lisp/repeat.el b/lisp/repeat.el
index b3c58f2f81..e704e4da56 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -404,7 +404,7 @@ repeat-post-hook
                                         (key-description repeat-exit-key))
                               ""))))
                  (if (current-message)
-                    (message "%s [%s]" (current-message) mess)
+                    (message #("%s [%s]" 3 7 (face deemphasized)) 
(current-message) mess)
                    (message mess))))

              ;; Adding an exit key

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

[Ramesh Nedunchezian]

On 10/04/21 7:10 am, Dmitry Gutov wrote:

> (*) Though the first impression, in comparison, was that it is too chatty. The hints are definitely helpful for discovery at first, though.
> 
> Maybe something like this would be an improvement? Experimental code warning.
> 
> diff --git a/lisp/repeat.el b/lisp/repeat.el
> index b3c58f2f81..e704e4da56 100644
> --- a/lisp/repeat.el
> +++ b/lisp/repeat.el
> @@ -404,7 +404,7 @@ repeat-post-hook
>                                         (key-description repeat-exit-key))
>                               ""))))
>                  (if (current-message)
> -                    (message "%s [%s]" (current-message) mess)
> +                    (message #("%s [%s]" 3 7 (face deemphasized)) (current-message) mess)
>                    (message mess))))
> 
>              ;; Adding an exit key

I created a repeat map for rectangle commands, and the echo area
becomes much more chattier. See
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47688.

It seems to me that repeat-mode is essentially a poor man's hydra.
May be the the symbol that holds the repeat map can specify a
`:help'-er property.

This `:help'-er can either

(a) give a fancy help string which the `repet-mode' can display in a
    pop-up

or

(b) the helper itself can prepare the string and arrange for providing
    a pop-up.

What I am saying is let the repeat map provide it's own `:help'-er
which the `repeat-mode' can hook in to.

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

[Juri Linkov]

> It seems to me that repeat-mode is essentially a poor man's hydra.

It is.  It provides only the basic functionality.
Don't expect many fancy things from repeat-mode.

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

[Ramesh Nedunchezian]

On 11/04/21 4:29 am, Juri Linkov wrote:
>> It seems to me that repeat-mode is essentially a poor man's hydra.
> 
> It is.  It provides only the basic functionality.
> Don't expect many fancy things from repeat-mode.

The code below is pulled out from the existing code i.e., it doesn't
add any additional layer of complexity or does anything fanciful.  But
it does add a bit of extensibility.

This is what I was suggesting:

    (defvar repeat-mode-helper
      (defun repeat-mode-helper (map)
	(let (keys)
	  (message "Coming here")
	  (map-keymap (lambda (key _) (push key keys)) map)
	  (let ((mess (format-message
		       "Repeat with %s%s"
		       (mapconcat (lambda (key)
				    (key-description (vector key)))
				  keys ", ")
		       (if repeat-exit-key
			   (format ", or exit with %s"
				   (key-description repeat-exit-key))
			 ""))))
	    (if (current-message)
		(message "%s [%s]" (current-message) mess)
	      (message mess))))))


And in `repeat-post-hook', do something like

    ;; Messaging
    (unless prefix-arg
      (funcall (or (get rep-sym 'help) repeat-mode-helper) rep-map))


For those who don't want hints, they can use

    (setq repeat-mode-helper #'ignore)

For those who want hints, but do /not/ want the hints hogging the echo
area, they could have their own custom helper like the one above,
after replacing

	(message mess)

with

	(tooltip-show mess)

--------------------------------

I would also appreciate if you could assess adding the exit key as a
property to the repeat mode symbol.

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

[Juri Linkov]

> This is what I was suggesting:
>
>     (defvar repeat-mode-helper
>       (defun repeat-mode-helper (map)
>[...]
>
> For those who don't want hints, they can use
>
>     (setq repeat-mode-helper #'ignore)

Thanks, this is a good idea.  Maybe the name is not the best one
since the word "helper" often means a place that provides some
utility functions.

Using the Emacs terminology, a better name would be 'repeat-mode-echo'.
I'll add such option soon.

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

[Juri Linkov]

> For those who don't want hints, they can use
>
>     (setq repeat-mode-helper #'ignore)
>
> For those who want hints, but do /not/ want the hints hogging the echo
> area, they could have their own custom helper like the one above,
> after replacing
>
> 	(message mess)
>
> with
>
> 	(tooltip-show mess)

This is implemented now, so you can customize it to disable messaging,
to do truncation of long messages, or to use tooltip-show, etc.

> I would also appreciate if you could assess adding the exit key as a
> property to the repeat mode symbol.

The exit key specific to some keymap can be easily added to that keymap.

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

[Juri Linkov]

> Maybe something like this would be an improvement? Experimental
> code warning.
>
> diff --git a/lisp/repeat.el b/lisp/repeat.el
> index b3c58f2f81..e704e4da56 100644
> --- a/lisp/repeat.el
> +++ b/lisp/repeat.el
> @@ -404,7 +404,7 @@ repeat-post-hook
>                                         (key-description repeat-exit-key))
>                               ""))))
>                  (if (current-message)
> -                    (message "%s [%s]" (current-message) mess)
> +                    (message #("%s [%s]" 3 7 (face deemphasized)) (current-message) mess)

I can't find the face named 'deemphasized'.

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

[Dmitry Gutov]

On 11.04.2021 01:58, Juri Linkov wrote:
> I can't find the face named 'deemphasized'.

Sorry.

Apparently, it's a face that only the tango-plus theme defines (unwisely).

It simply looks like less contrasting text. Try 'shadow' instead, it's 
about the same.

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

[Juri Linkov]

>> I can't find the face named 'deemphasized'.
>
> Sorry.
>
> Apparently, it's a face that only the tango-plus theme defines (unwisely).
>
> It simply looks like less contrasting text. Try 'shadow' instead, it's
> about the same.

I'm not sure about deemphasizing by default.  But now it's easy to
customize this, and add any emphazing.

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

[Dmitry Gutov]

On 12.04.2021 19:17, Juri Linkov wrote:
>>> I can't find the face named 'deemphasized'.
>>
>> Sorry.
>>
>> Apparently, it's a face that only the tango-plus theme defines (unwisely).
>>
>> It simply looks like less contrasting text. Try 'shadow' instead, it's
>> about the same.
> 
> I'm not sure about deemphasizing by default.  But now it's easy to
> customize this, and add any emphazing.

Thanks.

Consider adding a repeat-mode-echo option which follows what Smartrep 
does: instead of adding to the echo area, it just highlights the changed 
state in the mode line (basically a long mode lighter saying 
"========SMARTREP=======") and also colors the mode-line green.

IME that works well as an indicator that we're in a "special" state.

Though perhaps we could use a smaller text (like "[REPEAT]") and only 
color the text inside the brackets, rather than the whole mode-line. 
Could use some experimentation.

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

[Kévin Le Gouguec]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Consider adding a repeat-mode-echo option which follows what Smartrep
> does: instead of adding to the echo area, it just highlights the
> changed state in the mode line (basically a long mode lighter saying
> "========SMARTREP=======") and also colors the mode-line green.
>
> IME that works well as an indicator that we're in a "special" state.
>
> Though perhaps we could use a smaller text (like "[REPEAT]") and only
> color the text inside the brackets, rather than the whole
> mode-line. Could use some experimentation.

An entry in mode-line-modes, à la compilation-in-progress, would be
lovely IMO.

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

[Juri Linkov]

>> Consider adding a repeat-mode-echo option which follows what Smartrep
>> does: instead of adding to the echo area, it just highlights the
>> changed state in the mode line (basically a long mode lighter saying
>> "========SMARTREP=======") and also colors the mode-line green.
>>
>> IME that works well as an indicator that we're in a "special" state.
>>
>> Though perhaps we could use a smaller text (like "[REPEAT]") and only
>> color the text inside the brackets, rather than the whole
>> mode-line. Could use some experimentation.
>
> An entry in mode-line-modes, à la compilation-in-progress, would be
> lovely IMO.

compilation-in-progress is a good example of an unobtrusive indicator
in the mode line.  Now added to repeat-mode.

PS: like your message-subject-re-regexp turned the subject into
"Re: peat lambda" on emacs-devel, it removed the bug number
from the subject here :)

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

[Kévin Le Gouguec]

Juri Linkov <juri@linkov.net> writes:

>> An entry in mode-line-modes, à la compilation-in-progress, would be
>> lovely IMO.
>
> compilation-in-progress is a good example of an unobtrusive indicator
> in the mode line.  Now added to repeat-mode.

Thanks!  Was this bit intentional?

> ;;;###autoload
> (put 'mode-line-defining-kbd-macro 'risky-local-variable t)

Also, AFAICT the indication (whether in the mode-line or in the echo
area) only shows up once I start repeating a key (e.g. after C-x o o),
not when I first hit a repeatable key (e.g. after C-x o).

Is this intended?  I figure it would make sense to display the
indication before the user starts repeating, since the indication can
serve as a warning that repetition might occur.


At any rate, thanks a lot for working on this.  (And thanks to all for
the addition to diff-hl!)


> PS: like your message-subject-re-regexp turned the subject into
> "Re: peat lambda" on emacs-devel, it removed the bug number
> from the subject here :)

🤦 Thanks for the heads-up.  IIUC message-subject-re-regexp is used both
by Gnus summary buffers to "canonicalize" subjects in order to only show
them for thread roots, *and* by message-mode to strip cruft before
adding "Re:" to a reply.  I'll need to figure a way to decorrelate those
two uses…

(This is completely off-topic of course; I'm just thinking out loud)

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

[Juri Linkov]

> Thanks!  Was this bit intentional?
>
>> ;;;###autoload
>> (put 'mode-line-defining-kbd-macro 'risky-local-variable t)

Thanks for noticing this, I stole this from bindings.el,
but now that repeat-echo-mode-line-string is highlighted
correctly in the mode-line, I wonder if such property is needed
for repeat-echo-mode-line-string at all?  And why it was needed
for mode-line-defining-kbd-macro to keep its text properties?

After removing this line from bindings.el:
  (put 'mode-line-defining-kbd-macro 'risky-local-variable t)
mode-line-defining-kbd-macro loses its text property
face=font-lock-warning-face, but repeat-echo-mode-line-string keeps it.

This is explained in (info "(elisp) Properties in Mode"),
but I don't understand why text properties are preserved for
repeat-echo-mode-line-string without ‘risky-local-variable’ property?

> Also, AFAICT the indication (whether in the mode-line or in the echo
> area) only shows up once I start repeating a key (e.g. after C-x o o),
> not when I first hit a repeatable key (e.g. after C-x o).

Strange, I see it immediately after C-x o.

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

[Juri Linkov]

>> Also, AFAICT the indication (whether in the mode-line or in the echo
>> area) only shows up once I start repeating a key (e.g. after C-x o o),
>> not when I first hit a repeatable key (e.g. after C-x o).
>
> Strange, I see it immediately after C-x o.

Could you please try to move 'force-mode-line-update' out of condition,
so it updates the mode-line on activating the repeat map as well:

#+begin_src emacs-lisp
(defun repeat-echo-mode-line (map)
  "Display the repeat indicator in the mode line."
  (if map
      (unless (assq 'repeat-in-progress mode-line-modes)
        (add-to-list 'mode-line-modes (list 'repeat-in-progress
                                            repeat-echo-mode-line-string))))
  (force-mode-line-update t))
#+end_src

Maybe your mode-line is not updated immediately?

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

[Kévin Le Gouguec]

Juri Linkov <juri@linkov.net> writes:

>> Also, AFAICT the indication (whether in the mode-line or in the echo
>> area) only shows up once I start repeating a key (e.g. after C-x o o),
>> not when I first hit a repeatable key (e.g. after C-x o).
>
> Strange, I see it immediately after C-x o.

🤦 My apologies, I should have tried with emacs -Q before jumping to
conclusions; I actually had left C-x o bound to my own command…  I hope
I didn't waste too much of your time on this.

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

[Juri Linkov]

>>> Also, AFAICT the indication (whether in the mode-line or in the echo
>>> area) only shows up once I start repeating a key (e.g. after C-x o o),
>>> not when I first hit a repeatable key (e.g. after C-x o).
>>
>> Strange, I see it immediately after C-x o.
>
> 🤦 My apologies, I should have tried with emacs -Q before jumping to
> conclusions; I actually had left C-x o bound to my own command…  I hope
> I didn't waste too much of your time on this.

No problem.  I just wondered why the mode-line is automatically updated
when repeat-in-progress in mode-line-modes is non-nil, but not updated
when it becomes nil.

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

[Dmitry Gutov]

On 14.04.2021 21:10, Juri Linkov wrote:
> compilation-in-progress is a good example of an unobtrusive indicator
> in the mode line.  Now added to repeat-mode.

Looks nice, thank you.

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

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

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> 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.

I think that sounds correct.

> 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'.

Sounds promising.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Juri Linkov]

>> 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.
>
> I think that sounds correct.
>
>> 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'.
>
> Sounds promising.

So the fix is pushed now.