[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not `smartr
From: |
Juri Linkov |
Subject: |
bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not `smartrep' |
Date: |
Sun, 14 Nov 2021 22:25:07 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (x86_64-pc-linux-gnu) |
>> (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',
Juri Linkov <=