* bug#51281: 28.0.60; repeat-mode issues
@ 2021-10-19 7:12 Juri Linkov
2021-10-20 17:30 ` Juri Linkov
0 siblings, 1 reply; 10+ messages in thread
From: Juri Linkov @ 2021-10-19 7:12 UTC (permalink / raw)
To: 51281
[-- Attachment #1: Type: text/plain, Size: 253 bytes --]
Tags: patch
Currently there is a bug where the prefix arg changed for the repeatable
commands, is applied to the next non-repeatable command. For example:
C-- C-x o o C-n
or
C-x o C-- o C-n
`C-n' moves up, not down. This patch fixes this bug:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: repeat-mode-fixes.patch --]
[-- Type: text/x-diff, Size: 3944 bytes --]
diff --git a/lisp/repeat.el b/lisp/repeat.el
index ee9e14b515..f4526c20f4 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -390,7 +390,10 @@ repeat-mode
See `describe-repeat-maps' for a list of all repeatable command."
:global t :group 'convenience
(if (not repeat-mode)
- (remove-hook 'post-command-hook 'repeat-post-hook)
+ (progn
+ (remove-hook 'pre-command-hook 'repeat-pre-hook)
+ (remove-hook 'post-command-hook 'repeat-post-hook))
+ (add-hook 'pre-command-hook 'repeat-pre-hook)
(add-hook 'post-command-hook 'repeat-post-hook)
(let* ((keymaps nil)
(commands (all-completions
@@ -402,27 +405,60 @@ repeat-mode
(length commands)
(length (delete-dups keymaps))))))
-(defun repeat-post-hook ()
- "Function run after commands to set transient keymap for repeatable keys."
- (let ((was-in-progress repeat-in-progress))
- (setq repeat-in-progress nil)
+(defun repeat-map ()
+ "Return a map for keys repeatable after the current command."
(when repeat-mode
(let ((rep-map (or repeat-map
(and (symbolp real-this-command)
(get real-this-command 'repeat-map)))))
(when rep-map
- (when (boundp rep-map)
+ (when (and (symbolp rep-map) (boundp rep-map))
(setq rep-map (symbol-value rep-map)))
- (let ((map (copy-keymap rep-map)))
+ (if repeat-exit-key
+ ;; `repeat-exit-key' modifies the map by adding keys
+ (copy-keymap rep-map)
+ rep-map)))))
+
+(defun repeat-map-valid (map)
+ "Check if MAP can be used for the next command.
+Can contain more conditions."
+ (and map
+ ;; Avoid using repeatable keys when minibuffer prompt pops up
+ (zerop (minibuffer-depth))
;; 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
- (or (lookup-key map (this-command-keys-vector))
- prefix-arg))
+ (or (lookup-key map (vector last-nonmenu-event))
+ ;; `prefix-arg' can affect next repeatable commands
+ ;; (and repeat-keep-prefix prefix-arg)
+ )))
+
+(defun repeat-pre-hook ()
+ "Function run before commands to handle repeatable keys."
+ ;; Reset prefix-arg before the next non-repeatable command,
+ ;; e.g. `C-- C-x o o C-n' or `C-x o C-- o C-n', so `C-n'
+ ;; should not use `prefix-arg' to go in opposite direction.
+ (when (and repeat-keep-prefix prefix-arg repeat-in-progress)
+ (let ((map (repeat-map)))
+ (if map
+ ;; Optimize to use less logic in `repeat-map'
+ ;; when called again from `repeat-post-hook'
+ (setq repeat-map map)
+ ;; When `repeat-post-hook' will exit the repeatable sequence,
+ ;; this means the current command is not repeatable,
+ ;; so reset `prefix-arg' enabled for repeatable commands only.
+ (setq prefix-arg nil)))))
+
+(defun repeat-post-hook ()
+ "Function run after commands to set transient keymap for repeatable keys."
+ (let ((was-in-progress repeat-in-progress))
+ (setq repeat-in-progress nil)
+
+ (let ((map (repeat-map)))
+ (when (repeat-map-valid map)
;; Messaging
- (unless prefix-arg
+ (unless prefix-arg ;; Don't overwrite prefix arg echo
(funcall repeat-echo-function map))
;; Adding an exit key
@@ -446,7 +482,7 @@ repeat-post-hook
(lambda ()
(setq repeat-in-progress nil)
(funcall exitfun)
- (funcall repeat-echo-function nil)))))))))))
+ (funcall repeat-echo-function nil))))))))
(setq repeat-map nil)
(when (and was-in-progress (not repeat-in-progress))
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#51281: 28.0.60; repeat-mode issues
2021-10-19 7:12 bug#51281: 28.0.60; repeat-mode issues Juri Linkov
@ 2021-10-20 17:30 ` Juri Linkov
2021-10-20 18:59 ` bug#51281: [External] : " Drew Adams
2021-11-04 23:24 ` Lars Ingebrigtsen
0 siblings, 2 replies; 10+ messages in thread
From: Juri Linkov @ 2021-10-20 17:30 UTC (permalink / raw)
To: 51281
[-- Attachment #1: Type: text/plain, Size: 582 bytes --]
> Currently there is a bug where the prefix arg changed for the repeatable
> commands, is applied to the next non-repeatable command. For example:
>
> C-- C-x o o C-n
> or
> C-x o C-- o C-n
Actually, this too confusing feature that allows changing prefix args
during the repeating sequence increases code complexity enormously.
For posterity I'll leave here the patch that completely supports it.
But this feature will be removed (unless someone will ask to leave it):
C-x } } C-1 C-2 } } C-3 C-4 } }
Only this will remain:
C-x } } C-1 C-2 C-x } } C-3 C-4 C-x } }
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: repeat-prefix-arg.patch --]
[-- Type: text/x-diff, Size: 7134 bytes --]
diff --git a/lisp/repeat.el b/lisp/repeat.el
index ee9e14b515..47b080bc6c 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -390,7 +390,10 @@ repeat-mode
See `describe-repeat-maps' for a list of all repeatable command."
:global t :group 'convenience
(if (not repeat-mode)
- (remove-hook 'post-command-hook 'repeat-post-hook)
+ (progn
+ (remove-hook 'pre-command-hook 'repeat-pre-hook)
+ (remove-hook 'post-command-hook 'repeat-post-hook))
+ (add-hook 'pre-command-hook 'repeat-pre-hook)
(add-hook 'post-command-hook 'repeat-post-hook)
(let* ((keymaps nil)
(commands (all-completions
@@ -402,51 +405,108 @@ repeat-mode
(length commands)
(length (delete-dups keymaps))))))
+(defun repeat-map ()
+ "Return a transient map for keys repeatable after the current command."
+ (let ((rep-map (or repeat-map
+ (and (symbolp real-this-command)
+ (get real-this-command 'repeat-map)))))
+ (when rep-map
+ (when (and (symbolp rep-map) (boundp rep-map))
+ (setq rep-map (symbol-value rep-map)))
+ rep-map)))
+
+(defvar repeat-last-prefix-command nil)
+
+(defun repeat-check-map (map)
+ "Decides whether MAP can be used for the next command.
+Can contain more conditions."
+ (and map
+ ;; Avoid using repeatable keys when a minibuffer prompt pops up.
+ ;; FIXME: Instead of disallowing repeatable keys in the minibuffer,
+ ;; it would be better to detect when `minibuffer-depth' changes
+ ;; during a repeatable sequence, but this is impossible to do
+ ;; when a repeatable command that activates own minibuffer
+ ;; was called from the minibuffer, e.g. `M-x repeatable-command RET'
+ ;; where in `exit-minibuffer' (bound to RET) minibuffer-depth is 1,
+ ;; and if repeatable-command uses the minibuffer, it's also 1.
+ (zerop (minibuffer-depth))
+ (or
+ ;; Allow prefix commands change `prefix-arg' for next repeatable
+ ;; commands, i.e. don't disable transient map on such sequence
+ ;; `C-x } C-1 C-2 }' that changes window enlargement step to 12.
+ (and repeat-keep-prefix
+ (or (memq this-command
+ '(universal-argument universal-argument-more
+ digit-argument negative-argument))
+ prefix-arg))
+ ;; Exit when the last char is not among repeatable keys,
+ ;; so e.g. `C-x u u' repeats undo, whereas `C-/ u' doesn't.
+ (lookup-key map (vector last-nonmenu-event)))))
+
+(defun repeat-pre-hook ()
+ "Function run before commands to handle repeatable keys."
+ ;; Reset prefix-arg before the next non-repeatable command,
+ ;; e.g. `C-- C-x } } C-n' or `C-x } C-- } C-n', so `C-n'
+ ;; should not use `prefix-arg' to go in opposite direction.
+ (when (and repeat-mode repeat-keep-prefix prefix-arg repeat-in-progress)
+ (if (not (memq this-command
+ '(universal-argument universal-argument-more
+ digit-argument negative-argument)))
+ (let ((map (repeat-map)))
+ (if (repeat-check-map map)
+ ;; Optimize to use less logic in the function `repeat-map'.
+ ;; When called again from `repeat-post-hook' it will use
+ ;; the variable `repeat-map'.
+ (setq repeat-map map)
+ ;; When `repeat-post-hook' will exit the repeatable sequence,
+ ;; this means the current command is not repeatable,
+ ;; so reset `prefix-arg' enabled for repeatable commands only.
+ (setq prefix-arg nil)))
+ (unless (memq repeat-last-prefix-command
+ '(universal-argument universal-argument-more
+ digit-argument negative-argument))
+ (setq prefix-arg nil)))
+ (setq repeat-last-prefix-command this-command)))
+
(defun repeat-post-hook ()
"Function run after commands to set transient keymap for repeatable keys."
(let ((was-in-progress repeat-in-progress))
(setq repeat-in-progress nil)
- (when repeat-mode
- (let ((rep-map (or repeat-map
- (and (symbolp real-this-command)
- (get real-this-command 'repeat-map)))))
- (when rep-map
- (when (boundp rep-map)
- (setq rep-map (symbol-value rep-map)))
- (let ((map (copy-keymap rep-map)))
- ;; 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
- (or (lookup-key map (this-command-keys-vector))
- prefix-arg))
+ (let ((map (when repeat-mode (repeat-map))))
+ (when (repeat-check-map map)
- ;; Messaging
- (unless prefix-arg
- (funcall repeat-echo-function map))
+ ;; Messaging
+ (unless prefix-arg ;; Don't overwrite prefix arg echo
+ (funcall repeat-echo-function map))
- ;; Adding an exit key
- (when repeat-exit-key
- (define-key map repeat-exit-key 'ignore))
+ ;; Adding an exit key
+ (when repeat-exit-key
+ (setq map (copy-keymap map))
+ (define-key map repeat-exit-key 'ignore))
- (when (and repeat-keep-prefix (not prefix-arg))
- (setq prefix-arg current-prefix-arg))
+ ;; When the current command is not one of commands
+ ;; that set `prefix-arg' then keep the current prefix arg
+ ;; for the next command via `prefix-arg'.
+ (when (and repeat-keep-prefix
+ (not prefix-arg))
+ (setq prefix-arg current-prefix-arg))
- (setq repeat-in-progress t)
- (let ((exitfun (set-transient-map map)))
+ (setq repeat-in-progress t)
+ (let ((exitfun (set-transient-map map)))
- (when repeat-exit-timer
- (cancel-timer repeat-exit-timer)
- (setq repeat-exit-timer nil))
+ (when repeat-exit-timer
+ (cancel-timer repeat-exit-timer)
+ (setq repeat-exit-timer nil))
- (when repeat-exit-timeout
- (setq repeat-exit-timer
- (run-with-idle-timer
- repeat-exit-timeout nil
- (lambda ()
- (setq repeat-in-progress nil)
- (funcall exitfun)
- (funcall repeat-echo-function nil)))))))))))
+ (when repeat-exit-timeout
+ (setq repeat-exit-timer
+ (run-with-idle-timer
+ repeat-exit-timeout nil
+ (lambda ()
+ (setq repeat-in-progress nil)
+ (funcall exitfun)
+ (funcall repeat-echo-function nil))))))))
(setq repeat-map nil)
(when (and was-in-progress (not repeat-in-progress))
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#51281: [External] : bug#51281: 28.0.60; repeat-mode issues
2021-10-20 17:30 ` Juri Linkov
@ 2021-10-20 18:59 ` Drew Adams
2021-10-21 16:51 ` Juri Linkov
2021-11-04 23:24 ` Lars Ingebrigtsen
1 sibling, 1 reply; 10+ messages in thread
From: Drew Adams @ 2021-10-20 18:59 UTC (permalink / raw)
To: Juri Linkov, 51281@debbugs.gnu.org
> > Currently there is a bug where the prefix arg changed for the
> > repeatable commands, is applied to the next non-repeatable
> > command. For example: C-- C-x o o C-n or C-x o C-- o C-n
>
> Actually, this too confusing feature
It's not confusing at all.
> that allows changing prefix args during the repeating
> sequence increases code complexity enormously.
Too bad. In that case, feel free to revert the changes
you've made that impose such complexity. The code just
worked - has worked for a long time.
> this feature will be removed (unless someone will ask
> to leave it)
"unless someone..."
Yes, I ask that this important feature be kept,
not lost. Users should continue to be able to
add/change a prefix arg (of any kind) at any
time during repetition.
Starting the command over (with a different
prefix arg) is not _at all_ the same thing as
continuing the command invocation.
(Think command state, and Emacs state in general,
in addition to user interaction.)
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#51281: 28.0.60; repeat-mode issues
2021-10-20 17:30 ` Juri Linkov
2021-10-20 18:59 ` bug#51281: [External] : " Drew Adams
@ 2021-11-04 23:24 ` Lars Ingebrigtsen
2021-12-01 17:58 ` Juri Linkov
1 sibling, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-04 23:24 UTC (permalink / raw)
To: Juri Linkov; +Cc: 51281
Juri Linkov <juri@linkov.net> writes:
>> Currently there is a bug where the prefix arg changed for the repeatable
>> commands, is applied to the next non-repeatable command. For example:
>>
>> C-- C-x o o C-n
>> or
>> C-x o C-- o C-n
>
> Actually, this too confusing feature that allows changing prefix args
> during the repeating sequence increases code complexity enormously.
> For posterity I'll leave here the patch that completely supports it.
> But this feature will be removed (unless someone will ask to leave it):
>
> C-x } } C-1 C-2 } } C-3 C-4 } }
>
> Only this will remain:
>
> C-x } } C-1 C-2 C-x } } C-3 C-4 C-x } }
Well, it may be slightly on the confusing side, but it does feel
natural, I think? So I think fixing it is better than removing it.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#51281: 28.0.60; repeat-mode issues
2021-11-04 23:24 ` Lars Ingebrigtsen
@ 2021-12-01 17:58 ` Juri Linkov
2021-12-01 18:15 ` Eli Zaretskii
2022-09-05 19:40 ` bug#55986: 28.1; (setq repeat-keep-prefix t) breaks repeat-mode Lars Ingebrigtsen
0 siblings, 2 replies; 10+ messages in thread
From: Juri Linkov @ 2021-12-01 17:58 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: 51281
[-- Attachment #1: Type: text/plain, Size: 763 bytes --]
>> Actually, this too confusing feature that allows changing prefix args
>> during the repeating sequence increases code complexity enormously.
>> For posterity I'll leave here the patch that completely supports it.
>> But this feature will be removed (unless someone will ask to leave it):
>>
>> C-x } } C-1 C-2 } } C-3 C-4 } }
>>
>> Only this will remain:
>>
>> C-x } } C-1 C-2 C-x } } C-3 C-4 C-x } }
>
> Well, it may be slightly on the confusing side, but it does feel
> natural, I think? So I think fixing it is better than removing it.
I hesitate to make such massive change in repeat.el on the release branch.
I have more patches for repeat.el that I'm unsure about to push.
One of them is a patch that allows kbd syntax for 'repeat-exit-key':
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: repeat-exit-key-kbd.patch --]
[-- Type: text/x-diff, Size: 1568 bytes --]
diff --git a/lisp/repeat.el b/lisp/repeat.el
index 7bbb398873..8199bd3287 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -338,6 +338,7 @@ repeat-exit-key
"Key that stops the modal repeating of keys in sequence.
For example, you can set it to <return> like `isearch-exit'."
:type '(choice (const :tag "No special key to exit repeating sequence" nil)
+ (string :tag "Kbd string that exits repeating sequence")
(key-sequence :tag "Key that exits repeating sequence"))
:group 'convenience
:version "28.1")
@@ -437,7 +455,10 @@ repeat-post-hook
;; Adding an exit key
(when repeat-exit-key
- (define-key map repeat-exit-key 'ignore))
+ (define-key map (if (stringp repeat-exit-key)
+ (kbd repeat-exit-key)
+ repeat-exit-key)
+ 'ignore))
(when (and repeat-keep-prefix (not prefix-arg))
(setq prefix-arg current-prefix-arg))
@@ -476,7 +497,9 @@ repeat-echo-message-string
keys ", ")
(if repeat-exit-key
(format ", or exit with %s"
- (key-description repeat-exit-key))
+ (if (stringp repeat-exit-key)
+ repeat-exit-key
+ (key-description repeat-exit-key)))
""))))
(defun repeat-echo-message (keymap)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#51281: 28.0.60; repeat-mode issues
2021-12-01 17:58 ` Juri Linkov
@ 2021-12-01 18:15 ` Eli Zaretskii
2022-09-05 19:40 ` bug#55986: 28.1; (setq repeat-keep-prefix t) breaks repeat-mode Lars Ingebrigtsen
1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2021-12-01 18:15 UTC (permalink / raw)
To: Juri Linkov; +Cc: larsi, 51281
> From: Juri Linkov <juri@linkov.net>
> Date: Wed, 01 Dec 2021 19:58:49 +0200
> Cc: 51281@debbugs.gnu.org
>
> I hesitate to make such massive change in repeat.el on the release branch.
Yes, please install on master. The problem sounds sufficiently
obscure so as not to be too urgent to fix.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#55986: 28.1; (setq repeat-keep-prefix t) breaks repeat-mode
2021-12-01 17:58 ` Juri Linkov
2021-12-01 18:15 ` Eli Zaretskii
@ 2022-09-05 19:40 ` Lars Ingebrigtsen
2022-10-03 19:58 ` Juri Linkov
1 sibling, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-05 19:40 UTC (permalink / raw)
To: Juri Linkov; +Cc: 51281, 55986
Juri Linkov <juri@linkov.net> writes:
> I hesitate to make such massive change in repeat.el on the release branch.
>
> I have more patches for repeat.el that I'm unsure about to push.
> One of them is a patch that allows kbd syntax for 'repeat-exit-key':
(I only skimmed these two bug reports lightly.) This was more than half
a year ago, but at least some of this hasn't been pushed yet? Are these
changes still on the planning change?
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#55986: 28.1; (setq repeat-keep-prefix t) breaks repeat-mode
2022-09-05 19:40 ` bug#55986: 28.1; (setq repeat-keep-prefix t) breaks repeat-mode Lars Ingebrigtsen
@ 2022-10-03 19:58 ` Juri Linkov
0 siblings, 0 replies; 10+ messages in thread
From: Juri Linkov @ 2022-10-03 19:58 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: 51281, 55986
>> I hesitate to make such massive change in repeat.el on the release branch.
>>
>> I have more patches for repeat.el that I'm unsure about to push.
>> One of them is a patch that allows kbd syntax for 'repeat-exit-key':
>
> (I only skimmed these two bug reports lightly.) This was more than half
> a year ago, but at least some of this hasn't been pushed yet? Are these
> changes still on the planning change?
One of the patches is pushed in b374952b51, but another change is more massive.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-10-03 19:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-19 7:12 bug#51281: 28.0.60; repeat-mode issues Juri Linkov
2021-10-20 17:30 ` Juri Linkov
2021-10-20 18:59 ` bug#51281: [External] : " Drew Adams
2021-10-21 16:51 ` Juri Linkov
2021-10-21 17:09 ` Drew Adams
2021-11-04 23:24 ` Lars Ingebrigtsen
2021-12-01 17:58 ` Juri Linkov
2021-12-01 18:15 ` Eli Zaretskii
2022-09-05 19:40 ` bug#55986: 28.1; (setq repeat-keep-prefix t) breaks repeat-mode Lars Ingebrigtsen
2022-10-03 19:58 ` Juri Linkov
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).