* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
@ 2022-01-08 13:24 Augusto Stoffel
2022-01-08 18:59 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-01-08 13:24 UTC (permalink / raw)
To: 53126
[-- Attachment #1: Type: text/plain, Size: 926 bytes --]
The anzu package was mentioned recently in the mailing list. Emacs now
includes the lazy-count feature, which is the main purpose of anzu, but
it's still missing one nice feature from that package, namely the
highlighting and count of matches as one types a regexp or string to
replace in `query-replace{-regexp}'. See the below patch for a
(probably not yet mature) stab at this.
I've also added lazy highlighting to the `isearch-edit-string' command.
I didn't introduce any new customization variables. Lazy highlighting
in both `query-replace{-regexp}' and `isearch-edit-string' is controlled
by `isearch-lazy-highlight' (therefore is on by default) and lazy count
next to the minibuffer prompt is controlled by `isearch-lazy-count'
(therefore off by default).
I believe the problems pointed out in a previous iteration of this in
https://lists.gnu.org/archive/html/emacs-devel/2021-05/msg01196.html are
now solved.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-reading-from-minibuffer-with-lazy-highlight-an.patch --]
[-- Type: text/x-patch, Size: 9534 bytes --]
From 11e52a1bf1adb2a15dda3097d6c6016034ae4487 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 8 Jan 2022 11:08:46 +0100
Subject: [PATCH 1/2] Allow reading from minibuffer with lazy highlight and
match count
* lisp/isearch.el (isearch-lazy-count-display-function): New variable
allowing to display the lazy count in special ways.
(isearch-edit-string): Add lazy highlight and count of matching text.
(isearch-lazy-highlight-new-loop,
isearch-lazy-highlight-buffer-update): Use
`isearch-lazy-count-display-function' instead of hardcoded call to
`isearch-message'.
(isearch-read-with-highlight--overlay,
isearch-read-with-highlight--after-change,
isearch-read-with-highlight--exit, isearch-read-with-highlight-setup,
isearch-read-with-highlight-count): Variable and functions
implementing the lazy highlight functionality while reading from
minibuffer.
* lisp/simple.el (minibuffer-history-isearch-setup): Set
`isearch-lazy-count-display-function' appropriately.
* lisp/comint.el (comint-history-isearch-setup,
comint-history-isearch-end): Set `isearch-lazy-count-display-function'
appropriately.
---
lisp/comint.el | 2 ++
lisp/isearch.el | 91 +++++++++++++++++++++++++++++++++++++++----------
lisp/simple.el | 1 +
3 files changed, 76 insertions(+), 18 deletions(-)
diff --git a/lisp/comint.el b/lisp/comint.el
index fdea3e33bb..c1695bbbdf 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1515,6 +1515,7 @@ comint-history-isearch-setup
#'comint-history-isearch-wrap)
(setq-local isearch-push-state-function
#'comint-history-isearch-push-state)
+ (setq-local isearch-lazy-count-display-function nil)
(add-hook 'isearch-mode-end-hook 'comint-history-isearch-end nil t)))
(defun comint-history-isearch-end ()
@@ -1526,6 +1527,7 @@ comint-history-isearch-end
(setq isearch-message-function nil)
(setq isearch-wrap-function nil)
(setq isearch-push-state-function nil)
+ (setq-local isearch-lazy-count-display-function #'isearch-message)
(remove-hook 'isearch-mode-end-hook 'comint-history-isearch-end t)
(unless isearch-suspended
(custom-reevaluate-setting 'comint-history-isearch)))
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 7593a0ec98..ccb06d534e 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -465,6 +465,9 @@ lazy-count-suffix-format
:group 'lazy-count
:version "27.1")
+(defvar isearch-lazy-count-display-function #'isearch-message
+ "Function called when new lazy count results are available.")
+
\f
;; Define isearch help map.
@@ -1808,19 +1811,21 @@ isearch-edit-string
;; for some incompatibility with gmhist.
(minibuffer-history-symbol)
;; Search string might have meta information on text properties.
- (minibuffer-allow-text-properties t))
+ (minibuffer-allow-text-properties t)
+ (isearch-lazy-count-display-function #'isearch-read-with-highlight-count))
(setq isearch-new-string
- (read-from-minibuffer
- (isearch-message-prefix nil isearch-nonincremental)
- (cons isearch-string (1+ (or (isearch-fail-pos)
- (length isearch-string))))
- minibuffer-local-isearch-map nil
- (if isearch-regexp
- (cons 'regexp-search-ring
- (1+ (or regexp-search-ring-yank-pointer -1)))
- (cons 'search-ring
- (1+ (or search-ring-yank-pointer -1))))
- nil t)
+ (minibuffer-with-setup-hook #'isearch-read-with-highlight-setup
+ (read-from-minibuffer
+ (isearch-message-prefix nil isearch-nonincremental)
+ (cons isearch-string (1+ (or (isearch-fail-pos)
+ (length isearch-string))))
+ minibuffer-local-isearch-map nil
+ (if isearch-regexp
+ (cons 'regexp-search-ring
+ (1+ (or regexp-search-ring-yank-pointer -1)))
+ (cons 'search-ring
+ (1+ (or search-ring-yank-pointer -1))))
+ nil t))
isearch-new-message
(mapconcat 'isearch-text-char-description
isearch-new-string "")))))
@@ -4023,7 +4028,7 @@ isearch-lazy-highlight-new-loop
isearch-lazy-highlight-window-end))))))
;; something important did indeed change
(lazy-highlight-cleanup t (not (equal isearch-string ""))) ;stop old timer
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when (and isearch-lazy-count isearch-lazy-count-display-function)
(when (or (equal isearch-string "")
;; Check if this place was reached by a condition above
;; other than changed window boundaries (that shouldn't
@@ -4042,7 +4047,8 @@ isearch-lazy-highlight-new-loop
(setq isearch-lazy-count-current nil
isearch-lazy-count-total nil)
;; Delay updating the message if possible, to avoid flicker
- (when (string-equal isearch-string "") (isearch-message))))
+ (when (string-equal isearch-string "")
+ (funcall isearch-lazy-count-display-function))))
(setq isearch-lazy-highlight-window-start-changed nil)
(setq isearch-lazy-highlight-window-end-changed nil)
(setq isearch-lazy-highlight-error isearch-error)
@@ -4095,13 +4101,13 @@ isearch-lazy-highlight-new-loop
'isearch-lazy-highlight-start))))
;; Update the current match number only in isearch-mode and
;; unless isearch-mode is used specially with isearch-message-function
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when (and isearch-lazy-count isearch-lazy-count-display-function)
;; Update isearch-lazy-count-current only when it was already set
;; at the end of isearch-lazy-highlight-buffer-update
(when isearch-lazy-count-current
(setq isearch-lazy-count-current
(gethash (point) isearch-lazy-count-hash 0))
- (isearch-message))))
+ (funcall isearch-lazy-count-display-function))))
(defun isearch-lazy-highlight-search (string bound)
"Search ahead for the next or previous match, for lazy highlighting.
@@ -4302,16 +4308,65 @@ isearch-lazy-highlight-buffer-update
(setq looping nil
nomore t))))
(if nomore
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when (and isearch-lazy-count isearch-lazy-count-display-function)
(unless isearch-lazy-count-total
(setq isearch-lazy-count-total 0))
(setq isearch-lazy-count-current
(gethash opoint isearch-lazy-count-hash 0))
- (isearch-message))
+ (funcall isearch-lazy-count-display-function))
(setq isearch-lazy-highlight-timer
(run-at-time lazy-highlight-interval nil
'isearch-lazy-highlight-buffer-update)))))))))
+;; Reading from minibuffer with lazy highlight and match count
+
+(defvar isearch-read-with-highlight--overlay nil
+ "Overlay for minibuffer prompt updates.")
+
+(defvar isearch-read-with-highlight-transform #'identity
+ "Function to transform minibuffer text into a `isearch-string' for highlighting.")
+
+(defun isearch-read-with-highlight--after-change (_beg _end _len)
+ "Update lazy highlight state in minibuffer selected window."
+ (when isearch-lazy-highlight
+ (let ((inhibit-redisplay t) ;; Avoid cursor flickering
+ (string (minibuffer-contents)))
+ (with-minibuffer-selected-window
+ (setq isearch-string (funcall isearch-read-with-highlight-transform string))
+ (isearch-lazy-highlight-new-loop)))))
+
+(defun isearch-read-with-highlight--exit ()
+ "Unwind changes from `isearch-read-with-highlight-setup'."
+ (remove-hook 'after-change-functions
+ #'isearch-read-with-highlight--after-change)
+ (remove-hook 'minibuffer-exit-hook #'isearch-read-with-highlight--exit)
+ (setq isearch-read-with-highlight--overlay nil)
+ (lazy-highlight-cleanup))
+
+(defun isearch-read-with-highlight-setup ()
+ "Set up minibuffer for lazy highlight of matches in the original window.
+
+This is intended to be called via `minibuffer-with-setup-hook'.
+Note that several other isearch variables influence the lazy
+highlighting, including `isearch-regexp' and
+`isearch-lazy-count-display-function'."
+ (add-hook 'after-change-functions
+ #'isearch-read-with-highlight--after-change)
+ (add-hook 'minibuffer-exit-hook #'isearch-read-with-highlight--exit)
+ (setq isearch-read-with-highlight--overlay
+ (make-overlay (point-min) (point-min) (current-buffer) t t))
+ (isearch-read-with-highlight--after-change nil nil nil))
+
+(defun isearch-read-with-highlight-count ()
+ "Display total match count in the minibuffer prompt."
+ (when isearch-read-with-highlight--overlay
+ (overlay-put isearch-read-with-highlight--overlay
+ 'before-string
+ (and isearch-lazy-count-total
+ (not isearch-error)
+ (format "%s " isearch-lazy-count-total)))))
+
+\f
(defun isearch-resume (string regexp word forward message case-fold)
"Resume an incremental search.
STRING is the string or regexp searched for.
diff --git a/lisp/simple.el b/lisp/simple.el
index 070d2764fe..ecf07b912d 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2787,6 +2787,7 @@ minibuffer-history-isearch-setup
#'minibuffer-history-isearch-wrap)
(setq-local isearch-push-state-function
#'minibuffer-history-isearch-push-state)
+ (setq-local isearch-lazy-count-display-function nil)
(add-hook 'isearch-mode-end-hook 'minibuffer-history-isearch-end nil t))
(defun minibuffer-history-isearch-end ()
--
2.33.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Lazy-highlight-when-reading-query-replace-text.patch --]
[-- Type: text/x-patch, Size: 1896 bytes --]
From 6d9ec412addfc450f63487de0d90a1076cab47a0 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 8 Jan 2022 13:15:40 +0100
Subject: [PATCH 2/2] Lazy highlight when reading query-replace text
* lisp/replace.el (query-replace-read-from): Add lazy highlighting.
---
lisp/replace.el | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/lisp/replace.el b/lisp/replace.el
index 60e507c642..241a97a3a2 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -244,6 +244,17 @@ query-replace-read-from
(query-replace-descr
(cdar query-replace-defaults)))))
(t (format-prompt prompt nil))))
+ ;; Set up lazy highlighting while reading FROM regexp
+ (isearch-lazy-highlight query-replace-highlight)
+ (isearch-regexp regexp-flag)
+ (isearch-regexp-function nil)
+ (isearch-case-fold-search case-fold-search) ;; TODO: the case-folding rule here is complicated...
+ (isearch-read-with-highlight-transform
+ (lambda (string)
+ (let ((from (query-replace--split-string string)))
+ (if (consp from) (car from) from))))
+ (isearch-lazy-count-display-function
+ #'isearch-read-with-highlight-count)
(from
;; The save-excursion here is in case the user marks and copies
;; a region in order to specify the minibuffer input.
@@ -251,6 +262,7 @@ query-replace-read-from
(save-excursion
(minibuffer-with-setup-hook
(lambda ()
+ (isearch-read-with-highlight-setup)
(setq-local text-property-default-nonsticky
(append '((separator . t) (face . t))
text-property-default-nonsticky)))
--
2.33.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-01-08 13:24 bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc Augusto Stoffel
@ 2022-01-08 18:59 ` Juri Linkov
2022-01-08 19:35 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-01-08 18:59 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> The anzu package was mentioned recently in the mailing list. Emacs now
> includes the lazy-count feature, which is the main purpose of anzu, but
> it's still missing one nice feature from that package, namely the
> highlighting and count of matches as one types a regexp or string to
> replace in `query-replace{-regexp}'. See the below patch for a
> (probably not yet mature) stab at this.
Strange, I expected that the lazy-count feature applied to query-replace
should do a different thing - when querying about replacing the current match,
it should show the number of current match and the number of total matches.
The latter number should be updated after every replacement.
But your patches are intended for a different feature - highlighting
of matches in the buffer while entering an input string in the minibuffer.
I wonder how many users need this feature, when it's easy to construct
a query-replace string using highlighting/counting in isearch-mode, then type M-%
(isearch-query-replace) that invokes query-replace with the query-replace string.
Perhaps better to ask on emacs-devel if anyone needs this feature.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-01-08 18:59 ` Juri Linkov
@ 2022-01-08 19:35 ` Augusto Stoffel
2022-01-09 9:10 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-01-08 19:35 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Sat, 8 Jan 2022 at 20:59, Juri Linkov <juri@linkov.net> wrote:
> Strange, I expected that the lazy-count feature applied to query-replace
> should do a different thing - when querying about replacing the current match,
> it should show the number of current match and the number of total matches.
> The latter number should be updated after every replacement.
>
That would be handy too. I haven't checked how the `perform-replace'
loop works, but wouldn't this require a hook into the lazy count
procedure --- precisely what my patch introduces through the new
variable `isearch-lazy-count-display-function'?
> But your patches are intended for a different feature - highlighting
> of matches in the buffer while entering an input string in the minibuffer.
>
> I wonder how many users need this feature, when it's easy to construct
> a query-replace string using highlighting/counting in isearch-mode,
> then type M-%
> (isearch-query-replace) that invokes query-replace with the
> query-replace string.
>
Sure, this alternative method works. But somehow it's not the way I
usually start a replace, and I think there's nothing wrong with that
preference :-)
> Perhaps better to ask on emacs-devel if anyone needs this feature.
We can ask. But note that the bulk of the patch has a fairly general
purpose, namely to read a regexp with live preview of the matches.
There's probably a couple extra uses for this.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-01-08 19:35 ` Augusto Stoffel
@ 2022-01-09 9:10 ` Juri Linkov
2022-01-09 10:02 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-01-09 9:10 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> Strange, I expected that the lazy-count feature applied to query-replace
>> should do a different thing - when querying about replacing the current match,
>> it should show the number of current match and the number of total matches.
>> The latter number should be updated after every replacement.
>
> That would be handy too. I haven't checked how the `perform-replace'
> loop works, but wouldn't this require a hook into the lazy count
> procedure --- precisely what my patch introduces through the new
> variable `isearch-lazy-count-display-function'?
It seems the "Query replacing" prompt in perform-replace could just
display the values of isearch-lazy-count-current and
isearch-lazy-count-total. Or for asynchronous lazy-count a new hook
needs to be run at the end of isearch-lazy-highlight-buffer-update
in addition to the call of isearch-message.
>> But your patches are intended for a different feature - highlighting
>> of matches in the buffer while entering an input string in the minibuffer.
>>
>> I wonder how many users need this feature, when it's easy to construct
>> a query-replace string using highlighting/counting in isearch-mode,
>> then type M-%
>> (isearch-query-replace) that invokes query-replace with the
>> query-replace string.
>
> Sure, this alternative method works. But somehow it's not the way I
> usually start a replace, and I think there's nothing wrong with that
> preference :-)
But this method is more limiting - no keys to pull text from the buffer
like 'C-w' (isearch-yank-word-or-char) does in isearch-mode, no way
to navigate to the first match like it's possible in isearch-mode, etc.
>> Perhaps better to ask on emacs-devel if anyone needs this feature.
>
> We can ask. But note that the bulk of the patch has a fairly general
> purpose, namely to read a regexp with live preview of the matches.
> There's probably a couple extra uses for this.
Maybe then this feature could be added to read-regexp or even
to read-from-minibuffer? And activated by adding a setup function
to minibuffer-setup-hook like other minibuffer's features do, such as
icomplete-minibuffer-setup, minibuffer-depth-setup, rfn-eshadow-setup-minibuffer.
Then maybe a new feature could be named e.g. "lazy-minibuffer"?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-01-09 9:10 ` Juri Linkov
@ 2022-01-09 10:02 ` Augusto Stoffel
2022-01-09 10:30 ` Augusto Stoffel
2022-01-09 18:58 ` Juri Linkov
0 siblings, 2 replies; 57+ messages in thread
From: Augusto Stoffel @ 2022-01-09 10:02 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Sun, 9 Jan 2022 at 11:10, Juri Linkov <juri@linkov.net> wrote:
> It seems the "Query replacing" prompt in perform-replace could just
> display the values of isearch-lazy-count-current and
> isearch-lazy-count-total.
Yes, I guess you are right.
> Or for asynchronous lazy-count a new hook needs to be run at the end
> of isearch-lazy-highlight-buffer-update in addition to the call of
> isearch-message.
>
That's what the patch does (but with a variable pointing to a function
instead of a hook, although this can be changed). But note that it's
not enough to do something “in addition to the call of isearch-message”;
it's necessary to _suppress_ the call to isearch-message.
>>> But your patches are intended for a different feature - highlighting
>>> of matches in the buffer while entering an input string in the minibuffer.
>>>
>>> I wonder how many users need this feature, when it's easy to construct
>>> a query-replace string using highlighting/counting in isearch-mode,
>>> then type M-%
>>> (isearch-query-replace) that invokes query-replace with the
>>> query-replace string.
>>
>> Sure, this alternative method works. But somehow it's not the way I
>> usually start a replace, and I think there's nothing wrong with that
>> preference :-)
>
> But this method is more limiting - no keys to pull text from the buffer
> like 'C-w' (isearch-yank-word-or-char) does in isearch-mode, no way
> to navigate to the first match like it's possible in isearch-mode, etc.
>
OTOH, you can't do a replace-in-region that way.
> Maybe then this feature could be added to read-regexp or even
> to read-from-minibuffer? And activated by adding a setup function
> to minibuffer-setup-hook like other minibuffer's features do, such as
> icomplete-minibuffer-setup, minibuffer-depth-setup, rfn-eshadow-setup-minibuffer.
>
> Then maybe a new feature could be named e.g. "lazy-minibuffer"?
This is what the patch does, with code of this kind:
```
(let ((isearch-regexp t)
;; Whatever else isearch / lazy-highlight settings might be needed
(isearch-lazy-count-display-function #'isearch-read-with-highlight-count))
(minibuffer-with-setup-hook #'isearch-read-with-highlight-setup
(read-from-minibuffer "Type a regexp with preview: ")))
```
There could be a convenience wrapper for this code, but I'm not sure
that makes much sense, because isearch and lazy-highlight have too many
parameters one might need to set so it's better to be explicit/flexible.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-01-09 10:02 ` Augusto Stoffel
@ 2022-01-09 10:30 ` Augusto Stoffel
2022-01-09 18:58 ` Juri Linkov
1 sibling, 0 replies; 57+ messages in thread
From: Augusto Stoffel @ 2022-01-09 10:30 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Sun, 9 Jan 2022 at 11:02, Augusto Stoffel <arstoffel@gmail.com> wrote:
> OTOH, you can't do a replace-in-region that way.
Or rather -- you can, but it's tricky to keep the desired region
unchanged.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-01-09 10:02 ` Augusto Stoffel
2022-01-09 10:30 ` Augusto Stoffel
@ 2022-01-09 18:58 ` Juri Linkov
2022-01-10 17:34 ` Augusto Stoffel
1 sibling, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-01-09 18:58 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> Or for asynchronous lazy-count a new hook needs to be run at the end
>> of isearch-lazy-highlight-buffer-update in addition to the call of
>> isearch-message.
>
> That's what the patch does (but with a variable pointing to a function
> instead of a hook, although this can be changed). But note that it's
> not enough to do something “in addition to the call of isearch-message”;
> it's necessary to _suppress_ the call to isearch-message.
It would be great to use your new variable with a function
to show replacement counts in perform-replace. IIUC,
let-binding isearch-lazy-count-display-function to
isearch-read-with-highlight-count will suppress isearch-message?
>> Maybe then this feature could be added to read-regexp or even
>> to read-from-minibuffer? And activated by adding a setup function
>> to minibuffer-setup-hook like other minibuffer's features do, such as
>> icomplete-minibuffer-setup, minibuffer-depth-setup, rfn-eshadow-setup-minibuffer.
>
> This is what the patch does, with code of this kind:
>
> ```
> (let ((isearch-regexp t)
> ;; Whatever else isearch / lazy-highlight settings might be needed
> (isearch-lazy-count-display-function #'isearch-read-with-highlight-count))
> (minibuffer-with-setup-hook #'isearch-read-with-highlight-setup
> (read-from-minibuffer "Type a regexp with preview: ")))
> ```
>
> There could be a convenience wrapper for this code, but I'm not sure
> that makes much sense, because isearch and lazy-highlight have too many
> parameters one might need to set so it's better to be explicit/flexible.
I meant using simply
(add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
But it seems isearch-read-with-highlight-setup doesn't set
isearch-lazy-count-display-function.
>> Then maybe a new feature could be named e.g. "lazy-minibuffer"?
This feature has little to do with isearch. This is why there are
efforts to move away from the prefix isearch- for lazy-related
functions and variables, so we have now:
lazy-count-prefix-format
lazy-count-suffix-format
lazy-highlight-buffer
lazy-highlight-buffer-max-at-a-time
lazy-highlight-cleanup
lazy-highlight-initial-delay
lazy-highlight-interval
lazy-highlight-max-at-a-time
...
There are still isearch-specific names like isearch-lazy-count
that enables lazy-count in isearch-mode. What would be a similar
name for the minibuffer? Maybe minibuffer-lazy-count?
Then the prefix isearch- needs to be removed from other names too,
e.g. isearch-lazy-count-display-function -> lazy-count-display-function
isearch-read-with-highlight-setup maybe to minibuffer-lazy-highlight-setup,
etc.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-01-09 18:58 ` Juri Linkov
@ 2022-01-10 17:34 ` Augusto Stoffel
2022-01-10 19:09 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-01-10 17:34 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 2799 bytes --]
Hi Juri,
I attached a new patch (still a sketch) that requires no changes in
comint.el and simple.el. Perhaps you will find this approach more
acceptable.
More comments below.
On Sun, 9 Jan 2022 at 20:58, Juri Linkov <juri@linkov.net> wrote:
> It would be great to use your new variable with a function
> to show replacement counts in perform-replace. IIUC,
> let-binding isearch-lazy-count-display-function to
> isearch-read-with-highlight-count will suppress isearch-message?
>
I tried this and it's relatively simple to do, but there is a problem.
Suppose you want to replace all "a" with "z", and your buffer has 20
"a"s initially. Then, as you keep hitting "y" to confirm a replacement
the count will be
1/20, 1/19, ..., 1/1
since the number of "a"s decrease, and the point is always at the first
of the still-existing ones. But probably one should count the number of
prompts, so
1/20, 2/20, ..., 20/20
I think this means `perform-replace' has to implement its own way to
display a count.
> I meant using simply
>
> (add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
>
> But it seems isearch-read-with-highlight-setup doesn't set
> isearch-lazy-count-display-function.
>
I guess this could be done. But note that there are two possible types
of counts: a "current/total" counter or just a "total" counter. Each
use case calls for a different count style.
>>> Then maybe a new feature could be named e.g. "lazy-minibuffer"?
>
> This feature has little to do with isearch. This is why there are
> efforts to move away from the prefix isearch- for lazy-related
> functions and variables, so we have now:
>
> lazy-count-prefix-format
> lazy-count-suffix-format
> lazy-highlight-buffer
> lazy-highlight-buffer-max-at-a-time
> lazy-highlight-cleanup
> lazy-highlight-initial-delay
> lazy-highlight-interval
> lazy-highlight-max-at-a-time
> ...
>
> There are still isearch-specific names like isearch-lazy-count
> that enables lazy-count in isearch-mode. What would be a similar
> name for the minibuffer? Maybe minibuffer-lazy-count?
I see, the lazy-* names are the new ones!
>
> Then the prefix isearch- needs to be removed from other names too,
> e.g. isearch-lazy-count-display-function -> lazy-count-display-function
> isearch-read-with-highlight-setup maybe to minibuffer-lazy-highlight-setup,
> etc.
Yes, I agree. The names I came up with are horrible and
`lazy-minibuffer' is weird, but your current suggestion is nice.
By the way, I'm debating a bit whether
`isearch-lazy-count-display-function' should be:
1. Either nil or function, as it is right now,
2. #'ignore by default, so similar to 1) but a bit easier to use with
`add-function'
3. a hook, the main inconvenience being that it can't be easily let-bound.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-reading-from-minibuffer-with-lazy-highlight-an.patch --]
[-- Type: text/x-patch, Size: 8434 bytes --]
From 471c37844f0baba925bd948e13005d2227d43f65 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 8 Jan 2022 11:08:46 +0100
Subject: [PATCH 1/2] Allow reading from minibuffer with lazy highlight and
match count
* lisp/isearch.el (isearch-lazy-count-display-function): New variable
allowing to display the lazy count in special ways.
(isearch-edit-string): Add lazy highlight and count of matching text.
(isearch-lazy-highlight-new-loop,
isearch-lazy-highlight-buffer-update): Use
`isearch-lazy-count-display-function' instead of hardcoded call to
`isearch-message'.
(isearch-read-with-highlight--overlay,
isearch-read-with-highlight--after-change,
isearch-read-with-highlight--exit, isearch-read-with-highlight-setup,
isearch-read-with-highlight-count): Variable and functions
implementing the lazy highlight functionality while reading from
minibuffer.
* lisp/simple.el (minibuffer-history-isearch-setup): Set
`isearch-lazy-count-display-function' appropriately.
* lisp/comint.el (comint-history-isearch-setup,
comint-history-isearch-end): Set `isearch-lazy-count-display-function'
appropriately.
---
lisp/isearch.el | 100 +++++++++++++++++++++++++++++++++++++++---------
1 file changed, 82 insertions(+), 18 deletions(-)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 7593a0ec98..c10f365ee5 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -465,6 +465,9 @@ lazy-count-suffix-format
:group 'lazy-count
:version "27.1")
+(defvar isearch-lazy-count-display-function nil
+ "Function called when new lazy count results are available.")
+
\f
;; Define isearch help map.
@@ -1808,19 +1811,21 @@ isearch-edit-string
;; for some incompatibility with gmhist.
(minibuffer-history-symbol)
;; Search string might have meta information on text properties.
- (minibuffer-allow-text-properties t))
+ (minibuffer-allow-text-properties t)
+ (isearch-lazy-count-display-function #'isearch-read-with-highlight-count))
(setq isearch-new-string
- (read-from-minibuffer
- (isearch-message-prefix nil isearch-nonincremental)
- (cons isearch-string (1+ (or (isearch-fail-pos)
- (length isearch-string))))
- minibuffer-local-isearch-map nil
- (if isearch-regexp
- (cons 'regexp-search-ring
- (1+ (or regexp-search-ring-yank-pointer -1)))
- (cons 'search-ring
- (1+ (or search-ring-yank-pointer -1))))
- nil t)
+ (minibuffer-with-setup-hook #'isearch-read-with-highlight-setup
+ (read-from-minibuffer
+ (isearch-message-prefix nil isearch-nonincremental)
+ (cons isearch-string (1+ (or (isearch-fail-pos)
+ (length isearch-string))))
+ minibuffer-local-isearch-map nil
+ (if isearch-regexp
+ (cons 'regexp-search-ring
+ (1+ (or regexp-search-ring-yank-pointer -1)))
+ (cons 'search-ring
+ (1+ (or search-ring-yank-pointer -1))))
+ nil t))
isearch-new-message
(mapconcat 'isearch-text-char-description
isearch-new-string "")))))
@@ -4023,7 +4028,7 @@ isearch-lazy-highlight-new-loop
isearch-lazy-highlight-window-end))))))
;; something important did indeed change
(lazy-highlight-cleanup t (not (equal isearch-string ""))) ;stop old timer
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(when (or (equal isearch-string "")
;; Check if this place was reached by a condition above
;; other than changed window boundaries (that shouldn't
@@ -4042,7 +4047,11 @@ isearch-lazy-highlight-new-loop
(setq isearch-lazy-count-current nil
isearch-lazy-count-total nil)
;; Delay updating the message if possible, to avoid flicker
- (when (string-equal isearch-string "") (isearch-message))))
+ (when (string-equal isearch-string "")
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (when isearch-lazy-count-display-function
+ (funcall isearch-lazy-count-display-function)))))
(setq isearch-lazy-highlight-window-start-changed nil)
(setq isearch-lazy-highlight-window-end-changed nil)
(setq isearch-lazy-highlight-error isearch-error)
@@ -4095,13 +4104,16 @@ isearch-lazy-highlight-new-loop
'isearch-lazy-highlight-start))))
;; Update the current match number only in isearch-mode and
;; unless isearch-mode is used specially with isearch-message-function
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
;; Update isearch-lazy-count-current only when it was already set
;; at the end of isearch-lazy-highlight-buffer-update
(when isearch-lazy-count-current
(setq isearch-lazy-count-current
(gethash (point) isearch-lazy-count-hash 0))
- (isearch-message))))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (when isearch-lazy-count-display-function
+ (funcall isearch-lazy-count-display-function)))))
(defun isearch-lazy-highlight-search (string bound)
"Search ahead for the next or previous match, for lazy highlighting.
@@ -4302,16 +4314,68 @@ isearch-lazy-highlight-buffer-update
(setq looping nil
nomore t))))
(if nomore
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(unless isearch-lazy-count-total
(setq isearch-lazy-count-total 0))
(setq isearch-lazy-count-current
(gethash opoint isearch-lazy-count-hash 0))
- (isearch-message))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (when isearch-lazy-count-display-function
+ (funcall isearch-lazy-count-display-function)))
(setq isearch-lazy-highlight-timer
(run-at-time lazy-highlight-interval nil
'isearch-lazy-highlight-buffer-update)))))))))
+;; Reading from minibuffer with lazy highlight and match count
+
+(defvar isearch-read-with-highlight--overlay nil
+ "Overlay for minibuffer prompt updates.")
+
+(defvar isearch-read-with-highlight-transform #'identity
+ "Function to transform minibuffer text into a `isearch-string' for highlighting.")
+
+(defun isearch-read-with-highlight--after-change (_beg _end _len)
+ "Update lazy highlight state in minibuffer selected window."
+ (when isearch-lazy-highlight
+ (let ((inhibit-redisplay t) ;; Avoid cursor flickering
+ (string (minibuffer-contents)))
+ (with-minibuffer-selected-window
+ (setq isearch-string (funcall isearch-read-with-highlight-transform string))
+ (isearch-lazy-highlight-new-loop)))))
+
+(defun isearch-read-with-highlight--exit ()
+ "Unwind changes from `isearch-read-with-highlight-setup'."
+ (remove-hook 'after-change-functions
+ #'isearch-read-with-highlight--after-change)
+ (remove-hook 'minibuffer-exit-hook #'isearch-read-with-highlight--exit)
+ (setq isearch-read-with-highlight--overlay nil)
+ (lazy-highlight-cleanup))
+
+(defun isearch-read-with-highlight-setup ()
+ "Set up minibuffer for lazy highlight of matches in the original window.
+
+This is intended to be called via `minibuffer-with-setup-hook'.
+Note that several other isearch variables influence the lazy
+highlighting, including `isearch-regexp' and
+`isearch-lazy-count-display-function'."
+ (add-hook 'after-change-functions
+ #'isearch-read-with-highlight--after-change)
+ (add-hook 'minibuffer-exit-hook #'isearch-read-with-highlight--exit)
+ (setq isearch-read-with-highlight--overlay
+ (make-overlay (point-min) (point-min) (current-buffer) t t))
+ (isearch-read-with-highlight--after-change nil nil nil))
+
+(defun isearch-read-with-highlight-count ()
+ "Display total match count in the minibuffer prompt."
+ (when isearch-read-with-highlight--overlay
+ (overlay-put isearch-read-with-highlight--overlay
+ 'before-string
+ (and isearch-lazy-count-total
+ (not isearch-error)
+ (format "%s " isearch-lazy-count-total)))))
+
+\f
(defun isearch-resume (string regexp word forward message case-fold)
"Resume an incremental search.
STRING is the string or regexp searched for.
--
2.34.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Lazy-highlight-when-reading-query-replace-text.patch --]
[-- Type: text/x-patch, Size: 2014 bytes --]
From 8a24c1cc976e003494c3526304a02de8f37b5896 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 8 Jan 2022 13:15:40 +0100
Subject: [PATCH 2/2] Lazy highlight when reading query-replace text
* lisp/replace.el (query-replace-read-from): Add lazy highlighting.
---
lisp/replace.el | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/lisp/replace.el b/lisp/replace.el
index 60e507c642..31478ee4a9 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -244,6 +244,19 @@ query-replace-read-from
(query-replace-descr
(cdar query-replace-defaults)))))
(t (format-prompt prompt nil))))
+ ;; Set up lazy highlighting while reading FROM regexp.
+ ;; TODO: Treat the `region-noncontiguous-p' case by setting
+ ;; `isearch-filter-predicate'.
+ (isearch-lazy-highlight query-replace-highlight)
+ (isearch-regexp regexp-flag)
+ (isearch-regexp-function nil)
+ (isearch-case-fold-search case-fold-search) ;; TODO: the case-folding rule here is complicated...
+ (isearch-read-with-highlight-transform
+ (lambda (string)
+ (let ((from (query-replace--split-string string)))
+ (if (consp from) (car from) from))))
+ (isearch-lazy-count-display-function
+ #'isearch-read-with-highlight-count)
(from
;; The save-excursion here is in case the user marks and copies
;; a region in order to specify the minibuffer input.
@@ -251,6 +264,7 @@ query-replace-read-from
(save-excursion
(minibuffer-with-setup-hook
(lambda ()
+ (isearch-read-with-highlight-setup)
(setq-local text-property-default-nonsticky
(append '((separator . t) (face . t))
text-property-default-nonsticky)))
--
2.34.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-01-10 17:34 ` Augusto Stoffel
@ 2022-01-10 19:09 ` Juri Linkov
2022-02-26 16:13 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-01-10 19:09 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> I attached a new patch (still a sketch) that requires no changes in
> comint.el and simple.el. Perhaps you will find this approach more
> acceptable.
Thanks, no changes in other files is certainly a big plus.
>> It would be great to use your new variable with a function
>> to show replacement counts in perform-replace. IIUC,
>> let-binding isearch-lazy-count-display-function to
>> isearch-read-with-highlight-count will suppress isearch-message?
>
> I tried this and it's relatively simple to do, but there is a problem.
> Suppose you want to replace all "a" with "z", and your buffer has 20
> "a"s initially. Then, as you keep hitting "y" to confirm a replacement
> the count will be
>
> 1/20, 1/19, ..., 1/1
This is an interesting question. I tried in other editors,
and e.g. in the editor xed that is installed by default,
this is exactly what is displayed: 1/20, 1/19, ..., 1/1.
> since the number of "a"s decrease, and the point is always at the first
> of the still-existing ones. But probably one should count the number of
> prompts, so
>
> 1/20, 2/20, ..., 20/20
>
> I think this means `perform-replace' has to implement its own way to
> display a count.
Maybe this makes more sense, when the users will learn
what do these numbers mean.
>> I meant using simply
>>
>> (add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
>>
>> But it seems isearch-read-with-highlight-setup doesn't set
>> isearch-lazy-count-display-function.
>
> I guess this could be done.
Maybe two separate hooks could be defined? One highlights like
lazy-highlight, and another counts like lazy-count does:
(add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
(add-hook 'minibuffer-setup-hook 'isearch-read-with-count-setup)
> But note that there are two possible types
> of counts: a "current/total" counter or just a "total" counter. Each
> use case calls for a different count style.
The format of the total counter needs to be defined in a separate variable.
> By the way, I'm debating a bit whether
> `isearch-lazy-count-display-function' should be:
>
> 1. Either nil or function, as it is right now,
> 2. #'ignore by default, so similar to 1) but a bit easier to use with
> `add-function'
> 3. a hook, the main inconvenience being that it can't be easily let-bound.
This can be answered only by testing with all possible cases ;-)
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-01-10 19:09 ` Juri Linkov
@ 2022-02-26 16:13 ` Augusto Stoffel
2022-03-15 17:09 ` Juri Linkov
2022-03-15 17:24 ` Juri Linkov
0 siblings, 2 replies; 57+ messages in thread
From: Augusto Stoffel @ 2022-02-26 16:13 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 228 bytes --]
Hi Juri,
Sorry for getting back to this after such a long time. I've attached a
new patch that hopefully is good to merge, except for adding some NEWS
entry. Let me know what you think.
Inlined below some further comments.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-lazy-highlight-and-match-count-in-when-readi.patch --]
[-- Type: text/x-patch, Size: 12205 bytes --]
From 87ad2a45293e2fa1796a39f368d9ec16c0c4c070 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 8 Jan 2022 11:08:46 +0100
Subject: [PATCH] Display lazy highlight and match count in when reading
regexps
* lisp/isearch.el (lazy-count-update-hook): New hook allowing to
display the lazy count in special ways.
(isearch-edit-string): Add lazy highlight and count of matching text.
(isearch-lazy-highlight-new-loop,
isearch-lazy-highlight-buffer-update): Use
`isearch-lazy-count-display-function' instead of hardcoded call to
`isearch-message'.
(minibuffer-lazy-count-format, minibuffer-lazy-highlight-transform,
minibuffer-lazy-highlight--overlay, minibuffer-lazy-highlight--count,
minibuffer-lazy-highlight--after-change,
minibuffer-lazy-highlight--exit, minibuffer-lazy-highlight-setup):
Variables and functions implementing the lazy highlight functionality
while reading from minibuffer.
* lisp/replace.el (query-replace-read-from): Add lazy highlighting.
(replace--region-filter): New function, extracted from
'perform-replace'.
---
lisp/isearch.el | 84 +++++++++++++++++++++++++++++++++++++++++++++----
lisp/replace.el | 58 ++++++++++++++++++++++++----------
2 files changed, 120 insertions(+), 22 deletions(-)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 8970216398..6076cc4b5e 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1812,6 +1812,8 @@ isearch-edit-string
(minibuffer-history-symbol)
;; Search string might have meta information on text properties.
(minibuffer-allow-text-properties t))
+ (when isearch-lazy-highlight
+ (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup))
(setq isearch-new-string
(read-from-minibuffer
(isearch-message-prefix nil isearch-nonincremental)
@@ -3990,6 +3992,8 @@ isearch-lazy-highlight-error
(defvar isearch-lazy-count-current nil)
(defvar isearch-lazy-count-total nil)
(defvar isearch-lazy-count-hash (make-hash-table))
+(defvar lazy-count-update-hook nil
+ "Hook run after new lazy count results are computed.")
(defun lazy-highlight-cleanup (&optional force procrastinate)
"Stop lazy highlighting and remove extra highlighting from current buffer.
@@ -4048,7 +4052,7 @@ isearch-lazy-highlight-new-loop
isearch-lazy-highlight-window-end))))))
;; something important did indeed change
(lazy-highlight-cleanup t (not (equal isearch-string ""))) ;stop old timer
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(when (or (equal isearch-string "")
;; Check if this place was reached by a condition above
;; other than changed window boundaries (that shouldn't
@@ -4067,7 +4071,10 @@ isearch-lazy-highlight-new-loop
(setq isearch-lazy-count-current nil
isearch-lazy-count-total nil)
;; Delay updating the message if possible, to avoid flicker
- (when (string-equal isearch-string "") (isearch-message))))
+ (when (string-equal isearch-string "")
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))))
(setq isearch-lazy-highlight-window-start-changed nil)
(setq isearch-lazy-highlight-window-end-changed nil)
(setq isearch-lazy-highlight-error isearch-error)
@@ -4120,13 +4127,15 @@ isearch-lazy-highlight-new-loop
'isearch-lazy-highlight-start))))
;; Update the current match number only in isearch-mode and
;; unless isearch-mode is used specially with isearch-message-function
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
;; Update isearch-lazy-count-current only when it was already set
;; at the end of isearch-lazy-highlight-buffer-update
(when isearch-lazy-count-current
(setq isearch-lazy-count-current
(gethash (point) isearch-lazy-count-hash 0))
- (isearch-message))))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))))
(defun isearch-lazy-highlight-search (string bound)
"Search ahead for the next or previous match, for lazy highlighting.
@@ -4327,16 +4336,79 @@ isearch-lazy-highlight-buffer-update
(setq looping nil
nomore t))))
(if nomore
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(unless isearch-lazy-count-total
(setq isearch-lazy-count-total 0))
(setq isearch-lazy-count-current
(gethash opoint isearch-lazy-count-hash 0))
- (isearch-message))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))
(setq isearch-lazy-highlight-timer
(run-at-time lazy-highlight-interval nil
'isearch-lazy-highlight-buffer-update)))))))))
+;; Reading from minibuffer with lazy highlight and match count
+
+(defcustom minibuffer-lazy-count-format "%s "
+ "Format of the total number of matches for the prompt prefix."
+ :type '(choice (const :tag "Don't display a count" nil)
+ (string :tag "Display match count" "%s "))
+ :group 'lazy-count
+ :version "29.1")
+
+(defvar minibuffer-lazy-highlight-transform #'identity
+ "Function to transform minibuffer text into a `isearch-string' for highlighting.")
+
+(defvar minibuffer-lazy-highlight--overlay nil
+ "Overlay for minibuffer prompt updates.")
+
+(defun minibuffer-lazy-highlight--count ()
+ "Display total match count in the minibuffer prompt."
+ (when minibuffer-lazy-highlight--overlay
+ (overlay-put minibuffer-lazy-highlight--overlay
+ 'after-string
+ (and isearch-lazy-count-total
+ (not isearch-error)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)))))
+
+(defun minibuffer-lazy-highlight--after-change (_beg _end _len)
+ "Update lazy highlight state in minibuffer selected window."
+ (when isearch-lazy-highlight
+ (let ((inhibit-redisplay t) ;; Avoid cursor flickering
+ (string (minibuffer-contents)))
+ (with-minibuffer-selected-window
+ (setq isearch-string (funcall minibuffer-lazy-highlight-transform string))
+ (isearch-lazy-highlight-new-loop)))))
+
+(defun minibuffer-lazy-highlight--exit ()
+ "Unwind changes from `minibuffer-lazy-highlight-setup'."
+ (remove-hook 'after-change-functions
+ #'minibuffer-lazy-highlight--after-change)
+ (remove-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
+ (remove-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
+ (setq minibuffer-lazy-highlight--overlay nil)
+ (lazy-highlight-cleanup))
+
+(defun minibuffer-lazy-highlight-setup ()
+ "Set up minibuffer for lazy highlight of matches in the original window.
+
+This function is intended to be added to `minibuffer-setup-hook'.
+Note that several other isearch variables influence the lazy
+highlighting, including `isearch-regexp',
+`isearch-lazy-highlight' and `isearch-lazy-count'."
+ (remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
+ (add-hook 'after-change-functions
+ #'minibuffer-lazy-highlight--after-change)
+ (add-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
+ (add-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
+ (setq minibuffer-lazy-highlight--overlay
+ (and minibuffer-lazy-count-format
+ (make-overlay (point-min) (point-min) (current-buffer) t)))
+ (minibuffer-lazy-highlight--after-change nil nil nil))
+
+\f
(defun isearch-resume (string regexp word forward message case-fold)
"Resume an incremental search.
STRING is the string or regexp searched for.
diff --git a/lisp/replace.el b/lisp/replace.el
index 06be597855..bd5a694110 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -254,11 +254,31 @@ query-replace-read-from
(query-replace-descr
(cdar query-replace-defaults)))))
(t (format-prompt prompt nil))))
+ ;; Variables controlling lazy highlighting while reading
+ ;; FROM regexp, which is set up below.
+ (isearch-lazy-highlight query-replace-lazy-highlight)
+ (isearch-regexp regexp-flag)
+ (isearch-regexp-function nil)
+ (isearch-case-fold-search case-fold-search) ;; FIXME: the case-folding rule here is complicated...
+ (minibuffer-lazy-highlight-transform
+ (lambda (string)
+ (let ((from (query-replace--split-string string)))
+ (if (consp from) (car from) from))))
(from
;; The save-excursion here is in case the user marks and copies
;; a region in order to specify the minibuffer input.
;; That should not clobber the region for the query-replace itself.
(save-excursion
+ (when query-replace-lazy-highlight
+ (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
+ (when (use-region-p)
+ (letrec ((region-filter (replace--region-filter
+ (funcall region-extract-function 'bounds)))
+ (cleanup (lambda ()
+ (remove-function isearch-filter-predicate region-filter)
+ (remove-hook 'minibuffer-exit-hook cleanup))))
+ (add-function :after-while isearch-filter-predicate region-filter)
+ (add-hook 'minibuffer-exit-hook cleanup))))
(minibuffer-with-setup-hook
(lambda ()
(setq-local text-property-default-nonsticky
@@ -2773,6 +2793,26 @@ replace--push-stack
,search-str ,next-replace)
,stack))
+(defun replace--region-filter (bounds)
+ "Return a function that decides if a region is inside BOUNDS.
+BOUNDS is a list of cons cells of the form (START . END). The
+returned function takes as argument two buffer positions, START
+and END."
+ (let ((region-bounds
+ (mapcar (lambda (position)
+ (cons (copy-marker (car position))
+ (copy-marker (cdr position))))
+ bounds)))
+ (lambda (start end)
+ (delq nil (mapcar
+ (lambda (bounds)
+ (and
+ (>= start (car bounds))
+ (<= start (cdr bounds))
+ (>= end (car bounds))
+ (<= end (cdr bounds))))
+ region-bounds)))))
+
(defun perform-replace (from-string replacements
query-flag regexp-flag delimited-flag
&optional repeat-count map start end backward region-noncontiguous-p)
@@ -2857,22 +2897,8 @@ perform-replace
;; Unless a single contiguous chunk is selected, operate on multiple chunks.
(when region-noncontiguous-p
- (let ((region-bounds
- (mapcar (lambda (position)
- (cons (copy-marker (car position))
- (copy-marker (cdr position))))
- (funcall region-extract-function 'bounds))))
- (setq region-filter
- (lambda (start end)
- (delq nil (mapcar
- (lambda (bounds)
- (and
- (>= start (car bounds))
- (<= start (cdr bounds))
- (>= end (car bounds))
- (<= end (cdr bounds))))
- region-bounds))))
- (add-function :after-while isearch-filter-predicate region-filter)))
+ (setq region-filter (replace--region-filter
+ (funcall region-extract-function 'bounds))))
;; If region is active, in Transient Mark mode, operate on region.
(if backward
--
2.35.1
[-- Attachment #3: Type: text/plain, Size: 2743 bytes --]
On Mon, 10 Jan 2022 at 21:09, Juri Linkov <juri@linkov.net> wrote:
>> I attached a new patch (still a sketch) that requires no changes in
>> comint.el and simple.el. Perhaps you will find this approach more
>> acceptable.
>
> Thanks, no changes in other files is certainly a big plus.
>
>>> It would be great to use your new variable with a function
>>> to show replacement counts in perform-replace. IIUC,
>>> let-binding isearch-lazy-count-display-function to
>>> isearch-read-with-highlight-count will suppress isearch-message?
>>
>> I tried this and it's relatively simple to do, but there is a problem.
>> Suppose you want to replace all "a" with "z", and your buffer has 20
>> "a"s initially. Then, as you keep hitting "y" to confirm a replacement
>> the count will be
>>
>> 1/20, 1/19, ..., 1/1
>
> This is an interesting question. I tried in other editors,
> and e.g. in the editor xed that is installed by default,
> this is exactly what is displayed: 1/20, 1/19, ..., 1/1.
Let's do this at a later point, to keep this patch smaller and more
focused.
Note that there is one further fancy feature from anzu that we could add
eventually, namely the preview of the replacement text during replace.
This was requested in some recent bug. I think it's not so often that
such a thing would be useful, but it can be very handy occasionally.
>> since the number of "a"s decrease, and the point is always at the first
>> of the still-existing ones. But probably one should count the number of
>> prompts, so
>>
>> 1/20, 2/20, ..., 20/20
>>
>> I think this means `perform-replace' has to implement its own way to
>> display a count.
>
> Maybe this makes more sense, when the users will learn
> what do these numbers mean.
>
>>> I meant using simply
>>>
>>> (add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
>>>
>>> But it seems isearch-read-with-highlight-setup doesn't set
>>> isearch-lazy-count-display-function.
What used to be 'isearch-read-with-highlight-setup' is now
'minibuffer-lazy-highlight-setup'.
Just to make sure I understood you: your suggestion is for this function
to remove itself automagically from the minibuffer setup hook, to
dispense with the need of 'minibuffer-with-setup-hook'? This seems
handy but unusual, hence my question.
>> I guess this could be done.
>
> Maybe two separate hooks could be defined? One highlights like
> lazy-highlight, and another counts like lazy-count does:
>
> (add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
> (add-hook 'minibuffer-setup-hook 'isearch-read-with-count-setup)
The highlight without counting can be achieved by binding a suitable new
variable. Counting without highlight is not supported by isearch AFAIU.
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-02-26 16:13 ` Augusto Stoffel
@ 2022-03-15 17:09 ` Juri Linkov
2022-03-15 21:33 ` Augusto Stoffel
2022-03-15 17:24 ` Juri Linkov
1 sibling, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-15 17:09 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> Inlined below some further comments.
>
>>>> I meant using simply
>>>>
>>>> (add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
>>>>
>>>> But it seems isearch-read-with-highlight-setup doesn't set
>>>> isearch-lazy-count-display-function.
>
> What used to be 'isearch-read-with-highlight-setup' is now
> 'minibuffer-lazy-highlight-setup'.
>
> Just to make sure I understood you: your suggestion is for this function
> to remove itself automagically from the minibuffer setup hook, to
> dispense with the need of 'minibuffer-with-setup-hook'? This seems
> handy but unusual, hence my question.
> ...
> - In a previous message, you seemed to suggest making
> `minibuffer-lazy-highlight-setup' self-cleaning form the
> minibuffer-setup-hook, so as to dispense with the need for
> `with-minibuffer-setup-hook'. I did exactly that, but since I haven't
> seen this convention before, I wanted to double check.
Actually, my comment was about having the line that you already added
in your latest patch in 'minibuffer-lazy-highlight-setup':
(add-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
So I don't see the need to have this line:
(remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
Also I noticed that you changed the function 'isearch-lazy-count-display-function'
to the hook 'lazy-count-update-hook', and this looks fine,
I see no problem with this.
> - Besides query-replace, I only added lazy highlight to
> isearch-edit-string for now.
BTW, what is the relation between the minibuffer-lazy-highlight feature
and another proposed feature that immediately updates the search in
the buffer while editing the string in the minibuffer by isearch-edit-string?
Can minibuffer-lazy-highlight be considered as a lightweight version of
the buffer search from the minibuffer?
> There are a few more we could add (perhaps later),
> such as `occur' and `keep-lines'.
I tried (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
in the minibuffer of 'occur' and others, and it works nicely.
Maybe it could even semi-deprecate the package re-builder.el.
Thanks for this generally usable feature.
> - There's no customization variable to enable the minibuffer lazy
> highlight. The rationale is that each command that will use it should
> define its own user option (or use an existing one). For
> `isearch-edit-string' it's `isearch-lazy-highlight'; for
> `query-replace' it's `query-replace-lazy-highlight'; and so on.
A common customizable option to enable this everywhere would be nice too.
Maybe disabling is already possible by customizing
'minibuffer-lazy-count-format' to nil? Then the checks for
non-nil 'minibuffer-lazy-count-format' could be added to
more places, such as to wrap the whole '(condition-case error'
in query-replace-read-args with the 'when' condition, etc.
> - As to the lazy count during `perform-replace': I would like to leave
> this for later. In fact, I think the lazy highlight has some issues
> that need fixing beforehand. For instance, if I replace "a" with
> "aba", then the "a"'s from the replacement text also get lazy
> highlighted. We shouldn't refresh the lazy highlight during
> `preform-replace'. Then adding lazy count on top should be easy.
Patches welcome.
>>> I guess this could be done.
>>
>> Maybe two separate hooks could be defined? One highlights like
>> lazy-highlight, and another counts like lazy-count does:
>>
>> (add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
>> (add-hook 'minibuffer-setup-hook 'isearch-read-with-count-setup)
>
> The highlight without counting can be achieved by binding a suitable new
> variable. Counting without highlight is not supported by isearch AFAIU.
Counting without highlighting is only possible by redefining the function
'isearch-lazy-highlight-match' to a no-op function.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-02-26 16:13 ` Augusto Stoffel
2022-03-15 17:09 ` Juri Linkov
@ 2022-03-15 17:24 ` Juri Linkov
2022-03-15 21:21 ` Augusto Stoffel
1 sibling, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-15 17:24 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> Sorry for getting back to this after such a long time. I've attached a
> new patch that hopefully is good to merge, except for adding some NEWS
> entry. Let me know what you think.
Please see comments for your latest patch below:
> @@ -1812,6 +1812,8 @@ isearch-edit-string
> (minibuffer-history-symbol)
> ;; Search string might have meta information on text properties.
> (minibuffer-allow-text-properties t))
> + (when isearch-lazy-highlight
> + (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup))
Since this does both highlighting and counting, shouldn't the condition be
(when (and isearch-lazy-highlight isearch-lazy-count))
Or maybe new separate customizable options are needed, e.g.
'minibuffer-lazy-highlight' and 'minibuffer-lazy-count'?
> @@ -2350,7 +2352,9 @@ isearch-query-replace
> (isearch-recursive-edit nil)
> (isearch-string-propertized
> (isearch-string-propertize isearch-string)))
> - (isearch-done nil t)
> + (let ((lazy-highlight-cleanup (and lazy-highlight-cleanup
> + (not query-replace-lazy-highlight))))
> + (isearch-done nil t))
Is this some optimization? It seems it's intended to leave
some existing highlighting? Is this to avoid double highlighting?
Also maybe this condition could use a new variable as well.
> @@ -4048,7 +4056,7 @@ isearch-lazy-highlight-new-loop
> isearch-lazy-highlight-window-end))))))
> ;; something important did indeed change
> (lazy-highlight-cleanup t (not (equal isearch-string ""))) ;stop old timer
> - (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
> + (when isearch-lazy-count
> ...
> @@ -4067,7 +4075,10 @@ isearch-lazy-highlight-new-loop
> (setq isearch-lazy-count-current nil
> isearch-lazy-count-total nil)
> ;; Delay updating the message if possible, to avoid flicker
> - (when (string-equal isearch-string "") (isearch-message))))
> + (when (string-equal isearch-string "")
> + (when (and isearch-mode (null isearch-message-function))
> + (isearch-message))
> ...
> @@ -4120,13 +4131,15 @@ isearch-lazy-highlight-new-loop
> 'isearch-lazy-highlight-start))))
> ;; Update the current match number only in isearch-mode and
> ;; unless isearch-mode is used specially with isearch-message-function
> - (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
> + (when isearch-lazy-count
The problem is that when these conditions 'isearch-mode (null isearch-message-function)'
are removed, now this shows wrong counts in the minibuffer history search
(e.g. 'M-! C-r s C-r C-r ...') and the shell history search
(e.g. 'M-x shell RET M-r s C-r C-r ...'). Before this change
counting was disabled in the history search because it shows wrong numbers.
> +(defun minibuffer-lazy-highlight--count ()
> + "Display total match count in the minibuffer prompt."
> + (when minibuffer-lazy-highlight--overlay
> + (overlay-put minibuffer-lazy-highlight--overlay
> + 'after-string
> + (and isearch-lazy-count-total
> + (not isearch-error)
> + (format minibuffer-lazy-count-format
> + isearch-lazy-count-total)))))
> ...
> + (setq minibuffer-lazy-highlight--overlay
> + (and minibuffer-lazy-count-format
> + (make-overlay (point-min) (point-min) (current-buffer) t)))
For some reasons the package lisp/mb-depth.el uses 'after-string'
instead of 'before-string', and (make-overlay (point-min) (1+ (point-min)))
instead of (make-overlay (point-min) (point-min)),
so maybe better to do the same?
> @@ -365,14 +372,44 @@ query-replace-read-args
> + (condition-case error
> + (let (;; Variables controlling lazy highlighting while reading
> + ;; FROM and TO.
> + (lazy-highlight-cleanup nil)
> + (isearch-lazy-highlight query-replace-lazy-highlight)
> + (isearch-regexp regexp-flag)
> + (isearch-regexp-function nil)
Highlighting is still incorrect for word replacement ('C-u M-%')
and for non-nil 'replace-char-fold'. To handle these cases correctly,
'replace-highlight' uses:
(isearch-regexp-function (or replace-regexp-function
delimited-flag
(and replace-char-fold
(not regexp-flag)
#'char-fold-to-regexp)))
> @@ -2857,22 +2914,8 @@ perform-replace
> (when region-noncontiguous-p
> - (let ((region-bounds
> - (mapcar (lambda (position)
> - (cons (copy-marker (car position))
> - (copy-marker (cdr position))))
> - (funcall region-extract-function 'bounds))))
> - (setq region-filter
> - (lambda (start end)
> - (delq nil (mapcar
> - (lambda (bounds)
> - (and
> - (>= start (car bounds))
> - (<= start (cdr bounds))
> - (>= end (car bounds))
> - (<= end (cdr bounds))))
> - region-bounds))))
> - (add-function :after-while isearch-filter-predicate region-filter)))
> + (setq region-filter (replace--region-filter
> + (funcall region-extract-function 'bounds))))
I wonder why (add-function :after-while isearch-filter-predicate region-filter)
is removed?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-15 17:24 ` Juri Linkov
@ 2022-03-15 21:21 ` Augusto Stoffel
2022-03-16 19:02 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-15 21:21 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 7125 bytes --]
On Tue, 15 Mar 2022 at 19:24, Juri Linkov <juri@linkov.net> wrote:
>> Sorry for getting back to this after such a long time. I've attached a
>> new patch that hopefully is good to merge, except for adding some NEWS
>> entry. Let me know what you think.
>
> Please see comments for your latest patch below:
>
>> @@ -1812,6 +1812,8 @@ isearch-edit-string
>> (minibuffer-history-symbol)
>> ;; Search string might have meta information on text properties.
>> (minibuffer-allow-text-properties t))
>> + (when isearch-lazy-highlight
>> + (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup))
>
> Since this does both highlighting and counting, shouldn't the condition be
>
> (when (and isearch-lazy-highlight isearch-lazy-count))
>
> Or maybe new separate customizable options are needed, e.g.
> 'minibuffer-lazy-highlight' and 'minibuffer-lazy-count'?
isearch-edit-string already has the behavior your expect: if
isearch-lazy-count is nil but isearch-lazy-highlight is t, then the
matches are highlighted but the count is not displayed.
This happens because the new lazy-count-update-hook is only run when
isearch-lazy-highlight is non-nil.
I see no need for a customization variable, since the user already gets
exactly what they asked for.
>> @@ -2350,7 +2352,9 @@ isearch-query-replace
>> (isearch-recursive-edit nil)
>> (isearch-string-propertized
>> (isearch-string-propertize isearch-string)))
>> - (isearch-done nil t)
>> + (let ((lazy-highlight-cleanup (and lazy-highlight-cleanup
>> + (not query-replace-lazy-highlight))))
>> + (isearch-done nil t))
>
> Is this some optimization? It seems it's intended to leave
> some existing highlighting? Is this to avoid double highlighting?
>
> Also maybe this condition could use a new variable as well.
The weird let-binding for lazy-highlight-cleanup is indeed an
optimization. Without it, you can see the lazy highlights briefly flash
off an on again after you hit RET to confirm the replacement string.
(Same remark explains why condition-case is used instead of a
unwind-protect in query-replace-read-args).
>> @@ -4048,7 +4056,7 @@ isearch-lazy-highlight-new-loop
>> isearch-lazy-highlight-window-end))))))
>> ;; something important did indeed change
>> (lazy-highlight-cleanup t (not (equal isearch-string ""))) ;stop old timer
>> - (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
>> + (when isearch-lazy-count
>> ...
>> @@ -4067,7 +4075,10 @@ isearch-lazy-highlight-new-loop
>> (setq isearch-lazy-count-current nil
>> isearch-lazy-count-total nil)
>> ;; Delay updating the message if possible, to avoid flicker
>> - (when (string-equal isearch-string "") (isearch-message))))
>> + (when (string-equal isearch-string "")
>> + (when (and isearch-mode (null isearch-message-function))
>> + (isearch-message))
>> ...
>> @@ -4120,13 +4131,15 @@ isearch-lazy-highlight-new-loop
>> 'isearch-lazy-highlight-start))))
>> ;; Update the current match number only in isearch-mode and
>> ;; unless isearch-mode is used specially with isearch-message-function
>> - (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
>> + (when isearch-lazy-count
>
> The problem is that when these conditions 'isearch-mode (null isearch-message-function)'
> are removed, now this shows wrong counts in the minibuffer history search
> (e.g. 'M-! C-r s C-r C-r ...') and the shell history search
> (e.g. 'M-x shell RET M-r s C-r C-r ...'). Before this change
> counting was disabled in the history search because it shows wrong numbers.
>
Okay, so this means we should bind isearch-lazy-count to nil in these
commands, do you agree? It has always looked like a hack to me to check
for (null isearch-message-function).
>> +(defun minibuffer-lazy-highlight--count ()
>> + "Display total match count in the minibuffer prompt."
>> + (when minibuffer-lazy-highlight--overlay
>> + (overlay-put minibuffer-lazy-highlight--overlay
>> + 'after-string
>> + (and isearch-lazy-count-total
>> + (not isearch-error)
>> + (format minibuffer-lazy-count-format
>> + isearch-lazy-count-total)))))
>> ...
>> + (setq minibuffer-lazy-highlight--overlay
>> + (and minibuffer-lazy-count-format
>> + (make-overlay (point-min) (point-min) (current-buffer) t)))
>
> For some reasons the package lisp/mb-depth.el uses 'after-string'
> instead of 'before-string', and (make-overlay (point-min) (1+ (point-min)))
> instead of (make-overlay (point-min) (point-min)),
> so maybe better to do the same?
>
Okay, but do you know the reasons? I've changed to before-string, but I
don't like to make the overlay 1 char longer than it has to be :-P
>> @@ -365,14 +372,44 @@ query-replace-read-args
>> + (condition-case error
>> + (let (;; Variables controlling lazy highlighting while reading
>> + ;; FROM and TO.
>> + (lazy-highlight-cleanup nil)
>> + (isearch-lazy-highlight query-replace-lazy-highlight)
>> + (isearch-regexp regexp-flag)
>> + (isearch-regexp-function nil)
>
> Highlighting is still incorrect for word replacement ('C-u M-%')
> and for non-nil 'replace-char-fold'. To handle these cases correctly,
> 'replace-highlight' uses:
>
> (isearch-regexp-function (or replace-regexp-function
> delimited-flag
> (and replace-char-fold
> (not regexp-flag)
> #'char-fold-to-regexp)))
Okay, fixed this. (BTW, where is replace-regexp-function used? It's
not set anywhere in Emacs, and it's not a defcustom either.)
>> @@ -2857,22 +2914,8 @@ perform-replace
>> (when region-noncontiguous-p
>> - (let ((region-bounds
>> - (mapcar (lambda (position)
>> - (cons (copy-marker (car position))
>> - (copy-marker (cdr position))))
>> - (funcall region-extract-function 'bounds))))
>> - (setq region-filter
>> - (lambda (start end)
>> - (delq nil (mapcar
>> - (lambda (bounds)
>> - (and
>> - (>= start (car bounds))
>> - (<= start (cdr bounds))
>> - (>= end (car bounds))
>> - (<= end (cdr bounds))))
>> - region-bounds))))
>> - (add-function :after-while isearch-filter-predicate region-filter)))
>> + (setq region-filter (replace--region-filter
>> + (funcall region-extract-function 'bounds))))
>
> I wonder why (add-function :after-while isearch-filter-predicate region-filter)
> is removed?
Oops, that was a very serious typo. I've fixed it now.
Here are the changes I applied on top of my previous patch:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Changes-after-Juri-s-comments.patch --]
[-- Type: text/x-patch, Size: 3928 bytes --]
From 216c32ad2f74abf08ea10eb2b11a5c93a146fbe1 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Tue, 15 Mar 2022 22:11:36 +0100
Subject: [PATCH] Changes after Juri's comments
* lisp/comint.el (comint-history-isearch-setup,
comint-history-isearch-end): Make sure no lazy count is displayed.
* lisp/simple.el (minibuffer-history-isearch-setup): Make sure no lazy
count is displayed.
---
lisp/comint.el | 2 ++
lisp/isearch.el | 2 +-
lisp/replace.el | 10 ++++++++--
lisp/simple.el | 1 +
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/lisp/comint.el b/lisp/comint.el
index 4c82e74e4b..56082f622a 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1515,6 +1515,7 @@ comint-history-isearch-setup
#'comint-history-isearch-wrap)
(setq-local isearch-push-state-function
#'comint-history-isearch-push-state)
+ (setq-local isearch-lazy-count nil)
(add-hook 'isearch-mode-end-hook 'comint-history-isearch-end nil t)))
(defun comint-history-isearch-end ()
@@ -1526,6 +1527,7 @@ comint-history-isearch-end
(setq isearch-message-function nil)
(setq isearch-wrap-function nil)
(setq isearch-push-state-function nil)
+ (kill-local-variable 'isearch-lazy-count)
(remove-hook 'isearch-mode-end-hook 'comint-history-isearch-end t)
(unless isearch-suspended
(custom-reevaluate-setting 'comint-history-isearch)))
diff --git a/lisp/isearch.el b/lisp/isearch.el
index e8e3218256..bd337f38b7 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -4372,7 +4372,7 @@ minibuffer-lazy-highlight--count
"Display total match count in the minibuffer prompt."
(when minibuffer-lazy-highlight--overlay
(overlay-put minibuffer-lazy-highlight--overlay
- 'after-string
+ 'before-string
(and isearch-lazy-count-total
(not isearch-error)
(format minibuffer-lazy-count-format
diff --git a/lisp/replace.el b/lisp/replace.el
index 3e1be6f940..f45fb5fb25 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -378,7 +378,12 @@ query-replace-read-args
(lazy-highlight-cleanup nil)
(isearch-lazy-highlight query-replace-lazy-highlight)
(isearch-regexp regexp-flag)
- (isearch-regexp-function nil)
+ (isearch-regexp-function (or replace-regexp-function
+ (and current-prefix-arg
+ (not (eq current-prefix-arg '-)))
+ (and replace-char-fold
+ (not regexp-flag)
+ #'char-fold-to-regexp)))
(isearch-case-fold-search case-fold-search)
(minibuffer-lazy-highlight-transform
(lambda (string)
@@ -2915,7 +2920,8 @@ perform-replace
;; Unless a single contiguous chunk is selected, operate on multiple chunks.
(when region-noncontiguous-p
(setq region-filter (replace--region-filter
- (funcall region-extract-function 'bounds))))
+ (funcall region-extract-function 'bounds)))
+ (add-function :after-while isearch-filter-predicate region-filter))
;; If region is active, in Transient Mark mode, operate on region.
(if backward
diff --git a/lisp/simple.el b/lisp/simple.el
index accc119e2b..61319b6060 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2840,6 +2840,7 @@ minibuffer-history-isearch-setup
#'minibuffer-history-isearch-wrap)
(setq-local isearch-push-state-function
#'minibuffer-history-isearch-push-state)
+ (setq-local isearch-lazy-count nil)
(add-hook 'isearch-mode-end-hook 'minibuffer-history-isearch-end nil t))
(defun minibuffer-history-isearch-end ()
--
2.35.1
[-- Attachment #3: Type: text/plain, Size: 59 bytes --]
And this is the aggregated patch for the entire feature:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0001-Display-lazy-highlight-and-match-count-in-when-readi.patch --]
[-- Type: text/x-patch, Size: 17759 bytes --]
From 3e08f33fe45e41419278a614f7edb4f184ca9f5b Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 12 Mar 2022 10:43:54 +0100
Subject: [PATCH] Display lazy highlight and match count in when reading
regexps
* lisp/isearch.el (lazy-count-update-hook): New hook allowing to
display the lazy count in special ways.
(isearch-edit-string): Add lazy highlight and count of matching text.
(isearch-lazy-highlight-new-loop,
isearch-lazy-highlight-buffer-update): Use
`isearch-lazy-count-display-function' instead of hardcoded call to
`isearch-message'.
(minibuffer-lazy-count-format, minibuffer-lazy-highlight-transform,
minibuffer-lazy-highlight--overlay, minibuffer-lazy-highlight--count,
minibuffer-lazy-highlight--after-change,
minibuffer-lazy-highlight--exit, minibuffer-lazy-highlight-setup):
Variables and functions implementing the lazy highlight functionality
while reading from minibuffer.
* lisp/replace.el (query-replace-read-args): Add lazy highlighting.
(replace--region-filter): New function, extracted from
'perform-replace'.
(perform-replace): Use 'replace--region-filter'.
* lisp/comint.el (comint-history-isearch-setup,
comint-history-isearch-end): Make sure no lazy count is displayed.
* lisp/simple.el (minibuffer-history-isearch-setup): Make sure no lazy
count is displayed.
---
lisp/comint.el | 2 +
lisp/isearch.el | 106 +++++++++++++++++++++++++++++++++++++++++-------
lisp/replace.el | 99 ++++++++++++++++++++++++++++++++------------
lisp/simple.el | 1 +
4 files changed, 169 insertions(+), 39 deletions(-)
diff --git a/lisp/comint.el b/lisp/comint.el
index 4c82e74e4b..56082f622a 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1515,6 +1515,7 @@ comint-history-isearch-setup
#'comint-history-isearch-wrap)
(setq-local isearch-push-state-function
#'comint-history-isearch-push-state)
+ (setq-local isearch-lazy-count nil)
(add-hook 'isearch-mode-end-hook 'comint-history-isearch-end nil t)))
(defun comint-history-isearch-end ()
@@ -1526,6 +1527,7 @@ comint-history-isearch-end
(setq isearch-message-function nil)
(setq isearch-wrap-function nil)
(setq isearch-push-state-function nil)
+ (kill-local-variable 'isearch-lazy-count)
(remove-hook 'isearch-mode-end-hook 'comint-history-isearch-end t)
(unless isearch-suspended
(custom-reevaluate-setting 'comint-history-isearch)))
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 8970216398..bd337f38b7 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1812,6 +1812,8 @@ isearch-edit-string
(minibuffer-history-symbol)
;; Search string might have meta information on text properties.
(minibuffer-allow-text-properties t))
+ (when isearch-lazy-highlight
+ (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup))
(setq isearch-new-string
(read-from-minibuffer
(isearch-message-prefix nil isearch-nonincremental)
@@ -2350,7 +2352,9 @@ isearch-query-replace
(isearch-recursive-edit nil)
(isearch-string-propertized
(isearch-string-propertize isearch-string)))
- (isearch-done nil t)
+ (let ((lazy-highlight-cleanup (and lazy-highlight-cleanup
+ (not query-replace-lazy-highlight))))
+ (isearch-done nil t))
(isearch-clean-overlays)
(if (and isearch-other-end
(if backward
@@ -2366,13 +2370,15 @@ isearch-query-replace
(symbol-value query-replace-from-history-variable)))
(perform-replace
isearch-string-propertized
- (query-replace-read-to
- isearch-string-propertized
- (concat "Query replace"
- (isearch--describe-regexp-mode (or delimited isearch-regexp-function) t)
- (if backward " backward" "")
- (if (use-region-p) " in region" ""))
- isearch-regexp)
+ (unwind-protect
+ (query-replace-read-to
+ isearch-string-propertized
+ (concat "Query replace"
+ (isearch--describe-regexp-mode (or delimited isearch-regexp-function) t)
+ (if backward " backward" "")
+ (if (use-region-p) " in region" ""))
+ isearch-regexp)
+ (lazy-highlight-cleanup lazy-highlight-cleanup))
t isearch-regexp (or delimited isearch-regexp-function) nil nil
(if (use-region-p) (region-beginning))
(if (use-region-p) (region-end))
@@ -3990,6 +3996,8 @@ isearch-lazy-highlight-error
(defvar isearch-lazy-count-current nil)
(defvar isearch-lazy-count-total nil)
(defvar isearch-lazy-count-hash (make-hash-table))
+(defvar lazy-count-update-hook nil
+ "Hook run after new lazy count results are computed.")
(defun lazy-highlight-cleanup (&optional force procrastinate)
"Stop lazy highlighting and remove extra highlighting from current buffer.
@@ -4048,7 +4056,7 @@ isearch-lazy-highlight-new-loop
isearch-lazy-highlight-window-end))))))
;; something important did indeed change
(lazy-highlight-cleanup t (not (equal isearch-string ""))) ;stop old timer
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(when (or (equal isearch-string "")
;; Check if this place was reached by a condition above
;; other than changed window boundaries (that shouldn't
@@ -4067,7 +4075,10 @@ isearch-lazy-highlight-new-loop
(setq isearch-lazy-count-current nil
isearch-lazy-count-total nil)
;; Delay updating the message if possible, to avoid flicker
- (when (string-equal isearch-string "") (isearch-message))))
+ (when (string-equal isearch-string "")
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))))
(setq isearch-lazy-highlight-window-start-changed nil)
(setq isearch-lazy-highlight-window-end-changed nil)
(setq isearch-lazy-highlight-error isearch-error)
@@ -4120,13 +4131,15 @@ isearch-lazy-highlight-new-loop
'isearch-lazy-highlight-start))))
;; Update the current match number only in isearch-mode and
;; unless isearch-mode is used specially with isearch-message-function
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
;; Update isearch-lazy-count-current only when it was already set
;; at the end of isearch-lazy-highlight-buffer-update
(when isearch-lazy-count-current
(setq isearch-lazy-count-current
(gethash (point) isearch-lazy-count-hash 0))
- (isearch-message))))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))))
(defun isearch-lazy-highlight-search (string bound)
"Search ahead for the next or previous match, for lazy highlighting.
@@ -4327,16 +4340,81 @@ isearch-lazy-highlight-buffer-update
(setq looping nil
nomore t))))
(if nomore
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(unless isearch-lazy-count-total
(setq isearch-lazy-count-total 0))
(setq isearch-lazy-count-current
(gethash opoint isearch-lazy-count-hash 0))
- (isearch-message))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))
(setq isearch-lazy-highlight-timer
(run-at-time lazy-highlight-interval nil
'isearch-lazy-highlight-buffer-update)))))))))
+\f
+;; Reading from minibuffer with lazy highlight and match count
+
+(defcustom minibuffer-lazy-count-format "%s "
+ "Format of the total number of matches for the prompt prefix."
+ :type '(choice (const :tag "Don't display a count" nil)
+ (string :tag "Display match count" "%s "))
+ :group 'lazy-count
+ :version "29.1")
+
+(defvar minibuffer-lazy-highlight-transform #'identity
+ "Function to transform minibuffer text into a `isearch-string' for highlighting.")
+
+(defvar minibuffer-lazy-highlight--overlay nil
+ "Overlay for minibuffer prompt updates.")
+
+(defun minibuffer-lazy-highlight--count ()
+ "Display total match count in the minibuffer prompt."
+ (when minibuffer-lazy-highlight--overlay
+ (overlay-put minibuffer-lazy-highlight--overlay
+ 'before-string
+ (and isearch-lazy-count-total
+ (not isearch-error)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)))))
+
+(defun minibuffer-lazy-highlight--after-change (_beg _end _len)
+ "Update lazy highlight state in minibuffer selected window."
+ (when isearch-lazy-highlight
+ (let ((inhibit-redisplay t) ;; Avoid cursor flickering
+ (string (minibuffer-contents)))
+ (with-minibuffer-selected-window
+ (setq isearch-string (funcall minibuffer-lazy-highlight-transform string))
+ (isearch-lazy-highlight-new-loop)))))
+
+(defun minibuffer-lazy-highlight--exit ()
+ "Unwind changes from `minibuffer-lazy-highlight-setup'."
+ (remove-hook 'after-change-functions
+ #'minibuffer-lazy-highlight--after-change)
+ (remove-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
+ (remove-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
+ (setq minibuffer-lazy-highlight--overlay nil)
+ (when lazy-highlight-cleanup
+ (lazy-highlight-cleanup)))
+
+(defun minibuffer-lazy-highlight-setup ()
+ "Set up minibuffer for lazy highlight of matches in the original window.
+
+This function is intended to be added to `minibuffer-setup-hook'.
+Note that several other isearch variables influence the lazy
+highlighting, including `isearch-regexp',
+`isearch-lazy-highlight' and `isearch-lazy-count'."
+ (remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
+ (add-hook 'after-change-functions
+ #'minibuffer-lazy-highlight--after-change)
+ (add-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
+ (add-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
+ (setq minibuffer-lazy-highlight--overlay
+ (and minibuffer-lazy-count-format
+ (make-overlay (point-min) (point-min) (current-buffer) t)))
+ (minibuffer-lazy-highlight--after-change nil nil nil))
+
+\f
(defun isearch-resume (string regexp word forward message case-fold)
"Resume an incremental search.
STRING is the string or regexp searched for.
diff --git a/lisp/replace.el b/lisp/replace.el
index 06be597855..f45fb5fb25 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -352,8 +352,15 @@ query-replace-read-to
(query-replace-compile-replacement
(save-excursion
(let* ((history-add-new-input nil)
+ (count (if (and query-replace-lazy-highlight
+ minibuffer-lazy-count-format
+ isearch-lazy-count
+ isearch-lazy-count-total)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)
+ ""))
(to (read-from-minibuffer
- (format "%s %s with: " prompt (query-replace-descr from))
+ (format "%s%s %s with: " count prompt (query-replace-descr from))
nil nil nil
query-replace-to-history-variable from t)))
(add-to-history query-replace-to-history-variable to nil t)
@@ -365,14 +372,49 @@ query-replace-read-args
(unless noerror
(barf-if-buffer-read-only))
(save-mark-and-excursion
- (let* ((from (query-replace-read-from prompt regexp-flag))
- (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
- (query-replace-read-to from prompt regexp-flag))))
- (list from to
- (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
- (and (plist-member (text-properties-at 0 from) 'isearch-regexp-function)
- (get-text-property 0 'isearch-regexp-function from)))
- (and current-prefix-arg (eq current-prefix-arg '-))))))
+ (condition-case error
+ (let (;; Variables controlling lazy highlighting while reading
+ ;; FROM and TO.
+ (lazy-highlight-cleanup nil)
+ (isearch-lazy-highlight query-replace-lazy-highlight)
+ (isearch-regexp regexp-flag)
+ (isearch-regexp-function (or replace-regexp-function
+ (and current-prefix-arg
+ (not (eq current-prefix-arg '-)))
+ (and replace-char-fold
+ (not regexp-flag)
+ #'char-fold-to-regexp)))
+ (isearch-case-fold-search case-fold-search)
+ (minibuffer-lazy-highlight-transform
+ (lambda (string)
+ (let* ((split (query-replace--split-string string))
+ (from-string (if (consp split) (car split) split)))
+ (when (and case-fold-search search-upper-case)
+ (setq isearch-case-fold-search
+ (isearch-no-upper-case-p from-string regexp-flag)))
+ from-string)))
+ from to)
+ (when query-replace-lazy-highlight
+ (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
+ (when (use-region-p)
+ (letrec ((region-filter (replace--region-filter
+ (funcall region-extract-function 'bounds)))
+ (cleanup (lambda ()
+ (remove-function isearch-filter-predicate region-filter)
+ (remove-hook 'minibuffer-exit-hook cleanup))))
+ (add-function :after-while isearch-filter-predicate region-filter)
+ (add-hook 'minibuffer-exit-hook cleanup))))
+ (setq from (query-replace-read-from prompt regexp-flag))
+ (setq to (if (consp from)
+ (prog1 (cdr from) (setq from (car from)))
+ (query-replace-read-to from prompt regexp-flag)))
+ (list from to
+ (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
+ (and (plist-member (text-properties-at 0 from) 'isearch-regexp-function)
+ (get-text-property 0 'isearch-regexp-function from)))
+ (and current-prefix-arg (eq current-prefix-arg '-))))
+ (t (lazy-highlight-cleanup)
+ (signal (car error) (cdr error))))))
(defun query-replace (from-string to-string &optional delimited start end backward region-noncontiguous-p)
"Replace some occurrences of FROM-STRING with TO-STRING.
@@ -2773,6 +2815,26 @@ replace--push-stack
,search-str ,next-replace)
,stack))
+(defun replace--region-filter (bounds)
+ "Return a function that decides if a region is inside BOUNDS.
+BOUNDS is a list of cons cells of the form (START . END). The
+returned function takes as argument two buffer positions, START
+and END."
+ (let ((region-bounds
+ (mapcar (lambda (position)
+ (cons (copy-marker (car position))
+ (copy-marker (cdr position))))
+ bounds)))
+ (lambda (start end)
+ (delq nil (mapcar
+ (lambda (bounds)
+ (and
+ (>= start (car bounds))
+ (<= start (cdr bounds))
+ (>= end (car bounds))
+ (<= end (cdr bounds))))
+ region-bounds)))))
+
(defun perform-replace (from-string replacements
query-flag regexp-flag delimited-flag
&optional repeat-count map start end backward region-noncontiguous-p)
@@ -2857,22 +2919,9 @@ perform-replace
;; Unless a single contiguous chunk is selected, operate on multiple chunks.
(when region-noncontiguous-p
- (let ((region-bounds
- (mapcar (lambda (position)
- (cons (copy-marker (car position))
- (copy-marker (cdr position))))
- (funcall region-extract-function 'bounds))))
- (setq region-filter
- (lambda (start end)
- (delq nil (mapcar
- (lambda (bounds)
- (and
- (>= start (car bounds))
- (<= start (cdr bounds))
- (>= end (car bounds))
- (<= end (cdr bounds))))
- region-bounds))))
- (add-function :after-while isearch-filter-predicate region-filter)))
+ (setq region-filter (replace--region-filter
+ (funcall region-extract-function 'bounds)))
+ (add-function :after-while isearch-filter-predicate region-filter))
;; If region is active, in Transient Mark mode, operate on region.
(if backward
diff --git a/lisp/simple.el b/lisp/simple.el
index accc119e2b..61319b6060 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2840,6 +2840,7 @@ minibuffer-history-isearch-setup
#'minibuffer-history-isearch-wrap)
(setq-local isearch-push-state-function
#'minibuffer-history-isearch-push-state)
+ (setq-local isearch-lazy-count nil)
(add-hook 'isearch-mode-end-hook 'minibuffer-history-isearch-end nil t))
(defun minibuffer-history-isearch-end ()
--
2.35.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-15 17:09 ` Juri Linkov
@ 2022-03-15 21:33 ` Augusto Stoffel
2022-03-16 18:56 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-15 21:33 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Tue, 15 Mar 2022 at 19:09, Juri Linkov <juri@linkov.net> wrote:
>> Inlined below some further comments.
>>
>>>>> I meant using simply
>>>>>
>>>>> (add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
>>>>>
>>>>> But it seems isearch-read-with-highlight-setup doesn't set
>>>>> isearch-lazy-count-display-function.
>>
>> What used to be 'isearch-read-with-highlight-setup' is now
>> 'minibuffer-lazy-highlight-setup'.
>>
>> Just to make sure I understood you: your suggestion is for this function
>> to remove itself automagically from the minibuffer setup hook, to
>> dispense with the need of 'minibuffer-with-setup-hook'? This seems
>> handy but unusual, hence my question.
>> ...
>> - In a previous message, you seemed to suggest making
>> `minibuffer-lazy-highlight-setup' self-cleaning form the
>> minibuffer-setup-hook, so as to dispense with the need for
>> `with-minibuffer-setup-hook'. I did exactly that, but since I haven't
>> seen this convention before, I wanted to double check.
>
> Actually, my comment was about having the line that you already added
> in your latest patch in 'minibuffer-lazy-highlight-setup':
>
> (add-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
>
> So I don't see the need to have this line:
>
> (remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
Okay, but if I remove this line, then all calls to
minibuffer-lazy-highlight-setup will require a with-minibuffer-setup
hook. And this will be awkward:
(minibuffer-with-setup-hook (if isearch-lazy-highlight
#'minibuffer-lazy-highlight-setup
#'ignore)
(read-from-minibuffer "Something: "))
> Also I noticed that you changed the function 'isearch-lazy-count-display-function'
> to the hook 'lazy-count-update-hook', and this looks fine,
> I see no problem with this.
>
>> - Besides query-replace, I only added lazy highlight to
>> isearch-edit-string for now.
>
> BTW, what is the relation between the minibuffer-lazy-highlight feature
> and another proposed feature that immediately updates the search in
> the buffer while editing the string in the minibuffer by isearch-edit-string?
> Can minibuffer-lazy-highlight be considered as a lightweight version of
> the buffer search from the minibuffer?
Well, there's a package for that on ELPA (isearch-mb), so extending
isearch-edit-string to do that seems superfluous now?
>> There are a few more we could add (perhaps later),
>> such as `occur' and `keep-lines'.
>
> I tried (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
> in the minibuffer of 'occur' and others, and it works nicely.
> Maybe it could even semi-deprecate the package re-builder.el.
>
> Thanks for this generally usable feature.
By the way, this is a byproduct of that long discussion that led to
isearch-mb, so it was not all in vain :-).
>> - There's no customization variable to enable the minibuffer lazy
>> highlight. The rationale is that each command that will use it should
>> define its own user option (or use an existing one). For
>> `isearch-edit-string' it's `isearch-lazy-highlight'; for
>> `query-replace' it's `query-replace-lazy-highlight'; and so on.
>
> A common customizable option to enable this everywhere would be nice too.
> Maybe disabling is already possible by customizing
> 'minibuffer-lazy-count-format' to nil? Then the checks for
> non-nil 'minibuffer-lazy-count-format' could be added to
> more places, such as to wrap the whole '(condition-case error'
> in query-replace-read-args with the 'when' condition, etc.
Yes, the user can set minibuffer-lazy-count-format to nil to get rid of
the lazy count.
Concerning query-replace, why would anyone want to have lazy highlight
during the perform-replace loop, but not earlier? I'm not a fan of
adding a custom option here, not because it would be hard, but because
it seems totally unnecessary.
>> - As to the lazy count during `perform-replace': I would like to leave
>> this for later. In fact, I think the lazy highlight has some issues
>> that need fixing beforehand. For instance, if I replace "a" with
>> "aba", then the "a"'s from the replacement text also get lazy
>> highlighted. We shouldn't refresh the lazy highlight during
>> `preform-replace'. Then adding lazy count on top should be easy.
>
> Patches welcome.
>
>>>> I guess this could be done.
>>>
>>> Maybe two separate hooks could be defined? One highlights like
>>> lazy-highlight, and another counts like lazy-count does:
>>>
>>> (add-hook 'minibuffer-setup-hook 'isearch-read-with-highlight-setup)
>>> (add-hook 'minibuffer-setup-hook 'isearch-read-with-count-setup)
>>
>> The highlight without counting can be achieved by binding a suitable new
>> variable. Counting without highlight is not supported by isearch AFAIU.
>
> Counting without highlighting is only possible by redefining the function
> 'isearch-lazy-highlight-match' to a no-op function.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-15 21:33 ` Augusto Stoffel
@ 2022-03-16 18:56 ` Juri Linkov
2022-03-16 20:09 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-16 18:56 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> So I don't see the need to have this line:
>>
>> (remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
>
> Okay, but if I remove this line, then all calls to
> minibuffer-lazy-highlight-setup will require a with-minibuffer-setup
> hook. And this will be awkward:
>
> (minibuffer-with-setup-hook (if isearch-lazy-highlight
> #'minibuffer-lazy-highlight-setup
> #'ignore)
> (read-from-minibuffer "Something: "))
The answer depends on another question: how do you intend the users
would enable this feature? When to enable it in all minibuffers
(like e.g. minibuffer-depth-indicate-mode does) the users will add
to their init files:
(add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
Then removing the user's customization would not be a nice thing to do.
OTOH, minibuffer-with-setup-hook will remove only own hook, not the user's one.
So to allow enabling this feature selectively, minibuffer-with-setup-hook
is not quite awkward.
>> BTW, what is the relation between the minibuffer-lazy-highlight feature
>> and another proposed feature that immediately updates the search in
>> the buffer while editing the string in the minibuffer by isearch-edit-string?
>> Can minibuffer-lazy-highlight be considered as a lightweight version of
>> the buffer search from the minibuffer?
>
> Well, there's a package for that on ELPA (isearch-mb), so extending
> isearch-edit-string to do that seems superfluous now?
It's still possible to add this feature to isearch-edit-string,
when the change would not be too enormous. I recall squeezing
it into a small patch, but unfortunately it requires changes
in keymap priorities.
>>> There are a few more we could add (perhaps later),
>>> such as `occur' and `keep-lines'.
>>
>> I tried (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
>> in the minibuffer of 'occur' and others, and it works nicely.
>> Maybe it could even semi-deprecate the package re-builder.el.
>>
>> Thanks for this generally usable feature.
>
> By the way, this is a byproduct of that long discussion that led to
> isearch-mb, so it was not all in vain :-).
Are you sure these features can't be combined? One feature basically
runs isearch-search-and-update in the buffer from the minibuffer,
and this feature runs isearch-lazy-highlight-new-loop.
>>> - There's no customization variable to enable the minibuffer lazy
>>> highlight. The rationale is that each command that will use it should
>>> define its own user option (or use an existing one). For
>>> `isearch-edit-string' it's `isearch-lazy-highlight'; for
>>> `query-replace' it's `query-replace-lazy-highlight'; and so on.
>>
>> A common customizable option to enable this everywhere would be nice too.
>> Maybe disabling is already possible by customizing
>> 'minibuffer-lazy-count-format' to nil? Then the checks for
>> non-nil 'minibuffer-lazy-count-format' could be added to
>> more places, such as to wrap the whole '(condition-case error'
>> in query-replace-read-args with the 'when' condition, etc.
>
> Yes, the user can set minibuffer-lazy-count-format to nil to get rid of
> the lazy count.
>
> Concerning query-replace, why would anyone want to have lazy highlight
> during the perform-replace loop, but not earlier? I'm not a fan of
> adding a custom option here, not because it would be hard, but because
> it seems totally unnecessary.
Maybe a new option would make sense for the same reason why there is
the option isearch-lazy-count?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-15 21:21 ` Augusto Stoffel
@ 2022-03-16 19:02 ` Juri Linkov
2022-03-16 20:25 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-16 19:02 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>>> @@ -2350,7 +2352,9 @@ isearch-query-replace
>>> + (let ((lazy-highlight-cleanup (and lazy-highlight-cleanup
>>> + (not query-replace-lazy-highlight))))
>>> + (isearch-done nil t))
>>
>> Is this some optimization? It seems it's intended to leave
>> some existing highlighting? Is this to avoid double highlighting?
>
> The weird let-binding for lazy-highlight-cleanup is indeed an
> optimization. Without it, you can see the lazy highlights briefly flash
> off an on again after you hit RET to confirm the replacement string.
>
> (Same remark explains why condition-case is used instead of a
> unwind-protect in query-replace-read-args).
When I tried your latest patch, it still flashes when it starts
the perform-replace loop.
>> The problem is that when these conditions 'isearch-mode (null isearch-message-function)'
>> are removed, now this shows wrong counts in the minibuffer history search
>> (e.g. 'M-! C-r s C-r C-r ...') and the shell history search
>> (e.g. 'M-x shell RET M-r s C-r C-r ...'). Before this change
>> counting was disabled in the history search because it shows wrong numbers.
>
> Okay, so this means we should bind isearch-lazy-count to nil in these
> commands, do you agree? It has always looked like a hack to me to check
> for (null isearch-message-function).
Binding isearch-lazy-count to nil could serve as a temporary measure
until lazy-count will be supported in the history search.
>> For some reasons the package lisp/mb-depth.el uses 'after-string'
>> instead of 'before-string', and (make-overlay (point-min) (1+ (point-min)))
>> instead of (make-overlay (point-min) (point-min)),
>> so maybe better to do the same?
>
> Okay, but do you know the reasons? I've changed to before-string, but I
> don't like to make the overlay 1 char longer than it has to be :-P
I don't know the reason. Since it works in your patch, then it's ok.
>> Highlighting is still incorrect for word replacement ('C-u M-%')
>> and for non-nil 'replace-char-fold'. To handle these cases correctly,
>> 'replace-highlight' uses:
>>
>> (isearch-regexp-function (or replace-regexp-function
>> delimited-flag
>> (and replace-char-fold
>> (not regexp-flag)
>> #'char-fold-to-regexp)))
>
> Okay, fixed this. (BTW, where is replace-regexp-function used? It's
> not set anywhere in Emacs, and it's not a defcustom either.)
'replace-regexp-function' was added recently in bug#52558
to allow implementing more regexp types in bug#54017.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-16 18:56 ` Juri Linkov
@ 2022-03-16 20:09 ` Augusto Stoffel
2022-03-17 17:09 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-16 20:09 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Wed, 16 Mar 2022 at 20:56, Juri Linkov <juri@linkov.net> wrote:
>>> So I don't see the need to have this line:
>>>
>>> (remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
>>
>> Okay, but if I remove this line, then all calls to
>> minibuffer-lazy-highlight-setup will require a with-minibuffer-setup
>> hook. And this will be awkward:
>>
>> (minibuffer-with-setup-hook (if isearch-lazy-highlight
>> #'minibuffer-lazy-highlight-setup
>> #'ignore)
>> (read-from-minibuffer "Something: "))
>
> The answer depends on another question: how do you intend the users
> would enable this feature? When to enable it in all minibuffers
> (like e.g. minibuffer-depth-indicate-mode does) the users will add
> to their init files:
>
> (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
>
> Then removing the user's customization would not be a nice thing to do.
> OTOH, minibuffer-with-setup-hook will remove only own hook, not the user's one.
> So to allow enabling this feature selectively, minibuffer-with-setup-hook
> is not quite awkward.
My idea is:
- The users of the feature are Elisp programmers / package authors.
- I don't think end users can meaningfully do anything directly with
this new minibuffer hook.
- If package X wants to take advantage of the feature, then it will
either add minibuffer-lazy-highlight-setup to the
minibuffer-setup-hook unconditionally, or it will define an
X-lazy-highlight customization option to control this.
So I think the conclusion is that the current approach in my patch is an
good way to proceed here?
>>> BTW, what is the relation between the minibuffer-lazy-highlight feature
>>> and another proposed feature that immediately updates the search in
>>> the buffer while editing the string in the minibuffer by isearch-edit-string?
>>> Can minibuffer-lazy-highlight be considered as a lightweight version of
>>> the buffer search from the minibuffer?
>>
>> Well, there's a package for that on ELPA (isearch-mb), so extending
>> isearch-edit-string to do that seems superfluous now?
>
> It's still possible to add this feature to isearch-edit-string,
> when the change would not be too enormous. I recall squeezing
> it into a small patch, but unfortunately it requires changes
> in keymap priorities.
I would suggest taking a look at isearch-mb. I think the code is pretty
tight, and I would be unable to shorten the implementation other than by
deleting comment :-)
>>>> There are a few more we could add (perhaps later),
>>>> such as `occur' and `keep-lines'.
>>>
>>> I tried (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
>>> in the minibuffer of 'occur' and others, and it works nicely.
>>> Maybe it could even semi-deprecate the package re-builder.el.
>>>
>>> Thanks for this generally usable feature.
>>
>> By the way, this is a byproduct of that long discussion that led to
>> isearch-mb, so it was not all in vain :-).
>
> Are you sure these features can't be combined? One feature basically
> runs isearch-search-and-update in the buffer from the minibuffer,
> and this feature runs isearch-lazy-highlight-new-loop.
For one thing, isearch-mode has 2 essential commands (repeat forward and
backwards), a couple more necessary ones (quit, abort, scroll,
beginning/end of buffer, mode toggles), and then a number of commands
that end the search with a special action (query-replace, etc.).
These little details add up to the 283 lines in isearch-mb.el currently
has.
>>>> - There's no customization variable to enable the minibuffer lazy
>>>> highlight. The rationale is that each command that will use it should
>>>> define its own user option (or use an existing one). For
>>>> `isearch-edit-string' it's `isearch-lazy-highlight'; for
>>>> `query-replace' it's `query-replace-lazy-highlight'; and so on.
>>>
>>> A common customizable option to enable this everywhere would be nice too.
>>> Maybe disabling is already possible by customizing
>>> 'minibuffer-lazy-count-format' to nil? Then the checks for
>>> non-nil 'minibuffer-lazy-count-format' could be added to
>>> more places, such as to wrap the whole '(condition-case error'
>>> in query-replace-read-args with the 'when' condition, etc.
>>
>> Yes, the user can set minibuffer-lazy-count-format to nil to get rid of
>> the lazy count.
>>
>> Concerning query-replace, why would anyone want to have lazy highlight
>> during the perform-replace loop, but not earlier? I'm not a fan of
>> adding a custom option here, not because it would be hard, but because
>> it seems totally unnecessary.
>
> Maybe a new option would make sense for the same reason why there is
> the option isearch-lazy-count?
Okay, I'm not against this, but let's think about the names of these user
options. The existing option is named query-replace-lazy-highlight,
which seems to exactly describe the new feature. The existing feature
would more specifically be called perform-replace-lazy highlight.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-16 19:02 ` Juri Linkov
@ 2022-03-16 20:25 ` Augusto Stoffel
2022-03-17 17:05 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-16 20:25 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Wed, 16 Mar 2022 at 21:02, Juri Linkov <juri@linkov.net> wrote:
>>>> @@ -2350,7 +2352,9 @@ isearch-query-replace
>>>> + (let ((lazy-highlight-cleanup (and lazy-highlight-cleanup
>>>> + (not query-replace-lazy-highlight))))
>>>> + (isearch-done nil t))
>>>
>>> Is this some optimization? It seems it's intended to leave
>>> some existing highlighting? Is this to avoid double highlighting?
>>
>> The weird let-binding for lazy-highlight-cleanup is indeed an
>> optimization. Without it, you can see the lazy highlights briefly flash
>> off an on again after you hit RET to confirm the replacement string.
>>
>> (Same remark explains why condition-case is used instead of a
>> unwind-protect in query-replace-read-args).
>
> When I tried your latest patch, it still flashes when it starts
> the perform-replace loop.
Does it always happen for you? I see this occasionally, and as far as I
can tell it's random.
>>> The problem is that when these conditions 'isearch-mode (null isearch-message-function)'
>>> are removed, now this shows wrong counts in the minibuffer history search
>>> (e.g. 'M-! C-r s C-r C-r ...') and the shell history search
>>> (e.g. 'M-x shell RET M-r s C-r C-r ...'). Before this change
>>> counting was disabled in the history search because it shows wrong numbers.
>>
>> Okay, so this means we should bind isearch-lazy-count to nil in these
>> commands, do you agree? It has always looked like a hack to me to check
>> for (null isearch-message-function).
>
> Binding isearch-lazy-count to nil could serve as a temporary measure
> until lazy-count will be supported in the history search.
Okay, the patch from yesterday already does that.
>>> For some reasons the package lisp/mb-depth.el uses 'after-string'
>>> instead of 'before-string', and (make-overlay (point-min) (1+ (point-min)))
>>> instead of (make-overlay (point-min) (point-min)),
>>> so maybe better to do the same?
>>
>> Okay, but do you know the reasons? I've changed to before-string, but I
>> don't like to make the overlay 1 char longer than it has to be :-P
>
> I don't know the reason. Since it works in your patch, then it's ok.
Great.
>>> Highlighting is still incorrect for word replacement ('C-u M-%')
>>> and for non-nil 'replace-char-fold'. To handle these cases correctly,
>>> 'replace-highlight' uses:
>>>
>>> (isearch-regexp-function (or replace-regexp-function
>>> delimited-flag
>>> (and replace-char-fold
>>> (not regexp-flag)
>>> #'char-fold-to-regexp)))
>>
>> Okay, fixed this. (BTW, where is replace-regexp-function used? It's
>> not set anywhere in Emacs, and it's not a defcustom either.)
>
> 'replace-regexp-function' was added recently in bug#52558
> to allow implementing more regexp types in bug#54017.
Ah, I see. I must say, in isearch.el I don't like this trichotomy of
the regexp/literal/with isearch-regexp-function cases. Ideally, the
regexp and literal search cases would be handled by
isearch-regexp-function set to identity respectively regexp-quote. But
it would be a large refactoring, perhaps with only aesthetic benefits.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-16 20:25 ` Augusto Stoffel
@ 2022-03-17 17:05 ` Juri Linkov
2022-03-17 19:06 ` Augusto Stoffel
2022-03-17 19:45 ` Augusto Stoffel
0 siblings, 2 replies; 57+ messages in thread
From: Juri Linkov @ 2022-03-17 17:05 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> When I tried your latest patch, it still flashes when it starts
>> the perform-replace loop.
>
> Does it always happen for you? I see this occasionally, and as far as I
> can tell it's random.
It happens every time when starting perform-replace from isearch.
>>>> Highlighting is still incorrect for word replacement ('C-u M-%')
>>>> and for non-nil 'replace-char-fold'. To handle these cases correctly,
>>>> 'replace-highlight' uses:
>>>>
>>>> (isearch-regexp-function (or replace-regexp-function
>>>> delimited-flag
>>>> (and replace-char-fold
>>>> (not regexp-flag)
>>>> #'char-fold-to-regexp)))
>>>
>>> Okay, fixed this. (BTW, where is replace-regexp-function used? It's
>>> not set anywhere in Emacs, and it's not a defcustom either.)
>>
>> 'replace-regexp-function' was added recently in bug#52558
>> to allow implementing more regexp types in bug#54017.
>
> Ah, I see. I must say, in isearch.el I don't like this trichotomy of
> the regexp/literal/with isearch-regexp-function cases. Ideally, the
> regexp and literal search cases would be handled by
> isearch-regexp-function set to identity respectively regexp-quote. But
> it would be a large refactoring, perhaps with only aesthetic benefits.
Yep, too much changes for little benefit.
The biggest hunk in your patch are changes in query-replace-read-args
starting from 'condition-case error'. Would it be possible to simplify
this part? Maybe by some refactoring?
OTOH, your changes that add lazy-count-update-hook and remove
'(null isearch-message-function)' can be already pushed.
Could you please send a separate patch for pushing with these changes only?
Then the patch with minibuffer-lazy-count feature will be shorter.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-16 20:09 ` Augusto Stoffel
@ 2022-03-17 17:09 ` Juri Linkov
2022-03-17 19:10 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-17 17:09 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> My idea is:
>
> - The users of the feature are Elisp programmers / package authors.
> - I don't think end users can meaningfully do anything directly with
> this new minibuffer hook.
> - If package X wants to take advantage of the feature, then it will
> either add minibuffer-lazy-highlight-setup to the
> minibuffer-setup-hook unconditionally, or it will define an
> X-lazy-highlight customization option to control this.
>
> So I think the conclusion is that the current approach in my patch is an
> good way to proceed here?
So do you think end users should not be able to decide where they want
to use this feature? For example, if one user will want it for 'occur',
but another user doesn't want it in the 'occur' regexp-reading minibuffer,
they should have no choice?
>>>> BTW, what is the relation between the minibuffer-lazy-highlight feature
>>>> and another proposed feature that immediately updates the search in
>>>> the buffer while editing the string in the minibuffer by isearch-edit-string?
>>>> Can minibuffer-lazy-highlight be considered as a lightweight version of
>>>> the buffer search from the minibuffer?
>>>
>>> Well, there's a package for that on ELPA (isearch-mb), so extending
>>> isearch-edit-string to do that seems superfluous now?
>>
>> It's still possible to add this feature to isearch-edit-string,
>> when the change would not be too enormous. I recall squeezing
>> it into a small patch, but unfortunately it requires changes
>> in keymap priorities.
>
> I would suggest taking a look at isearch-mb. I think the code is pretty
> tight, and I would be unable to shorten the implementation other than by
> deleting comment :-)
I already looked at isearch-mb. Adding the same to isearch.el
will take only 52 lines.
>>>>> There are a few more we could add (perhaps later),
>>>>> such as `occur' and `keep-lines'.
>>>>
>>>> I tried (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
>>>> in the minibuffer of 'occur' and others, and it works nicely.
>>>> Maybe it could even semi-deprecate the package re-builder.el.
>>>>
>>>> Thanks for this generally usable feature.
>>>
>>> By the way, this is a byproduct of that long discussion that led to
>>> isearch-mb, so it was not all in vain :-).
>>
>> Are you sure these features can't be combined? One feature basically
>> runs isearch-search-and-update in the buffer from the minibuffer,
>> and this feature runs isearch-lazy-highlight-new-loop.
>
> For one thing, isearch-mode has 2 essential commands (repeat forward and
> backwards), a couple more necessary ones (quit, abort, scroll,
> beginning/end of buffer, mode toggles), and then a number of commands
> that end the search with a special action (query-replace, etc.).
>
> These little details add up to the 283 lines in isearch-mb.el currently
> has.
I wonder how this is affected by scroll, beginning/end of buffer, mode toggles?
These commands don't use the minibuffer.
>>>>> - There's no customization variable to enable the minibuffer lazy
>>>>> highlight. The rationale is that each command that will use it should
>>>>> define its own user option (or use an existing one). For
>>>>> `isearch-edit-string' it's `isearch-lazy-highlight'; for
>>>>> `query-replace' it's `query-replace-lazy-highlight'; and so on.
>>>>
>>>> A common customizable option to enable this everywhere would be nice too.
>>>> Maybe disabling is already possible by customizing
>>>> 'minibuffer-lazy-count-format' to nil? Then the checks for
>>>> non-nil 'minibuffer-lazy-count-format' could be added to
>>>> more places, such as to wrap the whole '(condition-case error'
>>>> in query-replace-read-args with the 'when' condition, etc.
>>>
>>> Yes, the user can set minibuffer-lazy-count-format to nil to get rid of
>>> the lazy count.
>>>
>>> Concerning query-replace, why would anyone want to have lazy highlight
>>> during the perform-replace loop, but not earlier? I'm not a fan of
>>> adding a custom option here, not because it would be hard, but because
>>> it seems totally unnecessary.
>>
>> Maybe a new option would make sense for the same reason why there is
>> the option isearch-lazy-count?
>
> Okay, I'm not against this, but let's think about the names of these user
> options. The existing option is named query-replace-lazy-highlight,
> which seems to exactly describe the new feature. The existing feature
> would more specifically be called perform-replace-lazy highlight.
Do you mean lazy-highlight in the minibuffer that reads a string to replace?
Then it could be named query-replace-read-lazy-highlight.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-17 17:05 ` Juri Linkov
@ 2022-03-17 19:06 ` Augusto Stoffel
2022-03-20 19:24 ` Juri Linkov
2022-03-17 19:45 ` Augusto Stoffel
1 sibling, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-17 19:06 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 355 bytes --]
On Thu, 17 Mar 2022 at 19:05, Juri Linkov <juri@linkov.net> wrote:
> OTOH, your changes that add lazy-count-update-hook and remove
> '(null isearch-message-function)' can be already pushed.
> Could you please send a separate patch for pushing with these changes only?
>
> Then the patch with minibuffer-lazy-count feature will be shorter.
There it is:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-lazy-highlight-and-match-count-while-reading-f.patch --]
[-- Type: text/x-patch, Size: 9413 bytes --]
From 3de37cfec647cedd69032df072a1176970224ecb Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Thu, 17 Mar 2022 20:02:13 +0100
Subject: [PATCH] Allow lazy highlight and match count while reading from
minibuffer
* lisp/isearch.el (lazy-count-update-hook): New hook allowing to
display the lazy count in special ways.
(isearch-edit-string): Add lazy highlight and count of matching text.
(isearch-lazy-highlight-new-loop,
isearch-lazy-highlight-buffer-update): Use
`isearch-lazy-count-display-function' instead of hardcoded call to
`isearch-message'.
(minibuffer-lazy-count-format, minibuffer-lazy-highlight-transform,
minibuffer-lazy-highlight--overlay, minibuffer-lazy-highlight--count,
minibuffer-lazy-highlight--after-change,
minibuffer-lazy-highlight--exit, minibuffer-lazy-highlight-setup):
Variables and functions implementing the lazy highlight functionality
while reading from minibuffer.
* lisp/comint.el (comint-history-isearch-setup,
comint-history-isearch-end): Make sure no lazy count is displayed.
* lisp/simple.el (minibuffer-history-isearch-setup): Make sure no lazy
count is displayed.
---
lisp/comint.el | 2 ++
lisp/isearch.el | 86 +++++++++++++++++++++++++++++++++++++++++++++----
lisp/simple.el | 1 +
3 files changed, 83 insertions(+), 6 deletions(-)
diff --git a/lisp/comint.el b/lisp/comint.el
index 4c82e74e4b..56082f622a 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1515,6 +1515,7 @@ comint-history-isearch-setup
#'comint-history-isearch-wrap)
(setq-local isearch-push-state-function
#'comint-history-isearch-push-state)
+ (setq-local isearch-lazy-count nil)
(add-hook 'isearch-mode-end-hook 'comint-history-isearch-end nil t)))
(defun comint-history-isearch-end ()
@@ -1526,6 +1527,7 @@ comint-history-isearch-end
(setq isearch-message-function nil)
(setq isearch-wrap-function nil)
(setq isearch-push-state-function nil)
+ (kill-local-variable 'isearch-lazy-count)
(remove-hook 'isearch-mode-end-hook 'comint-history-isearch-end t)
(unless isearch-suspended
(custom-reevaluate-setting 'comint-history-isearch)))
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 8970216398..1a83586ef8 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1812,6 +1812,8 @@ isearch-edit-string
(minibuffer-history-symbol)
;; Search string might have meta information on text properties.
(minibuffer-allow-text-properties t))
+ (when isearch-lazy-highlight
+ (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup))
(setq isearch-new-string
(read-from-minibuffer
(isearch-message-prefix nil isearch-nonincremental)
@@ -3990,6 +3992,8 @@ isearch-lazy-highlight-error
(defvar isearch-lazy-count-current nil)
(defvar isearch-lazy-count-total nil)
(defvar isearch-lazy-count-hash (make-hash-table))
+(defvar lazy-count-update-hook nil
+ "Hook run after new lazy count results are computed.")
(defun lazy-highlight-cleanup (&optional force procrastinate)
"Stop lazy highlighting and remove extra highlighting from current buffer.
@@ -4048,7 +4052,7 @@ isearch-lazy-highlight-new-loop
isearch-lazy-highlight-window-end))))))
;; something important did indeed change
(lazy-highlight-cleanup t (not (equal isearch-string ""))) ;stop old timer
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(when (or (equal isearch-string "")
;; Check if this place was reached by a condition above
;; other than changed window boundaries (that shouldn't
@@ -4067,7 +4071,10 @@ isearch-lazy-highlight-new-loop
(setq isearch-lazy-count-current nil
isearch-lazy-count-total nil)
;; Delay updating the message if possible, to avoid flicker
- (when (string-equal isearch-string "") (isearch-message))))
+ (when (string-equal isearch-string "")
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))))
(setq isearch-lazy-highlight-window-start-changed nil)
(setq isearch-lazy-highlight-window-end-changed nil)
(setq isearch-lazy-highlight-error isearch-error)
@@ -4120,13 +4127,15 @@ isearch-lazy-highlight-new-loop
'isearch-lazy-highlight-start))))
;; Update the current match number only in isearch-mode and
;; unless isearch-mode is used specially with isearch-message-function
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
;; Update isearch-lazy-count-current only when it was already set
;; at the end of isearch-lazy-highlight-buffer-update
(when isearch-lazy-count-current
(setq isearch-lazy-count-current
(gethash (point) isearch-lazy-count-hash 0))
- (isearch-message))))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))))
(defun isearch-lazy-highlight-search (string bound)
"Search ahead for the next or previous match, for lazy highlighting.
@@ -4327,16 +4336,81 @@ isearch-lazy-highlight-buffer-update
(setq looping nil
nomore t))))
(if nomore
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(unless isearch-lazy-count-total
(setq isearch-lazy-count-total 0))
(setq isearch-lazy-count-current
(gethash opoint isearch-lazy-count-hash 0))
- (isearch-message))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))
(setq isearch-lazy-highlight-timer
(run-at-time lazy-highlight-interval nil
'isearch-lazy-highlight-buffer-update)))))))))
+\f
+;; Reading from minibuffer with lazy highlight and match count
+
+(defcustom minibuffer-lazy-count-format "%s "
+ "Format of the total number of matches for the prompt prefix."
+ :type '(choice (const :tag "Don't display a count" nil)
+ (string :tag "Display match count" "%s "))
+ :group 'lazy-count
+ :version "29.1")
+
+(defvar minibuffer-lazy-highlight-transform #'identity
+ "Function to transform minibuffer text into a `isearch-string' for highlighting.")
+
+(defvar minibuffer-lazy-highlight--overlay nil
+ "Overlay for minibuffer prompt updates.")
+
+(defun minibuffer-lazy-highlight--count ()
+ "Display total match count in the minibuffer prompt."
+ (when minibuffer-lazy-highlight--overlay
+ (overlay-put minibuffer-lazy-highlight--overlay
+ 'before-string
+ (and isearch-lazy-count-total
+ (not isearch-error)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)))))
+
+(defun minibuffer-lazy-highlight--after-change (_beg _end _len)
+ "Update lazy highlight state in minibuffer selected window."
+ (when isearch-lazy-highlight
+ (let ((inhibit-redisplay t) ;; Avoid cursor flickering
+ (string (minibuffer-contents)))
+ (with-minibuffer-selected-window
+ (setq isearch-string (funcall minibuffer-lazy-highlight-transform string))
+ (isearch-lazy-highlight-new-loop)))))
+
+(defun minibuffer-lazy-highlight--exit ()
+ "Unwind changes from `minibuffer-lazy-highlight-setup'."
+ (remove-hook 'after-change-functions
+ #'minibuffer-lazy-highlight--after-change)
+ (remove-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
+ (remove-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
+ (setq minibuffer-lazy-highlight--overlay nil)
+ (when lazy-highlight-cleanup
+ (lazy-highlight-cleanup)))
+
+(defun minibuffer-lazy-highlight-setup ()
+ "Set up minibuffer for lazy highlight of matches in the original window.
+
+This function is intended to be added to `minibuffer-setup-hook'.
+Note that several other isearch variables influence the lazy
+highlighting, including `isearch-regexp',
+`isearch-lazy-highlight' and `isearch-lazy-count'."
+ (remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
+ (add-hook 'after-change-functions
+ #'minibuffer-lazy-highlight--after-change)
+ (add-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
+ (add-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
+ (setq minibuffer-lazy-highlight--overlay
+ (and minibuffer-lazy-count-format
+ (make-overlay (point-min) (point-min) (current-buffer) t)))
+ (minibuffer-lazy-highlight--after-change nil nil nil))
+
+\f
(defun isearch-resume (string regexp word forward message case-fold)
"Resume an incremental search.
STRING is the string or regexp searched for.
diff --git a/lisp/simple.el b/lisp/simple.el
index accc119e2b..61319b6060 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2840,6 +2840,7 @@ minibuffer-history-isearch-setup
#'minibuffer-history-isearch-wrap)
(setq-local isearch-push-state-function
#'minibuffer-history-isearch-push-state)
+ (setq-local isearch-lazy-count nil)
(add-hook 'isearch-mode-end-hook 'minibuffer-history-isearch-end nil t))
(defun minibuffer-history-isearch-end ()
--
2.35.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-17 17:09 ` Juri Linkov
@ 2022-03-17 19:10 ` Augusto Stoffel
2022-03-17 20:40 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-17 19:10 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Thu, 17 Mar 2022 at 19:09, Juri Linkov <juri@linkov.net> wrote:
>> My idea is:
>>
>> - The users of the feature are Elisp programmers / package authors.
>> - I don't think end users can meaningfully do anything directly with
>> this new minibuffer hook.
>> - If package X wants to take advantage of the feature, then it will
>> either add minibuffer-lazy-highlight-setup to the
>> minibuffer-setup-hook unconditionally, or it will define an
>> X-lazy-highlight customization option to control this.
>>
>> So I think the conclusion is that the current approach in my patch is an
>> good way to proceed here?
>
> So do you think end users should not be able to decide where they want
> to use this feature? For example, if one user will want it for 'occur',
> but another user doesn't want it in the 'occur' regexp-reading minibuffer,
> they should have no choice?
Of course there should an option, maybe occur-lazy-highlight. This is
why isearch.el itself should _not_ contain a customization option called
`minibuffer-lazy-highlight': each use (or group of uses) of this
functionality should define their own customization option.
>>>>> BTW, what is the relation between the minibuffer-lazy-highlight feature
>>>>> and another proposed feature that immediately updates the search in
>>>>> the buffer while editing the string in the minibuffer by isearch-edit-string?
>>>>> Can minibuffer-lazy-highlight be considered as a lightweight version of
>>>>> the buffer search from the minibuffer?
>>>>
>>>> Well, there's a package for that on ELPA (isearch-mb), so extending
>>>> isearch-edit-string to do that seems superfluous now?
>>>
>>> It's still possible to add this feature to isearch-edit-string,
>>> when the change would not be too enormous. I recall squeezing
>>> it into a small patch, but unfortunately it requires changes
>>> in keymap priorities.
>>
>> I would suggest taking a look at isearch-mb. I think the code is pretty
>> tight, and I would be unable to shorten the implementation other than by
>> deleting comment :-)
>
> I already looked at isearch-mb. Adding the same to isearch.el
> will take only 52 lines.
Okay, I can give my opinion about these changes if you wish.
>>>>>> There are a few more we could add (perhaps later),
>>>>>> such as `occur' and `keep-lines'.
>>>>>
>>>>> I tried (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
>>>>> in the minibuffer of 'occur' and others, and it works nicely.
>>>>> Maybe it could even semi-deprecate the package re-builder.el.
>>>>>
>>>>> Thanks for this generally usable feature.
>>>>
>>>> By the way, this is a byproduct of that long discussion that led to
>>>> isearch-mb, so it was not all in vain :-).
>>>
>>> Are you sure these features can't be combined? One feature basically
>>> runs isearch-search-and-update in the buffer from the minibuffer,
>>> and this feature runs isearch-lazy-highlight-new-loop.
>>
>> For one thing, isearch-mode has 2 essential commands (repeat forward and
>> backwards), a couple more necessary ones (quit, abort, scroll,
>> beginning/end of buffer, mode toggles), and then a number of commands
>> that end the search with a special action (query-replace, etc.).
>>
>> These little details add up to the 283 lines in isearch-mb.el currently
>> has.
>
> I wonder how this is affected by scroll, beginning/end of buffer, mode toggles?
> These commands don't use the minibuffer.
I probably misunderstood you then. I thought you wanted to allow C-s
and C-r in isearch-edit-string to go back and forth in the search
buffer, but apparently this is not the case. (Then I need to see your
52-line patch to understand what exactly you mean...)
>>>>>> - There's no customization variable to enable the minibuffer lazy
>>>>>> highlight. The rationale is that each command that will use it should
>>>>>> define its own user option (or use an existing one). For
>>>>>> `isearch-edit-string' it's `isearch-lazy-highlight'; for
>>>>>> `query-replace' it's `query-replace-lazy-highlight'; and so on.
>>>>>
>>>>> A common customizable option to enable this everywhere would be nice too.
>>>>> Maybe disabling is already possible by customizing
>>>>> 'minibuffer-lazy-count-format' to nil? Then the checks for
>>>>> non-nil 'minibuffer-lazy-count-format' could be added to
>>>>> more places, such as to wrap the whole '(condition-case error'
>>>>> in query-replace-read-args with the 'when' condition, etc.
>>>>
>>>> Yes, the user can set minibuffer-lazy-count-format to nil to get rid of
>>>> the lazy count.
>>>>
>>>> Concerning query-replace, why would anyone want to have lazy highlight
>>>> during the perform-replace loop, but not earlier? I'm not a fan of
>>>> adding a custom option here, not because it would be hard, but because
>>>> it seems totally unnecessary.
>>>
>>> Maybe a new option would make sense for the same reason why there is
>>> the option isearch-lazy-count?
>>
>> Okay, I'm not against this, but let's think about the names of these user
>> options. The existing option is named query-replace-lazy-highlight,
>> which seems to exactly describe the new feature. The existing feature
>> would more specifically be called perform-replace-lazy highlight.
>
> Do you mean lazy-highlight in the minibuffer that reads a string to replace?
> Then it could be named query-replace-read-lazy-highlight.
I personally find it a bit unnecessary to have separate options for
query-replace-read-lazy-highlight and query-replace-lazy-highlight. Of
course it's nice to have fine-grained control, but at some point it just
becomes too difficult to configure things. But I don't mind adding it.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-17 17:05 ` Juri Linkov
2022-03-17 19:06 ` Augusto Stoffel
@ 2022-03-17 19:45 ` Augusto Stoffel
2022-03-17 20:43 ` Juri Linkov
1 sibling, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-17 19:45 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Thu, 17 Mar 2022 at 19:05, Juri Linkov <juri@linkov.net> wrote:
>>> When I tried your latest patch, it still flashes when it starts
>>> the perform-replace loop.
>>
>> Does it always happen for you? I see this occasionally, and as far as I
>> can tell it's random.
>
> It happens every time when starting perform-replace from isearch.
Interesting, I can't see thing flash at that particular place. But I've
replaced the unwind-protect by a more fine-grained condition-case. Let
me know if it helps.
> The biggest hunk in your patch are changes in query-replace-read-args
> starting from 'condition-case error'. Would it be possible to simplify
> this part? Maybe by some refactoring?
The only refactoring opportunity I see it to define
(defun replace-regexp-function (delimited-flag)
(or replace-regexp-function
delimited-flag
(and replace-char-fold
(not regexp-flag)
#'char-fold-to-regexp)))
which can be used in 3 places. Should I do that?
Otherwise, I dont' see much room for improvement. There are a lot of
moving parts there, so to me things already turned out surprisingly
short.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-17 19:10 ` Augusto Stoffel
@ 2022-03-17 20:40 ` Juri Linkov
2022-03-17 21:42 ` Augusto Stoffel
2022-03-20 9:38 ` Augusto Stoffel
0 siblings, 2 replies; 57+ messages in thread
From: Juri Linkov @ 2022-03-17 20:40 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>>> My idea is:
>>>
>>> - The users of the feature are Elisp programmers / package authors.
>>> - I don't think end users can meaningfully do anything directly with
>>> this new minibuffer hook.
>>> - If package X wants to take advantage of the feature, then it will
>>> either add minibuffer-lazy-highlight-setup to the
>>> minibuffer-setup-hook unconditionally, or it will define an
>>> X-lazy-highlight customization option to control this.
>>>
>>> So I think the conclusion is that the current approach in my patch is an
>>> good way to proceed here?
>>
>> So do you think end users should not be able to decide where they want
>> to use this feature? For example, if one user will want it for 'occur',
>> but another user doesn't want it in the 'occur' regexp-reading minibuffer,
>> they should have no choice?
>
> Of course there should an option, maybe occur-lazy-highlight. This is
> why isearch.el itself should _not_ contain a customization option called
> `minibuffer-lazy-highlight': each use (or group of uses) of this
> functionality should define their own customization option.
This means dozens of new options for every possible command that uses
the minibuffer: occur-lazy-highlight, keep-lines-lazy-highlight,
flush-lines-lazy-highlight, kill-matching-lines-lazy-highlight,
copy-matching-lines-lazy-highlight, how-many-lazy-highlight, ...
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-17 19:45 ` Augusto Stoffel
@ 2022-03-17 20:43 ` Juri Linkov
0 siblings, 0 replies; 57+ messages in thread
From: Juri Linkov @ 2022-03-17 20:43 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> On Thu, 17 Mar 2022 at 19:05, Juri Linkov <juri@linkov.net> wrote:
>
>>>> When I tried your latest patch, it still flashes when it starts
>>>> the perform-replace loop.
>>>
>>> Does it always happen for you? I see this occasionally, and as far as I
>>> can tell it's random.
>>
>> It happens every time when starting perform-replace from isearch.
>
> Interesting, I can't see thing flash at that particular place. But I've
> replaced the unwind-protect by a more fine-grained condition-case. Let
> me know if it helps.
>
>> The biggest hunk in your patch are changes in query-replace-read-args
>> starting from 'condition-case error'. Would it be possible to simplify
>> this part? Maybe by some refactoring?
>
> The only refactoring opportunity I see it to define
>
> (defun replace-regexp-function (delimited-flag)
> (or replace-regexp-function
> delimited-flag
> (and replace-char-fold
> (not regexp-flag)
> #'char-fold-to-regexp)))
>
> which can be used in 3 places. Should I do that?
>
> Otherwise, I dont' see much room for improvement. There are a lot of
> moving parts there, so to me things already turned out surprisingly
> short.
I meant a function that translates arguments of query-replace/perform-replace
to search parameters. They are currently used in replace-search and
replace-highlight. But maybe could be reused by query-replace-read-args?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-17 20:40 ` Juri Linkov
@ 2022-03-17 21:42 ` Augusto Stoffel
2022-03-20 9:38 ` Augusto Stoffel
1 sibling, 0 replies; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-17 21:42 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Thu, 17 Mar 2022 at 22:40, Juri Linkov <juri@linkov.net> wrote:
>> Of course there should an option, maybe occur-lazy-highlight. This is
>> why isearch.el itself should _not_ contain a customization option called
>> `minibuffer-lazy-highlight': each use (or group of uses) of this
>> functionality should define their own customization option.
>
> This means dozens of new options for every possible command that uses
> the minibuffer: occur-lazy-highlight, keep-lines-lazy-highlight,
> flush-lines-lazy-highlight, kill-matching-lines-lazy-highlight,
> copy-matching-lines-lazy-highlight, how-many-lazy-highlight, ...
I'd say one option that applies to this whole group of commands that
work on lines. If we can find a good name for this variable, it
probably means this a good idea :-)
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-17 20:40 ` Juri Linkov
2022-03-17 21:42 ` Augusto Stoffel
@ 2022-03-20 9:38 ` Augusto Stoffel
2022-03-20 18:51 ` Juri Linkov
1 sibling, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-20 9:38 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126, Dmitry Gutov
On Thu, 17 Mar 2022 at 22:40, Juri Linkov <juri@linkov.net> wrote:
> This means dozens of new options for every possible command that uses
> the minibuffer: occur-lazy-highlight, keep-lines-lazy-highlight,
> flush-lines-lazy-highlight, kill-matching-lines-lazy-highlight,
> copy-matching-lines-lazy-highlight, how-many-lazy-highlight, ...
I'm experimenting with adding lazy-highlight directly into
`read-regexp', controlled by a new option `read-regexp-lazy-highlight',
which, preferably, would be t by default. Thus, in particular, all the
above commands would get lazy-highlight by default.
At first this felt somewhat intrusive, and third-party code might
require adaptation. The advantage is that the said adaptation is very
easy. Namely, a package author would have three options:
- Do nothing. Then read-regexp will have lazy highlighting as dictated
by read-regexp-lazy-highlight.
- If lazy-highlighting makes no sense at all in a given context, then
let-bind read-regexp-lazy-highlight to nil.
- If customizability is desired, define `package-X-lazy-highlight' and
let-bind read-regexp-lazy-highlighting to that.
What do you think? (This is probably also the approach with the minimal
number of additional code/changed lines, which seems to be desirable.)
Dmitry -- I've CC'ed you because I noticed project.el makes a bunch of
calls to read-regexp, and also a call query-replace-read-args at one
point. To summarize the story here: would you like to have lazy
highlight and lazy count (in “anzu” style) while reading regexps and
query-replace arguments in package.el? How fine-grained would you like
the user options to be here, and what should the defaults be?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-20 9:38 ` Augusto Stoffel
@ 2022-03-20 18:51 ` Juri Linkov
2022-03-24 19:03 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-20 18:51 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126, Dmitry Gutov
>> This means dozens of new options for every possible command that uses
>> the minibuffer: occur-lazy-highlight, keep-lines-lazy-highlight,
>> flush-lines-lazy-highlight, kill-matching-lines-lazy-highlight,
>> copy-matching-lines-lazy-highlight, how-many-lazy-highlight, ...
>
> I'm experimenting with adding lazy-highlight directly into
> `read-regexp', controlled by a new option `read-regexp-lazy-highlight',
> which, preferably, would be t by default. Thus, in particular, all the
> above commands would get lazy-highlight by default.
>
> At first this felt somewhat intrusive, and third-party code might
> require adaptation. The advantage is that the said adaptation is very
> easy. Namely, a package author would have three options:
>
> - Do nothing. Then read-regexp will have lazy highlighting as dictated
> by read-regexp-lazy-highlight.
> - If lazy-highlighting makes no sense at all in a given context, then
> let-bind read-regexp-lazy-highlight to nil.
> - If customizability is desired, define `package-X-lazy-highlight' and
> let-bind read-regexp-lazy-highlighting to that.
>
> What do you think? (This is probably also the approach with the minimal
> number of additional code/changed lines, which seems to be desirable.)
Sorry, I have no idea who and how might want to use lazy-highlighting
in the minibuffer. I'd just provide a hook that any user can add
to the minibuffer-setup-hook, or any package author can add
to minibuffer-with-setup-hook. But in any case we need more opinions.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-17 19:06 ` Augusto Stoffel
@ 2022-03-20 19:24 ` Juri Linkov
2022-03-20 19:59 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-20 19:24 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> OTOH, your changes that add lazy-count-update-hook and remove
>> '(null isearch-message-function)' can be already pushed.
>> Could you please send a separate patch for pushing with these changes only?
>>
>> Then the patch with minibuffer-lazy-count feature will be shorter.
>
> There it is:
>
> From 3de37cfec647cedd69032df072a1176970224ecb Mon Sep 17 00:00:00 2001
> From: Augusto Stoffel <arstoffel@gmail.com>
> Date: Thu, 17 Mar 2022 20:02:13 +0100
> Subject: [PATCH] Allow lazy highlight and match count while reading from
> minibuffer
Could you please split it to more logically separate patches?
1. a patch that removes checks for 'isearch-mode (null isearch-message-function)'
and adds '(setq-local isearch-lazy-count nil)'.
It could also contain more changes in comint.el etc
from your earlier patch from 14 May 2021.
2. a patch that adds and runs 'lazy-count-update-hook'.
3. a patch with minibuffer-lazy-highlight feature in isearch.el.
Then what will remain to do is to decide how it would be better to activate
the minibuffer-lazy-highlight feature.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-20 19:24 ` Juri Linkov
@ 2022-03-20 19:59 ` Augusto Stoffel
2022-03-20 20:29 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-20 19:59 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]
On Sun, 20 Mar 2022 at 21:24, Juri Linkov <juri@linkov.net> wrote:
> Could you please split it to more logically separate patches?
>
> 1. a patch that removes checks for 'isearch-mode (null isearch-message-function)'
> and adds '(setq-local isearch-lazy-count nil)'.
> It could also contain more changes in comint.el etc
> from your earlier patch from 14 May 2021.
>
> 2. a patch that adds and runs 'lazy-count-update-hook'.
>
> 3. a patch with minibuffer-lazy-highlight feature in isearch.el.
>
> Then what will remain to do is to decide how it would be better to activate
> the minibuffer-lazy-highlight feature.
Okay. Points 1 and 2 are hard to disentangle, and 1 strictly is only
necessary because of 2. Also, separating things further risks
introducing bugs in code that has been tested quite a bit by now.
So my patch 00001 is for your points 1 and 2, patch 0002 is for point 3,
and patch 0003 adds lazy highlight to isearch-edit-string, where I guess
there is no controversy about what the configuration variables should
be.
Feel free to copy-edit my patches before merging if desired.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Add-lazy-highlight-to-isearch-edit-string.patch --]
[-- Type: text/x-patch, Size: 995 bytes --]
From e5b00cd015043eb7c3eca564994babfa2c2f8e0e Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sun, 20 Mar 2022 20:49:32 +0100
Subject: [PATCH 3/3] Add lazy highlight to 'isearch-edit-string'
* lisp/isearch.el (isearch-edit-string): Activate lazy highlight and
lazy count, provided 'isearch-lazy-highlight' respectively
'isearch-lazy-count' are non-nil.
---
lisp/isearch.el | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 1ee5f2e9a8..1a83586ef8 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1812,6 +1812,8 @@ isearch-edit-string
(minibuffer-history-symbol)
;; Search string might have meta information on text properties.
(minibuffer-allow-text-properties t))
+ (when isearch-lazy-highlight
+ (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup))
(setq isearch-new-string
(read-from-minibuffer
(isearch-message-prefix nil isearch-nonincremental)
--
2.35.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Allow-lazy-highlight-and-match-count-while-reading-f.patch --]
[-- Type: text/x-patch, Size: 4116 bytes --]
From 10290ac4e7f816ab33edad896448ac7d26951225 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sun, 20 Mar 2022 20:46:31 +0100
Subject: [PATCH 2/3] Allow lazy highlight and match count while reading from
minibuffer
* lisp/isearch.el (minibuffer-lazy-highlight-setup): New function, can
be added to 'minibuffer-setup-hook' to enable lazy highlight and count
while reading from minibuffer.
(minibuffer-lazy-count-format, minibuffer-lazy-highlight-transform,
minibuffer-lazy-highlight--overlay, minibuffer-lazy-highlight--count,
minibuffer-lazy-highlight--after-change,
minibuffer-lazy-highlight--exit): Auxiliary variables and functions
implementing the lazy highlight functionality while reading from
minibuffer.
---
lisp/isearch.el | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index b1951a8659..1ee5f2e9a8 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -4346,6 +4346,69 @@ isearch-lazy-highlight-buffer-update
(run-at-time lazy-highlight-interval nil
'isearch-lazy-highlight-buffer-update)))))))))
+\f
+;; Reading from minibuffer with lazy highlight and match count
+
+(defcustom minibuffer-lazy-count-format "%s "
+ "Format of the total number of matches for the prompt prefix."
+ :type '(choice (const :tag "Don't display a count" nil)
+ (string :tag "Display match count" "%s "))
+ :group 'lazy-count
+ :version "29.1")
+
+(defvar minibuffer-lazy-highlight-transform #'identity
+ "Function to transform minibuffer text into a `isearch-string' for highlighting.")
+
+(defvar minibuffer-lazy-highlight--overlay nil
+ "Overlay for minibuffer prompt updates.")
+
+(defun minibuffer-lazy-highlight--count ()
+ "Display total match count in the minibuffer prompt."
+ (when minibuffer-lazy-highlight--overlay
+ (overlay-put minibuffer-lazy-highlight--overlay
+ 'before-string
+ (and isearch-lazy-count-total
+ (not isearch-error)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)))))
+
+(defun minibuffer-lazy-highlight--after-change (_beg _end _len)
+ "Update lazy highlight state in minibuffer selected window."
+ (when isearch-lazy-highlight
+ (let ((inhibit-redisplay t) ;; Avoid cursor flickering
+ (string (minibuffer-contents)))
+ (with-minibuffer-selected-window
+ (setq isearch-string (funcall minibuffer-lazy-highlight-transform string))
+ (isearch-lazy-highlight-new-loop)))))
+
+(defun minibuffer-lazy-highlight--exit ()
+ "Unwind changes from `minibuffer-lazy-highlight-setup'."
+ (remove-hook 'after-change-functions
+ #'minibuffer-lazy-highlight--after-change)
+ (remove-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
+ (remove-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
+ (setq minibuffer-lazy-highlight--overlay nil)
+ (when lazy-highlight-cleanup
+ (lazy-highlight-cleanup)))
+
+(defun minibuffer-lazy-highlight-setup ()
+ "Set up minibuffer for lazy highlight of matches in the original window.
+
+This function is intended to be added to `minibuffer-setup-hook'.
+Note that several other isearch variables influence the lazy
+highlighting, including `isearch-regexp',
+`isearch-lazy-highlight' and `isearch-lazy-count'."
+ (remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
+ (add-hook 'after-change-functions
+ #'minibuffer-lazy-highlight--after-change)
+ (add-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
+ (add-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
+ (setq minibuffer-lazy-highlight--overlay
+ (and minibuffer-lazy-count-format
+ (make-overlay (point-min) (point-min) (current-buffer) t)))
+ (minibuffer-lazy-highlight--after-change nil nil nil))
+
+\f
(defun isearch-resume (string regexp word forward message case-fold)
"Resume an incremental search.
STRING is the string or regexp searched for.
--
2.35.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0001-New-hook-lazy-count-update-hook.patch --]
[-- Type: text/x-patch, Size: 5520 bytes --]
From 3eb2ec884310fc86bde1d9a897a266e5011fe9ec Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sun, 20 Mar 2022 20:43:10 +0100
Subject: [PATCH 1/3] New hook, lazy-count-update-hook
* lisp/isearch.el (lazy-count-update-hook): New hook allowing to
display the lazy count in special ways.
(isearch-lazy-highlight-new-loop,
isearch-lazy-highlight-buffer-update): Run `lazy-count-update-hook' at
appropriate times.
* lisp/comint.el (comint-history-isearch-setup,
comint-history-isearch-end): Make sure no lazy count is displayed.
* lisp/simple.el (minibuffer-history-isearch-setup): Make sure no lazy
count is displayed.
---
lisp/comint.el | 2 ++
lisp/isearch.el | 21 +++++++++++++++------
lisp/simple.el | 1 +
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/lisp/comint.el b/lisp/comint.el
index 4c82e74e4b..56082f622a 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1515,6 +1515,7 @@ comint-history-isearch-setup
#'comint-history-isearch-wrap)
(setq-local isearch-push-state-function
#'comint-history-isearch-push-state)
+ (setq-local isearch-lazy-count nil)
(add-hook 'isearch-mode-end-hook 'comint-history-isearch-end nil t)))
(defun comint-history-isearch-end ()
@@ -1526,6 +1527,7 @@ comint-history-isearch-end
(setq isearch-message-function nil)
(setq isearch-wrap-function nil)
(setq isearch-push-state-function nil)
+ (kill-local-variable 'isearch-lazy-count)
(remove-hook 'isearch-mode-end-hook 'comint-history-isearch-end t)
(unless isearch-suspended
(custom-reevaluate-setting 'comint-history-isearch)))
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 8970216398..b1951a8659 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3990,6 +3990,8 @@ isearch-lazy-highlight-error
(defvar isearch-lazy-count-current nil)
(defvar isearch-lazy-count-total nil)
(defvar isearch-lazy-count-hash (make-hash-table))
+(defvar lazy-count-update-hook nil
+ "Hook run after new lazy count results are computed.")
(defun lazy-highlight-cleanup (&optional force procrastinate)
"Stop lazy highlighting and remove extra highlighting from current buffer.
@@ -4048,7 +4050,7 @@ isearch-lazy-highlight-new-loop
isearch-lazy-highlight-window-end))))))
;; something important did indeed change
(lazy-highlight-cleanup t (not (equal isearch-string ""))) ;stop old timer
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(when (or (equal isearch-string "")
;; Check if this place was reached by a condition above
;; other than changed window boundaries (that shouldn't
@@ -4067,7 +4069,10 @@ isearch-lazy-highlight-new-loop
(setq isearch-lazy-count-current nil
isearch-lazy-count-total nil)
;; Delay updating the message if possible, to avoid flicker
- (when (string-equal isearch-string "") (isearch-message))))
+ (when (string-equal isearch-string "")
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))))
(setq isearch-lazy-highlight-window-start-changed nil)
(setq isearch-lazy-highlight-window-end-changed nil)
(setq isearch-lazy-highlight-error isearch-error)
@@ -4120,13 +4125,15 @@ isearch-lazy-highlight-new-loop
'isearch-lazy-highlight-start))))
;; Update the current match number only in isearch-mode and
;; unless isearch-mode is used specially with isearch-message-function
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
;; Update isearch-lazy-count-current only when it was already set
;; at the end of isearch-lazy-highlight-buffer-update
(when isearch-lazy-count-current
(setq isearch-lazy-count-current
(gethash (point) isearch-lazy-count-hash 0))
- (isearch-message))))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))))
(defun isearch-lazy-highlight-search (string bound)
"Search ahead for the next or previous match, for lazy highlighting.
@@ -4327,12 +4334,14 @@ isearch-lazy-highlight-buffer-update
(setq looping nil
nomore t))))
(if nomore
- (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
+ (when isearch-lazy-count
(unless isearch-lazy-count-total
(setq isearch-lazy-count-total 0))
(setq isearch-lazy-count-current
(gethash opoint isearch-lazy-count-hash 0))
- (isearch-message))
+ (when (and isearch-mode (null isearch-message-function))
+ (isearch-message))
+ (run-hooks 'lazy-count-update-hook))
(setq isearch-lazy-highlight-timer
(run-at-time lazy-highlight-interval nil
'isearch-lazy-highlight-buffer-update)))))))))
diff --git a/lisp/simple.el b/lisp/simple.el
index accc119e2b..61319b6060 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2840,6 +2840,7 @@ minibuffer-history-isearch-setup
#'minibuffer-history-isearch-wrap)
(setq-local isearch-push-state-function
#'minibuffer-history-isearch-push-state)
+ (setq-local isearch-lazy-count nil)
(add-hook 'isearch-mode-end-hook 'minibuffer-history-isearch-end nil t))
(defun minibuffer-history-isearch-end ()
--
2.35.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-20 19:59 ` Augusto Stoffel
@ 2022-03-20 20:29 ` Juri Linkov
2022-03-20 20:56 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-20 20:29 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> Could you please split it to more logically separate patches?
>>
>> 1. a patch that removes checks for 'isearch-mode (null isearch-message-function)'
>> and adds '(setq-local isearch-lazy-count nil)'.
>> It could also contain more changes in comint.el etc
>> from your earlier patch from 14 May 2021.
>>
>> 2. a patch that adds and runs 'lazy-count-update-hook'.
>>
>> 3. a patch with minibuffer-lazy-highlight feature in isearch.el.
>>
>> Then what will remain to do is to decide how it would be better to activate
>> the minibuffer-lazy-highlight feature.
>
> Okay. Points 1 and 2 are hard to disentangle, and 1 strictly is only
> necessary because of 2. Also, separating things further risks
> introducing bugs in code that has been tested quite a bit by now.
>
> So my patch 00001 is for your points 1 and 2, patch 0002 is for point 3,
> and patch 0003 adds lazy highlight to isearch-edit-string, where I guess
> there is no controversy about what the configuration variables should
> be.
Thanks, pushed now, with a NEWS entry.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-20 20:29 ` Juri Linkov
@ 2022-03-20 20:56 ` Augusto Stoffel
2022-03-23 18:20 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-20 20:56 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Sun, 20 Mar 2022 at 22:29, Juri Linkov <juri@linkov.net> wrote:
> Thanks, pushed now, with a NEWS entry.
Thanks!
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-20 20:56 ` Augusto Stoffel
@ 2022-03-23 18:20 ` Juri Linkov
2022-03-23 18:54 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-23 18:20 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> So my patch 00001 is for your points 1 and 2, patch 0002 is for point 3,
> and patch 0003 adds lazy highlight to isearch-edit-string, where I guess
> there is no controversy about what the configuration variables should
> be.
Unfortunately, there is a regression: isearch-yank-char-in-minibuffer
bound to C-f and [right] in the minibuffer of isearch-edit-string
doesn't work anymore.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-23 18:20 ` Juri Linkov
@ 2022-03-23 18:54 ` Augusto Stoffel
2022-03-23 19:17 ` Eli Zaretskii
2022-03-23 19:53 ` Juri Linkov
0 siblings, 2 replies; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-23 18:54 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
On Wed, 23 Mar 2022 at 20:20, Juri Linkov <juri@linkov.net> wrote:
> Unfortunately, there is a regression: isearch-yank-char-in-minibuffer
> bound to C-f and [right] in the minibuffer of isearch-edit-string
> doesn't work anymore.
The following patch solves to problem for me.
The point doesn't move visually, so selecting the isearch window puts it
in the right place. But what exactly happens to the point in the
searched buffer? I find this rather confusing!
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-regression-in-isearch-yank-char-in-minibuffer.patch --]
[-- Type: text/x-patch, Size: 924 bytes --]
From b9ed18bdfa72667b7d1e25982b7fd85dcb2c4166 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Wed, 23 Mar 2022 19:43:13 +0100
Subject: [PATCH] Fix regression in isearch-yank-char-in-minibuffer
* lisp/isearch.el (isearch-yank-char-in-minibuffer): Select the
original window in order to restore point. This is needed when
minibuffer lazy highlight is in effect.
---
lisp/isearch.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 1a83586ef8..9b311cb49e 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2670,7 +2670,7 @@ isearch-yank-char-in-minibuffer
(interactive "p")
(if (eobp)
(insert
- (with-current-buffer (cadr (buffer-list))
+ (with-minibuffer-selected-window
(buffer-substring-no-properties
(point) (progn (forward-char arg) (point)))))
(forward-char arg)))
--
2.35.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-23 18:54 ` Augusto Stoffel
@ 2022-03-23 19:17 ` Eli Zaretskii
2022-03-23 19:53 ` Juri Linkov
1 sibling, 0 replies; 57+ messages in thread
From: Eli Zaretskii @ 2022-03-23 19:17 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126, juri
> From: Augusto Stoffel <arstoffel@gmail.com>
> Date: Wed, 23 Mar 2022 19:54:14 +0100
> Cc: 53126@debbugs.gnu.org
>
> The point doesn't move visually, so selecting the isearch window puts it
> in the right place. But what exactly happens to the point in the
> searched buffer? I find this rather confusing!
Because of switch-to-buffer-preserve-window-point, perhaps?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-23 18:54 ` Augusto Stoffel
2022-03-23 19:17 ` Eli Zaretskii
@ 2022-03-23 19:53 ` Juri Linkov
2022-03-23 20:06 ` Juri Linkov
1 sibling, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-23 19:53 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> Unfortunately, there is a regression: isearch-yank-char-in-minibuffer
>> bound to C-f and [right] in the minibuffer of isearch-edit-string
>> doesn't work anymore.
>
> The following patch solves to problem for me.
>
> The point doesn't move visually, so selecting the isearch window puts it
> in the right place. But what exactly happens to the point in the
> searched buffer? I find this rather confusing!
>
> @@ -2670,7 +2670,7 @@ isearch-yank-char-in-minibuffer
> (interactive "p")
> (if (eobp)
> (insert
> - (with-current-buffer (cadr (buffer-list))
> + (with-minibuffer-selected-window
Thanks, I confirm your patch fixes the problem.
But before pushing it, I'd like to understand
how your previous patches changed the order
of the buffer list? Before your changes,
(cadr (buffer-list)) returned the original buffer,
but now it returns the minibuffer.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-23 19:53 ` Juri Linkov
@ 2022-03-23 20:06 ` Juri Linkov
2022-03-23 20:30 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-23 20:06 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> + (with-minibuffer-selected-window
>
> Thanks, I confirm your patch fixes the problem.
> But before pushing it, I'd like to understand
> how your previous patches changed the order
> of the buffer list? Before your changes,
> (cadr (buffer-list)) returned the original buffer,
> but now it returns the minibuffer.
Probably the buffer order is changed by
with-minibuffer-selected-window in
minibuffer-lazy-highlight--after-change.
This is fine, so I pushed your patch.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-23 20:06 ` Juri Linkov
@ 2022-03-23 20:30 ` Augusto Stoffel
2022-03-23 20:43 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-23 20:30 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Wed, 23 Mar 2022 at 22:06, Juri Linkov <juri@linkov.net> wrote:
>>> + (with-minibuffer-selected-window
>>
>> Thanks, I confirm your patch fixes the problem.
>> But before pushing it, I'd like to understand
>> how your previous patches changed the order
>> of the buffer list? Before your changes,
>> (cadr (buffer-list)) returned the original buffer,
>> but now it returns the minibuffer.
>
> Probably the buffer order is changed by
> with-minibuffer-selected-window in
> minibuffer-lazy-highlight--after-change.
> This is fine, so I pushed your patch.
FTR, I just tested and the culprit is not the
with-minibuffer-selected-window from
minibuffer-lazy-highlight--after-change (which makes sense, since that
macro uses NORECORD internally), but rather the calls to `select-window'
without NORECORD argument in isearch.el.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-23 20:30 ` Augusto Stoffel
@ 2022-03-23 20:43 ` Juri Linkov
0 siblings, 0 replies; 57+ messages in thread
From: Juri Linkov @ 2022-03-23 20:43 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> Probably the buffer order is changed by
>> with-minibuffer-selected-window in
>> minibuffer-lazy-highlight--after-change.
>> This is fine, so I pushed your patch.
>
> FTR, I just tested and the culprit is not the
> with-minibuffer-selected-window from
> minibuffer-lazy-highlight--after-change (which makes sense, since that
> macro uses NORECORD internally), but rather the calls to `select-window'
> without NORECORD argument in isearch.el.
I see that you use with-minibuffer-selected-window
to start a new loop isearch-lazy-highlight-new-loop,
but later every iteration of isearch-lazy-highlight-update
needs to select the window again. Then adding NORECORD argument
to `select-window' in lazy-highlight functions makes sense indeed.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-20 18:51 ` Juri Linkov
@ 2022-03-24 19:03 ` Augusto Stoffel
2022-03-25 8:39 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-24 19:03 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 932 bytes --]
On Sun, 20 Mar 2022 at 20:51, Juri Linkov <juri@linkov.net> wrote:
> Sorry, I have no idea who and how might want to use lazy-highlighting
> in the minibuffer. I'd just provide a hook that any user can add
> to the minibuffer-setup-hook, or any package author can add
> to minibuffer-with-setup-hook. But in any case we need more opinions.
All right, I think this brings us back to the original idea: we add lazy
highlight to query-replace now and decide later about the other commands
in replace.el.
I've attached an updated patch that applies over the already merged
changes.
Juri: you mentioned the idea of adding a new option
`query-replace-read-lazy-highlight'. This is easier to add then remove
in the future, so my suggestion would be to first wait and see if anyone
actually needs that option. For now lazy highlight when reading the
query-replace args is controlled by the good old
`query-replace-lazy-highlight'.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-lazy-highlight-and-match-count-in-query-repl.patch --]
[-- Type: text/x-patch, Size: 8983 bytes --]
From bc85df88bf3bee99997163b6233ff82445eac66b Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Thu, 17 Mar 2022 20:17:26 +0100
Subject: [PATCH] Display lazy highlight and match count in query-replace
* lisp/isearch.el (isearch-query-replace): Don't clean up lazy
highlight if applicable.
* lisp/replace.el (query-replace-read-args, query-replace-read-to):
Add lazy highlighting and count.
(replace--region-filter): New function, extracted from
'perform-replace'.
(perform-replace): Use 'replace--region-filter'.
---
lisp/isearch.el | 21 +++++++----
lisp/replace.el | 99 ++++++++++++++++++++++++++++++++++++-------------
2 files changed, 87 insertions(+), 33 deletions(-)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 9b311cb49e..e93e4d8b92 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2352,7 +2352,9 @@ isearch-query-replace
(isearch-recursive-edit nil)
(isearch-string-propertized
(isearch-string-propertize isearch-string)))
- (isearch-done nil t)
+ (let ((lazy-highlight-cleanup (and lazy-highlight-cleanup
+ (not query-replace-lazy-highlight))))
+ (isearch-done nil t))
(isearch-clean-overlays)
(if (and isearch-other-end
(if backward
@@ -2368,13 +2370,16 @@ isearch-query-replace
(symbol-value query-replace-from-history-variable)))
(perform-replace
isearch-string-propertized
- (query-replace-read-to
- isearch-string-propertized
- (concat "Query replace"
- (isearch--describe-regexp-mode (or delimited isearch-regexp-function) t)
- (if backward " backward" "")
- (if (use-region-p) " in region" ""))
- isearch-regexp)
+ (condition-case error
+ (query-replace-read-to
+ isearch-string-propertized
+ (concat "Query replace"
+ (isearch--describe-regexp-mode (or delimited isearch-regexp-function) t)
+ (if backward " backward" "")
+ (if (use-region-p) " in region" ""))
+ isearch-regexp)
+ (t (lazy-highlight-cleanup lazy-highlight-cleanup)
+ (signal (car error) (cdr error))))
t isearch-regexp (or delimited isearch-regexp-function) nil nil
(if (use-region-p) (region-beginning))
(if (use-region-p) (region-end))
diff --git a/lisp/replace.el b/lisp/replace.el
index 06be597855..a56e493d99 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -352,8 +352,15 @@ query-replace-read-to
(query-replace-compile-replacement
(save-excursion
(let* ((history-add-new-input nil)
+ (count (if (and query-replace-lazy-highlight
+ minibuffer-lazy-count-format
+ isearch-lazy-count
+ isearch-lazy-count-total)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)
+ ""))
(to (read-from-minibuffer
- (format "%s %s with: " prompt (query-replace-descr from))
+ (format "%s%s %s with: " count prompt (query-replace-descr from))
nil nil nil
query-replace-to-history-variable from t)))
(add-to-history query-replace-to-history-variable to nil t)
@@ -365,14 +372,49 @@ query-replace-read-args
(unless noerror
(barf-if-buffer-read-only))
(save-mark-and-excursion
- (let* ((from (query-replace-read-from prompt regexp-flag))
- (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
- (query-replace-read-to from prompt regexp-flag))))
- (list from to
- (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
- (and (plist-member (text-properties-at 0 from) 'isearch-regexp-function)
- (get-text-property 0 'isearch-regexp-function from)))
- (and current-prefix-arg (eq current-prefix-arg '-))))))
+ (condition-case error
+ (let (;; Variables controlling lazy highlighting while reading
+ ;; FROM and TO.
+ (isearch-case-fold-search case-fold-search)
+ (isearch-lazy-highlight query-replace-lazy-highlight)
+ (isearch-regexp regexp-flag)
+ (isearch-regexp-function (or replace-regexp-function
+ (and current-prefix-arg
+ (not (eq current-prefix-arg '-)))
+ (and replace-char-fold
+ (not regexp-flag)
+ #'char-fold-to-regexp)))
+ (lazy-highlight-cleanup nil)
+ (minibuffer-lazy-highlight-transform
+ (lambda (string)
+ (let* ((split (query-replace--split-string string))
+ (from-string (if (consp split) (car split) split)))
+ (when (and case-fold-search search-upper-case)
+ (setq isearch-case-fold-search
+ (isearch-no-upper-case-p from-string regexp-flag)))
+ from-string)))
+ from to)
+ (when query-replace-lazy-highlight
+ (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
+ (when (use-region-p)
+ (letrec ((region-filter (replace--region-filter
+ (funcall region-extract-function 'bounds)))
+ (cleanup (lambda ()
+ (remove-function isearch-filter-predicate region-filter)
+ (remove-hook 'minibuffer-exit-hook cleanup))))
+ (add-function :after-while isearch-filter-predicate region-filter)
+ (add-hook 'minibuffer-exit-hook cleanup))))
+ (setq from (query-replace-read-from prompt regexp-flag))
+ (setq to (if (consp from)
+ (prog1 (cdr from) (setq from (car from)))
+ (query-replace-read-to from prompt regexp-flag)))
+ (list from to
+ (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
+ (and (plist-member (text-properties-at 0 from) 'isearch-regexp-function)
+ (get-text-property 0 'isearch-regexp-function from)))
+ (and current-prefix-arg (eq current-prefix-arg '-))))
+ (t (lazy-highlight-cleanup)
+ (signal (car error) (cdr error))))))
(defun query-replace (from-string to-string &optional delimited start end backward region-noncontiguous-p)
"Replace some occurrences of FROM-STRING with TO-STRING.
@@ -2773,6 +2815,26 @@ replace--push-stack
,search-str ,next-replace)
,stack))
+(defun replace--region-filter (bounds)
+ "Return a function that decides if a region is inside BOUNDS.
+BOUNDS is a list of cons cells of the form (START . END). The
+returned function takes as argument two buffer positions, START
+and END."
+ (let ((region-bounds
+ (mapcar (lambda (position)
+ (cons (copy-marker (car position))
+ (copy-marker (cdr position))))
+ bounds)))
+ (lambda (start end)
+ (delq nil (mapcar
+ (lambda (bounds)
+ (and
+ (>= start (car bounds))
+ (<= start (cdr bounds))
+ (>= end (car bounds))
+ (<= end (cdr bounds))))
+ region-bounds)))))
+
(defun perform-replace (from-string replacements
query-flag regexp-flag delimited-flag
&optional repeat-count map start end backward region-noncontiguous-p)
@@ -2857,22 +2919,9 @@ perform-replace
;; Unless a single contiguous chunk is selected, operate on multiple chunks.
(when region-noncontiguous-p
- (let ((region-bounds
- (mapcar (lambda (position)
- (cons (copy-marker (car position))
- (copy-marker (cdr position))))
- (funcall region-extract-function 'bounds))))
- (setq region-filter
- (lambda (start end)
- (delq nil (mapcar
- (lambda (bounds)
- (and
- (>= start (car bounds))
- (<= start (cdr bounds))
- (>= end (car bounds))
- (<= end (cdr bounds))))
- region-bounds))))
- (add-function :after-while isearch-filter-predicate region-filter)))
+ (setq region-filter (replace--region-filter
+ (funcall region-extract-function 'bounds)))
+ (add-function :after-while isearch-filter-predicate region-filter))
;; If region is active, in Transient Mark mode, operate on region.
(if backward
--
2.35.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-24 19:03 ` Augusto Stoffel
@ 2022-03-25 8:39 ` Juri Linkov
2022-03-25 9:43 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-25 8:39 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> @@ -365,14 +372,49 @@ query-replace-read-args
> - (let* ((from (query-replace-read-from prompt regexp-flag))
> - (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
> - (query-replace-read-to from prompt regexp-flag))))
> - (list from to
> - (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
> - (and (plist-member (text-properties-at 0 from) 'isearch-regexp-function)
> - (get-text-property 0 'isearch-regexp-function from)))
> - (and current-prefix-arg (eq current-prefix-arg '-))))))
The name of the function is `query-replace-read-args'.
And now most of the function is dealing with highlighting:
> + (condition-case error
> + (let (;; Variables controlling lazy highlighting while reading
> + ;; FROM and TO.
> + (isearch-case-fold-search case-fold-search)
> + (isearch-lazy-highlight query-replace-lazy-highlight)
> + (isearch-regexp regexp-flag)
> + (isearch-regexp-function (or replace-regexp-function
> + (and current-prefix-arg
> + (not (eq current-prefix-arg '-)))
> + (and replace-char-fold
> + (not regexp-flag)
> + #'char-fold-to-regexp)))
> + (lazy-highlight-cleanup nil)
> + (minibuffer-lazy-highlight-transform
> + (lambda (string)
> + (let* ((split (query-replace--split-string string))
> + (from-string (if (consp split) (car split) split)))
> + (when (and case-fold-search search-upper-case)
> + (setq isearch-case-fold-search
> + (isearch-no-upper-case-p from-string regexp-flag)))
> + from-string)))
> + from to)
> + (when query-replace-lazy-highlight
> + (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
> + (when (use-region-p)
> + (letrec ((region-filter (replace--region-filter
> + (funcall region-extract-function 'bounds)))
> + (cleanup (lambda ()
> + (remove-function isearch-filter-predicate region-filter)
> + (remove-hook 'minibuffer-exit-hook cleanup))))
> + (add-function :after-while isearch-filter-predicate region-filter)
> + (add-hook 'minibuffer-exit-hook cleanup))))
> + (setq from (query-replace-read-from prompt regexp-flag))
> + (setq to (if (consp from)
> + (prog1 (cdr from) (setq from (car from)))
> + (query-replace-read-to from prompt regexp-flag)))
> + (list from to
> + (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
> + (and (plist-member (text-properties-at 0 from) 'isearch-regexp-function)
> + (get-text-property 0 'isearch-regexp-function from)))
> + (and current-prefix-arg (eq current-prefix-arg '-))))
> + (t (lazy-highlight-cleanup)
> + (signal (car error) (cdr error))))))
This highlighting needs to be refactored from `query-replace-read-args'
into a separate highlighting function.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-25 8:39 ` Juri Linkov
@ 2022-03-25 9:43 ` Augusto Stoffel
2022-03-27 7:46 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-03-25 9:43 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Fri, 25 Mar 2022 at 10:39, Juri Linkov <juri@linkov.net> wrote:
> The name of the function is `query-replace-read-args'.
> And now most of the function is dealing with highlighting:
The good thing is that it was a rather short function before, so now
it's still reasonably sized. Also, the lazy highlight for query-replace
is essentially taken care of in one single place.
> This highlighting needs to be refactored from `query-replace-read-args'
> into a separate highlighting function.
Okay, you've said something along these lines before but I don't
understand what you mean.
Lazy highlight is controlled by lots of variables. These variables need
to be set for the duration of the minibuffer session that reads the
replacement strings. If we move this to a separate function, we can't
use 'let' anymore, so we basically need to reinvent the dynamic scoping
feature of Elisp (i.e., save the current values, set the variables to
new values, then restore the old values in some hook). And an even ugly
workaround would be needed to replace the necessary condition-case. Or
do I misunderstand you altogether?
The above accounts for most of the additions to query-replace-read-args.
What could move elsewhere are the 10 lines starting from
(when query-replace-lazy-highlight
but this will not change the fact that most of query-replace-read-args
now takes care of setting up lazy highlight, so I don't see any real
benefits to this.
Or maybe the issue is with this line:
(add-hook 'minibuffer-exit-hook cleanup)
which adds a closure rather than a symbol to the hook? That I could be
changed. I think it would require to add a defvar just to store
'region-filter', among other things I don't like, but I can do it.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-25 9:43 ` Augusto Stoffel
@ 2022-03-27 7:46 ` Juri Linkov
2022-04-01 9:06 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-03-27 7:46 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> Lazy highlight is controlled by lots of variables. These variables need
> to be set for the duration of the minibuffer session that reads the
> replacement strings. If we move this to a separate function, we can't
> use 'let' anymore, so we basically need to reinvent the dynamic scoping
> feature of Elisp (i.e., save the current values, set the variables to
> new values, then restore the old values in some hook). And an even ugly
> workaround would be needed to replace the necessary condition-case. Or
> do I misunderstand you altogether?
Do you think a macro could help to make this function short again?
Then when you would want to add highlighting to the minibuffer
reading the TO part of replacement too or to other minibuffers,
it would be easy to just add a single line with that macro.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-03-27 7:46 ` Juri Linkov
@ 2022-04-01 9:06 ` Augusto Stoffel
2022-04-01 16:35 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-04-01 9:06 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Sun, 27 Mar 2022 at 10:46, Juri Linkov <juri@linkov.net> wrote:
>> Lazy highlight is controlled by lots of variables. These variables need
>> to be set for the duration of the minibuffer session that reads the
>> replacement strings. If we move this to a separate function, we can't
>> use 'let' anymore, so we basically need to reinvent the dynamic scoping
>> feature of Elisp (i.e., save the current values, set the variables to
>> new values, then restore the old values in some hook). And an even ugly
>> workaround would be needed to replace the necessary condition-case. Or
>> do I misunderstand you altogether?
>
> Do you think a macro could help to make this function short again?
That could be done, of course, but I'm pretty sure that this macro would
be used exactly once. I don't think we should have a general-purpose
macro to set up the lazy highlighting for arbitrary minibuffer commands,
because complex cases like query-replace will be rare, and in any case
we can't anticipate all special requirements.
Anyway, should I refactor my patch to use a macro?
Something else that could make 'query-replace-read-args' slightly
shorter is to define
(defun replace-regexp-function (delimited-flag)
(or replace-regexp-function
delimited-flag
(and replace-char-fold
(not regexp-flag)
#'char-fold-to-regexp)))
This function could be used in 3 different places in replace.el.
> Then when you would want to add highlighting to the minibuffer
> reading the TO part of replacement too or to other minibuffers,
> it would be easy to just add a single line with that macro.
Right, a preview of the replacement text will also need some extra work.
This will probably involve some extension to
'isearch-lazy-highlight-update', making it even more complicated...
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-01 9:06 ` Augusto Stoffel
@ 2022-04-01 16:35 ` Juri Linkov
2022-04-01 18:12 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-04-01 16:35 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>>> Lazy highlight is controlled by lots of variables. These variables need
>>> to be set for the duration of the minibuffer session that reads the
>>> replacement strings. If we move this to a separate function, we can't
>>> use 'let' anymore, so we basically need to reinvent the dynamic scoping
>>> feature of Elisp (i.e., save the current values, set the variables to
>>> new values, then restore the old values in some hook). And an even ugly
>>> workaround would be needed to replace the necessary condition-case. Or
>>> do I misunderstand you altogether?
>>
>> Do you think a macro could help to make this function short again?
>
> That could be done, of course, but I'm pretty sure that this macro would
> be used exactly once. I don't think we should have a general-purpose
> macro to set up the lazy highlighting for arbitrary minibuffer commands,
> because complex cases like query-replace will be rare,
Why do you think query-replace is more complex than other minibuffer commands?
Other minibuffer commands need to set all the same a dozen of variables that
control lazy highlighting. Otherwise, they will highlight some random matches
that happen to reuse values left from a previous isearch.
> and in any case we can't anticipate all special requirements.
To anticipate all requirements, I recommend to try adding this feature
at least to occur. Then we will see what code needs to be shared.
> Anyway, should I refactor my patch to use a macro?
As long as experimentation will prove at least occur can use this macro.
> Something else that could make 'query-replace-read-args' slightly
> shorter is to define
>
> (defun replace-regexp-function (delimited-flag)
> (or replace-regexp-function
> delimited-flag
> (and replace-char-fold
> (not regexp-flag)
> #'char-fold-to-regexp)))
>
> This function could be used in 3 different places in replace.el.
This would be nice, maybe to be used as an optional argument of the macro.
>> Then when you would want to add highlighting to the minibuffer
>> reading the TO part of replacement too or to other minibuffers,
>> it would be easy to just add a single line with that macro.
>
> Right, a preview of the replacement text will also need some extra work.
> This will probably involve some extension to
> 'isearch-lazy-highlight-update', making it even more complicated...
Why? Shouldn't it just highlight the same way the text
entered into the minibuffer that reads the replacement?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-01 16:35 ` Juri Linkov
@ 2022-04-01 18:12 ` Augusto Stoffel
2022-04-02 18:23 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-04-01 18:12 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 475 bytes --]
On Fri, 1 Apr 2022 at 19:35, Juri Linkov <juri@linkov.net> wrote:
> Why do you think query-replace is more complex than other minibuffer commands?
> Other minibuffer commands need to set all the same a dozen of variables that
> control lazy highlighting. Otherwise, they will highlight some random matches
> that happen to reuse values left from a previous isearch.
Without having tested, the lazy highlight feature for occur would only
require the addition of 4 lines:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: occur.patch --]
[-- Type: text/x-patch, Size: 714 bytes --]
diff --git a/lisp/replace.el b/lisp/replace.el
index a56e493d99..cd1bf9057f 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1741,7 +1741,11 @@ occur-excluded-properties
:version "22.1")
(defun occur-read-primary-args ()
- (let* ((perform-collect (consp current-prefix-arg))
+ (when isearch-lazy-highlight
+ (add-hook 'minibuffer-setup-hook 'minibuffer-lazy-highlight-setup))
+ (let* ((isearch-regexp t)
+ (isearch-case-fold-search case-fold-search)
+ (perform-collect (consp current-prefix-arg))
(regexp (read-regexp (if perform-collect
"Collect strings matching regexp"
"List lines matching regexp")
[-- Attachment #3: Type: text/plain, Size: 1040 bytes --]
So yes, I'd say query-replace is very likely to be most complex case
there will be. And it has several unique features that make it quite
different from other potential uses of lazy highlight:
- The splitting of the TO and FROM strings at "->".
- The value of case-fold-search can change on the fly
- We must clean up the highlight only if the user quits, to avoid
flickering at the beginning of the perform-replace stage.
Okay, as I write this I realize occur would require some special stuff
if the region is active. This indeed should be factored out. But
hopefully you will agree with the 3 points above :-).
>> Right, a preview of the replacement text will also need some extra work.
>> This will probably involve some extension to
>> 'isearch-lazy-highlight-update', making it even more complicated...
>
> Why? Shouldn't it just highlight the same way the text
> entered into the minibuffer that reads the replacement?
I'm referring to the anzu feature whereby the replacement text is shown
next to each match, like this:
[-- Attachment #4: anzu.png --]
[-- Type: image/png, Size: 69493 bytes --]
[-- Attachment #5: Type: text/plain, Size: 158 bytes --]
This will not be totally trivial to implement, right? And it will add
some extra stuff to query-replace-read-args that is very much
query-replace specific.
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-01 18:12 ` Augusto Stoffel
@ 2022-04-02 18:23 ` Juri Linkov
2022-04-03 8:32 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-04-02 18:23 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> Why do you think query-replace is more complex than other minibuffer commands?
>> Other minibuffer commands need to set all the same a dozen of variables that
>> control lazy highlighting. Otherwise, they will highlight some random matches
>> that happen to reuse values left from a previous isearch.
>
> Without having tested, the lazy highlight feature for occur would only
> require the addition of 4 lines:
I could test it, but I'm quite sure 4 lines would not be sufficient,
because lazy-highlight relies on much more isearch variables.
> So yes, I'd say query-replace is very likely to be most complex case
> there will be. And it has several unique features that make it quite
> different from other potential uses of lazy highlight:
>
> - The splitting of the TO and FROM strings at "->".
Agreed.
> - The value of case-fold-search can change on the fly
Please clarify when it can be changed on the fly.
> - We must clean up the highlight only if the user quits, to avoid
> flickering at the beginning of the perform-replace stage.
I don't know how important is to avoid flickering at the beginning,
when it flickers after every replacement anyway.
> Okay, as I write this I realize occur would require some special stuff
> if the region is active. This indeed should be factored out. But
> hopefully you will agree with the 3 points above :-).
I agreed on 1.5 points :-)
>>> Right, a preview of the replacement text will also need some extra work.
>>> This will probably involve some extension to
>>> 'isearch-lazy-highlight-update', making it even more complicated...
>>
>> Why? Shouldn't it just highlight the same way the text
>> entered into the minibuffer that reads the replacement?
>
> I'm referring to the anzu feature whereby the replacement text is shown
> next to each match, like this:
>
> This will not be totally trivial to implement, right? And it will add
> some extra stuff to query-replace-read-args that is very much
> query-replace specific.
I very much doubt in usefulness of such a feature. I thought it could
simply highlight the replacement text to show the places in the buffer
that already contain such replacement text.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-02 18:23 ` Juri Linkov
@ 2022-04-03 8:32 ` Augusto Stoffel
2022-04-03 17:06 ` Juri Linkov
2022-04-04 16:37 ` Juri Linkov
0 siblings, 2 replies; 57+ messages in thread
From: Augusto Stoffel @ 2022-04-03 8:32 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 275 bytes --]
I've attached a sketch of a macro to help activating the minibuffer lazy
highlight. It doesn't make query-replace-read-args exactly short, but
if you like this, I can prepare the patch.
Note that I haven't tested this code at all. It's for impressionistic
purposes only.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-lazy-highlight-and-match-count-in-query-repl.patch --]
[-- Type: text/x-patch, Size: 8664 bytes --]
From 92394967d27bb2750a88a19951182d1465b57040 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Thu, 17 Mar 2022 20:17:26 +0100
Subject: [PATCH] Display lazy highlight and match count in query-replace
* lisp/isearch.el (isearch-query-replace): Don't clean up lazy
highlight if applicable.
* lisp/replace.el (query-replace-read-args, query-replace-read-to):
Add lazy highlighting and count.
(replace--region-filter): New function, extracted from
'perform-replace'.
(perform-replace): Use 'replace--region-filter'.
---
lisp/isearch.el | 65 +++++++++++++++++++++++++++++++++++++++++++------
lisp/replace.el | 49 +++++++++++++++++++++++--------------
2 files changed, 88 insertions(+), 26 deletions(-)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 956b115ce4..b28a7b88a8 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2352,7 +2352,9 @@ isearch-query-replace
(isearch-recursive-edit nil)
(isearch-string-propertized
(isearch-string-propertize isearch-string)))
- (isearch-done nil t)
+ (let ((lazy-highlight-cleanup (and lazy-highlight-cleanup
+ (not query-replace-lazy-highlight))))
+ (isearch-done nil t))
(isearch-clean-overlays)
(if (and isearch-other-end
(if backward
@@ -2368,13 +2370,16 @@ isearch-query-replace
(symbol-value query-replace-from-history-variable)))
(perform-replace
isearch-string-propertized
- (query-replace-read-to
- isearch-string-propertized
- (concat "Query replace"
- (isearch--describe-regexp-mode (or delimited isearch-regexp-function) t)
- (if backward " backward" "")
- (if (use-region-p) " in region" ""))
- isearch-regexp)
+ (condition-case error
+ (query-replace-read-to
+ isearch-string-propertized
+ (concat "Query replace"
+ (isearch--describe-regexp-mode (or delimited isearch-regexp-function) t)
+ (if backward " backward" "")
+ (if (use-region-p) " in region" ""))
+ isearch-regexp)
+ (t (lazy-highlight-cleanup lazy-highlight-cleanup)
+ (signal (car error) (cdr error))))
t isearch-regexp (or delimited isearch-regexp-function) nil nil
(if (use-region-p) (region-beginning))
(if (use-region-p) (region-end))
@@ -4413,6 +4418,50 @@ minibuffer-lazy-highlight-setup
(make-overlay (point-min) (point-min) (current-buffer) t)))
(minibuffer-lazy-highlight--after-change nil nil nil))
+(cl-defmacro with-minibuffer-lazy-highlight (lazy-highlight &rest body &keys (regexp 'unset) (regexp-function 'unset) (case-fold 'unset) (cleanup 'unset) (region 'unset) (transform 'unset))
+ (while (keywordp (car body) (setq body (cddr body))))
+ `(let ((isearch-lazy-highlight ,lazy-highlight)
+ ,@(unless (eq 'unset ,case-fold) `(isearch-case-fold-search ,case-fold))
+ ,@(unless (eq 'unset ,regexp) `(isearch-regexp ,regexp))
+ ,@(unless (eq 'unset ,regexp-function) `(isearch-regexp-function ,regexp-function))
+ ,@(unless (eq 'unset ,cleanup) `(lazy-highlight-cleanup ,cleanup))
+ ,@(unless (eq 'unset ,transform )`(minibuffer-lazy-highlight-transform ,transform)))
+ (when isearch-lazy-highlight
+ (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
+ (when ,region
+ (letrec ((region-filter (isearch-region-filter
+ (funcall region-extract-function ,region)))
+ (cleanup (lambda ()
+ (remove-function isearch-filter-predicate region-filter)
+ (remove-hook 'minibuffer-exit-hook cleanup))))
+ (add-function :after-while isearch-filter-predicate region-filter)
+ (add-hook 'minibuffer-exit-hook cleanup))))
+ (condition-case error
+ (progn ,@body)
+ (t (lazy-highlight-cleanup)
+ (signal (car error) (cdr error))))))
+
+(defun isearch-region-filter (bounds)
+ "Return a function that decides if a region is inside BOUNDS.
+BOUNDS is a list of cons cells of the form (START . END). The
+returned function takes as argument two buffer positions, START
+and END."
+ (let ((region-bounds
+ (mapcar (lambda (position)
+ (cons (copy-marker (car position))
+ (copy-marker (cdr position))))
+ bounds)))
+ (lambda (start end)
+ (delq nil (mapcar
+ (lambda (bounds)
+ (and
+ (>= start (car bounds))
+ (<= start (cdr bounds))
+ (>= end (car bounds))
+ (<= end (cdr bounds))))
+ region-bounds)))))
+
+
\f
(defun isearch-resume (string regexp word forward message case-fold)
"Resume an incremental search.
diff --git a/lisp/replace.el b/lisp/replace.el
index e6f565d802..bed4a2824d 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -352,8 +352,15 @@ query-replace-read-to
(query-replace-compile-replacement
(save-excursion
(let* ((history-add-new-input nil)
+ (count (if (and query-replace-lazy-highlight
+ minibuffer-lazy-count-format
+ isearch-lazy-count
+ isearch-lazy-count-total)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)
+ ""))
(to (read-from-minibuffer
- (format "%s %s with: " prompt (query-replace-descr from))
+ (format "%s%s %s with: " count prompt (query-replace-descr from))
nil nil nil
query-replace-to-history-variable from t)))
(add-to-history query-replace-to-history-variable to nil t)
@@ -365,7 +372,26 @@ query-replace-read-args
(unless noerror
(barf-if-buffer-read-only))
(save-mark-and-excursion
- (let* ((from (query-replace-read-from prompt regexp-flag))
+ (let* ((from (with-minibuffer-lazy-highlight
+ query-replace-lazy-highlight
+ :case-fold case-fold-search
+ :regexp regexp-flag
+ :regexp-function (or replace-regexp-function
+ (and current-prefix-arg
+ (not (eq current-prefix-arg '-)))
+ (and replace-char-fold
+ (not regexp-flag)
+ #'char-fold-to-regexp)))
+ :cleanup nil
+ :transform (minibuffer-lazy-highlight-transform
+ (lambda (string)
+ (let* ((split (query-replace--split-string string))
+ (from-string (if (consp split) (car split) split)))
+ (when (and case-fold-search search-upper-case)
+ (setq isearch-case-fold-search
+ (isearch-no-upper-case-p from-string regexp-flag)))
+ from-string)))
+ (query-replace-read-from prompt regexp-flag))
(to (if (consp from) (prog1 (cdr from) (setq from (car from)))
(query-replace-read-to from prompt regexp-flag))))
(list from to
@@ -2862,22 +2888,9 @@ perform-replace
;; Unless a single contiguous chunk is selected, operate on multiple chunks.
(when region-noncontiguous-p
- (let ((region-bounds
- (mapcar (lambda (position)
- (cons (copy-marker (car position))
- (copy-marker (cdr position))))
- (funcall region-extract-function 'bounds))))
- (setq region-filter
- (lambda (start end)
- (delq nil (mapcar
- (lambda (bounds)
- (and
- (>= start (car bounds))
- (<= start (cdr bounds))
- (>= end (car bounds))
- (<= end (cdr bounds))))
- region-bounds))))
- (add-function :after-while isearch-filter-predicate region-filter)))
+ (setq region-filter (isearch-region-filter
+ (funcall region-extract-function 'bounds)))
+ (add-function :after-while isearch-filter-predicate region-filter))
;; If region is active, in Transient Mark mode, operate on region.
(if backward
--
2.35.1
[-- Attachment #3: Type: text/plain, Size: 1227 bytes --]
Below a response to your last message.
On Sat, 2 Apr 2022 at 21:23, Juri Linkov <juri@linkov.net> wrote:
>> - The value of case-fold-search can change on the fly
>
> Please clarify when it can be changed on the fly.
I refer to the following lines of my patch, which are necessary.
(when (and case-fold-search search-upper-case)
(setq isearch-case-fold-search
(isearch-no-upper-case-p from-string regexp-flag)))
(Incidentally, I like better the ripgrep smart case, where an uppercase
character only matches itself, and lowercase characters match both upper
and lower case.)
>> I'm referring to the anzu feature whereby the replacement text is shown
>> next to each match, like this:
>>
>> This will not be totally trivial to implement, right? And it will add
>> some extra stuff to query-replace-read-args that is very much
>> query-replace specific.
>
> I very much doubt in usefulness of such a feature. I thought it could
> simply highlight the replacement text to show the places in the buffer
> that already contain such replacement text.
The anzu thing is only really meaningful if you are doing some complex
replacements with \, and whatnot. But in that case, I guess it could
help a lot?
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-03 8:32 ` Augusto Stoffel
@ 2022-04-03 17:06 ` Juri Linkov
2022-04-04 16:37 ` Juri Linkov
1 sibling, 0 replies; 57+ messages in thread
From: Juri Linkov @ 2022-04-03 17:06 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> I've attached a sketch of a macro to help activating the minibuffer lazy
> highlight. It doesn't make query-replace-read-args exactly short,
I guess it will be much shorter for other less complex uses like occur?
So the complexity of this feature activation will be hidden inside the macro.
> Note that I haven't tested this code at all. It's for impressionistic
> purposes only.
This code doesn't look bad when other uses will have just one line with
with-minibuffer-lazy-highlight. But then the macro
with-minibuffer-lazy-highlight needs to set some reasonable
default values to isearch variables, such as setting
isearch-forward to t, etc.
> but if you like this, I can prepare the patch.
Currently it fails with
Eager macro-expansion failure: (error
"Malformed argument list ends with: ...
so it seems the macro complexity makes it too fragile.
OTOH, spreading this complexity in every feature use
doesn't look better.
> The anzu thing is only really meaningful if you are doing some complex
> replacements with \, and whatnot. But in that case, I guess it could
> help a lot?
I can see how it could help during replacements, but not during entering
the replacement string.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-03 8:32 ` Augusto Stoffel
2022-04-03 17:06 ` Juri Linkov
@ 2022-04-04 16:37 ` Juri Linkov
2022-04-05 16:38 ` Augusto Stoffel
1 sibling, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-04-04 16:37 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> I've attached a sketch of a macro to help activating the minibuffer lazy
> highlight. It doesn't make query-replace-read-args exactly short, but
> if you like this, I can prepare the patch.
I realized we already have the macro minibuffer-with-setup-hook,
so we don't need more macros.
The purpose of minibuffer-with-setup-hook is to set up the minibuffer
to the initial state with buffer-local variables. In case of
lazy-highlighting, we can't set isearch variables directly,
but all isearch variables can be replaced by one variable
that contains a set of lazy-highlight parameters. For example:
(minibuffer-with-setup-hook
(lambda ()
(setq-local lazy-highlight-params
`((case-fold ,case-fold-search)
(regexp ,regexp-flag)
(regexp-function ,(or replace-regexp-function
(and current-prefix-arg
(not (eq current-prefix-arg '-)))
(and replace-char-fold
(not regexp-flag)
#'char-fold-to-regexp)))))
(minibuffer-lazy-highlight-init))
(query-replace-read-from prompt regexp-flag))
where minibuffer-lazy-highlight-init does a little more than
minibuffer-lazy-highlight-setup. Or maybe it's not needed,
and minibuffer-lazy-highlight-setup should be enough.
Or maybe instead of setq-local is can let-bind variables.
Please also note that condition-case can be replaced by
a hook in minibuffer-exit-hook that can remove highlighting
after exiting the minibuffer.
Alternatively, the same lambda above could be added to
(add-hook 'minibuffer-setup-hook (lambda () ...))
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-04 16:37 ` Juri Linkov
@ 2022-04-05 16:38 ` Augusto Stoffel
2022-04-05 17:12 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-04-05 16:38 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Mon, 4 Apr 2022 at 19:37, Juri Linkov <juri@linkov.net> wrote:
>> I've attached a sketch of a macro to help activating the minibuffer lazy
>> highlight. It doesn't make query-replace-read-args exactly short, but
>> if you like this, I can prepare the patch.
>
> I realized we already have the macro minibuffer-with-setup-hook,
> so we don't need more macros.
>
> The purpose of minibuffer-with-setup-hook is to set up the minibuffer
> to the initial state with buffer-local variables. In case of
> lazy-highlighting, we can't set isearch variables directly,
> but all isearch variables can be replaced by one variable
> that contains a set of lazy-highlight parameters. For example:
>
> (minibuffer-with-setup-hook
> (lambda ()
> (setq-local lazy-highlight-params
> `((case-fold ,case-fold-search)
> (regexp ,regexp-flag)
> (regexp-function ,(or replace-regexp-function
> (and current-prefix-arg
> (not (eq current-prefix-arg '-)))
> (and replace-char-fold
> (not regexp-flag)
> #'char-fold-to-regexp)))))
> (minibuffer-lazy-highlight-init))
> (query-replace-read-from prompt regexp-flag))
With this code, case-fold-search and all following lazy highlight params
are evaluated with the minibuffer as current buffer, which is not the
intended behavior.
But that's a good point, we don't need a macro. Among several
variations, we could make the setup code look like this:
(minibuffer-with-setup-hook
(minibuffer-lazy-highlight-init :case-fold case-fold-search
:regexp regexp-flag
...)
(query-replace-read-from prompt regexp-flag))
where now `minibuffer-lazy-highlight-init' is not the function that
initializes stuff, but rather a function that returns a closure that
initializes stuff.
> where minibuffer-lazy-highlight-init does a little more than
> minibuffer-lazy-highlight-setup. Or maybe it's not needed,
> and minibuffer-lazy-highlight-setup should be enough.
> Or maybe instead of setq-local is can let-bind variables.
>
> Please also note that condition-case can be replaced by
> a hook in minibuffer-exit-hook that can remove highlighting
> after exiting the minibuffer.
If it was a `unwind-protect', I would agree. But I don't know how to
simulate a `condition-case'. Specifically, how can we determine if some
hook (the minibuffer-exit-hook in this case) is being run "normally" or
as part of the recovery from a signaled error?
> Alternatively, the same lambda above could be added to
>
> (add-hook 'minibuffer-setup-hook (lambda () ...))
Why was it again that we want to avoid saying something like this?
(let ((case-fold-search whatever)
(isearch-regexp regexp-flag))
(minibuffer-with-setup-hook #'minibuffer-lazy-highlight-init
(query-replace-read-from prompt regexp-flag)))
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-05 16:38 ` Augusto Stoffel
@ 2022-04-05 17:12 ` Juri Linkov
2022-04-07 19:32 ` Augusto Stoffel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2022-04-05 17:12 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
> But that's a good point, we don't need a macro. Among several
> variations, we could make the setup code look like this:
>
> (minibuffer-with-setup-hook
> (minibuffer-lazy-highlight-init :case-fold case-fold-search
> :regexp regexp-flag
> ...)
> (query-replace-read-from prompt regexp-flag))
>
> where now `minibuffer-lazy-highlight-init' is not the function that
> initializes stuff, but rather a function that returns a closure that
> initializes stuff.
Looks good.
>> Please also note that condition-case can be replaced by
>> a hook in minibuffer-exit-hook that can remove highlighting
>> after exiting the minibuffer.
>
> If it was a `unwind-protect', I would agree. But I don't know how to
> simulate a `condition-case'. Specifically, how can we determine if some
> hook (the minibuffer-exit-hook in this case) is being run "normally" or
> as part of the recovery from a signaled error?
Shouldn't both cases clean up highlight from the buffer?
Then I see no need to distinguish each case. Or if really needed,
you can try to bind the cleanup to command-error-function.
>> Alternatively, the same lambda above could be added to
>>
>> (add-hook 'minibuffer-setup-hook (lambda () ...))
>
> Why was it again that we want to avoid saying something like this?
>
> (let ((case-fold-search whatever)
> (isearch-regexp regexp-flag))
> (minibuffer-with-setup-hook #'minibuffer-lazy-highlight-init
> (query-replace-read-from prompt regexp-flag)))
4 lines look nice, unlike 20 lines in one of your patches ;-)
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-05 17:12 ` Juri Linkov
@ 2022-04-07 19:32 ` Augusto Stoffel
2022-04-08 7:32 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-04-07 19:32 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 833 bytes --]
On Tue, 5 Apr 2022 at 20:12, Juri Linkov <juri@linkov.net> wrote:
>> But that's a good point, we don't need a macro. Among several
>> variations, we could make the setup code look like this:
>>
>> (minibuffer-with-setup-hook
>> (minibuffer-lazy-highlight-init :case-fold case-fold-search
>> :regexp regexp-flag
>> ...)
>> (query-replace-read-from prompt regexp-flag))
>>
>> where now `minibuffer-lazy-highlight-init' is not the function that
>> initializes stuff, but rather a function that returns a closure that
>> initializes stuff.
>
> Looks good.
Okay, I've refactored my code like this. I actually like it better that
way. (As a downside, the stuff that was already merged to isearch.el is
completely changed.)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-lazy-highlight-and-match-count-in-query-repl.patch --]
[-- Type: text/x-patch, Size: 14361 bytes --]
From 173c24fc4f90c92f5c9035c76fc578bf11d33294 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Thu, 17 Mar 2022 20:17:26 +0100
Subject: [PATCH] Display lazy highlight and match count in query-replace
* lisp/isearch.el (isearch-query-replace): Don't clean up lazy
highlight if applicable.
* lisp/replace.el (query-replace-read-args, query-replace-read-to):
Add lazy highlighting and count.
(replace--region-filter): New function, extracted from
'perform-replace'.
(perform-replace): Use 'replace--region-filter'.
---
lisp/isearch.el | 144 +++++++++++++++++++++++++++---------------------
lisp/replace.el | 77 +++++++++++++++++++-------
2 files changed, 140 insertions(+), 81 deletions(-)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 956b115ce4..48f2c3bf41 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1812,20 +1812,20 @@ isearch-edit-string
(minibuffer-history-symbol)
;; Search string might have meta information on text properties.
(minibuffer-allow-text-properties t))
- (when isearch-lazy-highlight
- (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup))
(setq isearch-new-string
- (read-from-minibuffer
- (isearch-message-prefix nil isearch-nonincremental)
- (cons isearch-string (1+ (or (isearch-fail-pos)
- (length isearch-string))))
- minibuffer-local-isearch-map nil
- (if isearch-regexp
- (cons 'regexp-search-ring
- (1+ (or regexp-search-ring-yank-pointer -1)))
- (cons 'search-ring
- (1+ (or search-ring-yank-pointer -1))))
- nil t)
+ (minibuffer-with-setup-hook
+ (minibuffer-lazy-highlight-setup)
+ (read-from-minibuffer
+ (isearch-message-prefix nil isearch-nonincremental)
+ (cons isearch-string (1+ (or (isearch-fail-pos)
+ (length isearch-string))))
+ minibuffer-local-isearch-map nil
+ (if isearch-regexp
+ (cons 'regexp-search-ring
+ (1+ (or regexp-search-ring-yank-pointer -1)))
+ (cons 'search-ring
+ (1+ (or search-ring-yank-pointer -1))))
+ nil t))
isearch-new-message
(mapconcat 'isearch-text-char-description
isearch-new-string "")))))
@@ -4361,57 +4361,77 @@ minibuffer-lazy-count-format
:group 'lazy-count
:version "29.1")
-(defvar minibuffer-lazy-highlight-transform #'identity
- "Function to transform minibuffer text into a `isearch-string' for highlighting.")
-
-(defvar minibuffer-lazy-highlight--overlay nil
- "Overlay for minibuffer prompt updates.")
-
-(defun minibuffer-lazy-highlight--count ()
- "Display total match count in the minibuffer prompt."
- (when minibuffer-lazy-highlight--overlay
- (overlay-put minibuffer-lazy-highlight--overlay
- 'before-string
- (and isearch-lazy-count-total
- (not isearch-error)
- (format minibuffer-lazy-count-format
- isearch-lazy-count-total)))))
-
-(defun minibuffer-lazy-highlight--after-change (_beg _end _len)
- "Update lazy highlight state in minibuffer selected window."
- (when isearch-lazy-highlight
- (let ((inhibit-redisplay t) ;; Avoid cursor flickering
- (string (minibuffer-contents)))
- (with-minibuffer-selected-window
- (setq isearch-string (funcall minibuffer-lazy-highlight-transform string))
- (isearch-lazy-highlight-new-loop)))))
-
-(defun minibuffer-lazy-highlight--exit ()
- "Unwind changes from `minibuffer-lazy-highlight-setup'."
- (remove-hook 'after-change-functions
- #'minibuffer-lazy-highlight--after-change)
- (remove-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
- (remove-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
- (setq minibuffer-lazy-highlight--overlay nil)
- (when lazy-highlight-cleanup
- (lazy-highlight-cleanup)))
-
-(defun minibuffer-lazy-highlight-setup ()
+(cl-defun minibuffer-lazy-highlight-setup (&key (highlight isearch-lazy-highlight)
+ (cleanup lazy-highlight-cleanup)
+ (transform #'identity)
+ (filter nil)
+ (regexp isearch-regexp)
+ (regexp-function isearch-regexp-function)
+ (case-fold isearch-case-fold-search)
+ (lax-whitespace (if regexp
+ isearch-regexp-lax-whitespace
+ isearch-lax-whitespace)))
"Set up minibuffer for lazy highlight of matches in the original window.
-This function is intended to be added to `minibuffer-setup-hook'.
-Note that several other isearch variables influence the lazy
-highlighting, including `isearch-regexp',
-`isearch-lazy-highlight' and `isearch-lazy-count'."
- (remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
- (add-hook 'after-change-functions
- #'minibuffer-lazy-highlight--after-change)
- (add-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
- (add-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
- (setq minibuffer-lazy-highlight--overlay
- (and minibuffer-lazy-count-format
- (make-overlay (point-min) (point-min) (current-buffer) t)))
- (minibuffer-lazy-highlight--after-change nil nil nil))
+This function return a closure intended to be added to
+`minibuffer-setup-hook'. It accepts the following keyword
+arguments, all of which have a default based on the current
+isearch settings:
+
+HIGHLIGHT: Whether to perform lazy highlight.
+CLEANUP: Whether to clean up the lazy highlight when the minibuffer
+exits.
+TRANSFORM: A function to transform the minibuffer contents into a
+search string.
+FILTER: A function to add to `isearch-filter-predicate'.
+REGEXP: The value of `isearch-regexp' to use for lazy highlight.
+REGEXP-FUNCTION: The value of `isearch-regexp-function' to use for
+lazy highlight.
+CASE-FOLD: The value of `isearch-case-fold' to use for lazy highlight.
+LAX-WHITESPACE: The value of `isearch-(regexp-)lax-whitespace' to use
+for lazy highlight."
+ (if (not highlight)
+ #'ignore
+ (let ((unwind (make-symbol "minibuffer-lazy-highlight--unwind"))
+ (after-change (make-symbol "minibuffer-lazy-highlight--after-change"))
+ (display-count (make-symbol "minibuffer-lazy-highlight--display-count"))
+ overlay)
+ (fset unwind
+ (lambda ()
+ (remove-hook 'minibuffer-exit-hook unwind)
+ (remove-hook 'after-change-functions after-change)
+ (remove-hook 'lazy-count-update-hook display-count)
+ (remove-function isearch-filter-predicate filter)
+ (when cleanup (lazy-highlight-cleanup))))
+ (fset after-change
+ (lambda (_beg _end _len)
+ (let ((inhibit-redisplay t) ;; Avoid cursor flickering
+ (string (minibuffer-contents)))
+ (with-minibuffer-selected-window
+ (let* ((isearch-forward t)
+ (isearch-regexp regexp)
+ (isearch-regexp-function regexp-function)
+ (isearch-case-fold-search case-fold)
+ (isearch-lax-whitespace lax-whitespace)
+ (isearch-regexp-lax-whitespace lax-whitespace)
+ (isearch-string (funcall transform string)))
+ (isearch-lazy-highlight-new-loop))))))
+ (fset display-count
+ (lambda ()
+ (overlay-put overlay 'before-string
+ (and isearch-lazy-count-total
+ (not isearch-error)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)))))
+ (lambda ()
+ (add-hook 'minibuffer-exit-hook unwind)
+ (add-hook 'after-change-functions after-change)
+ (when filter
+ (add-function :after-while isearch-filter-predicate filter))
+ (when minibuffer-lazy-count-format
+ (setq overlay (make-overlay (point-min) (point-min) (current-buffer) t))
+ (add-hook 'lazy-count-update-hook display-count))
+ (funcall after-change nil nil nil)))))
\f
(defun isearch-resume (string regexp word forward message case-fold)
diff --git a/lisp/replace.el b/lisp/replace.el
index e6f565d802..fed4dfd457 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -352,8 +352,15 @@ query-replace-read-to
(query-replace-compile-replacement
(save-excursion
(let* ((history-add-new-input nil)
+ (count (if (and query-replace-lazy-highlight
+ minibuffer-lazy-count-format
+ isearch-lazy-count
+ isearch-lazy-count-total)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)
+ ""))
(to (read-from-minibuffer
- (format "%s %s with: " prompt (query-replace-descr from))
+ (format "%s%s %s with: " count prompt (query-replace-descr from))
nil nil nil
query-replace-to-history-variable from t)))
(add-to-history query-replace-to-history-variable to nil t)
@@ -365,11 +372,29 @@ query-replace-read-args
(unless noerror
(barf-if-buffer-read-only))
(save-mark-and-excursion
- (let* ((from (query-replace-read-from prompt regexp-flag))
+ (let* ((delimited-flag (and current-prefix-arg
+ (not (eq current-prefix-arg '-))))
+ (from (minibuffer-with-setup-hook
+ (minibuffer-lazy-highlight-setup
+ :case-fold case-fold-search
+ :filter (when (use-region-p)
+ (replace--region-filter
+ (funcall region-extract-function 'bounds)))
+ :highlight query-replace-lazy-highlight
+ :regexp regexp-flag
+ :regexp-function (replace-regexp-function delimited-flag regexp-flag)
+ :transform (lambda (string)
+ (let* ((split (query-replace--split-string string))
+ (from-string (if (consp split) (car split) split)))
+ (when (and case-fold-search search-upper-case)
+ (setq isearch-case-fold-search
+ (isearch-no-upper-case-p from-string regexp-flag)))
+ from-string)))
+ (query-replace-read-from prompt regexp-flag)))
(to (if (consp from) (prog1 (cdr from) (setq from (car from)))
(query-replace-read-to from prompt regexp-flag))))
(list from to
- (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
+ (or delimited-flag
(and (plist-member (text-properties-at 0 from) 'isearch-regexp-function)
(get-text-property 0 'isearch-regexp-function from)))
(and current-prefix-arg (eq current-prefix-arg '-))))))
@@ -2656,6 +2681,13 @@ replace-regexp-function
is not to be interpreted literally, but instead should be converted
to a regexp that is actually used for the search.")
+(defun replace-regexp-function (delimited-flag regexp-flag)
+ (or replace-regexp-function
+ delimited-flag
+ (and replace-char-fold
+ (not regexp-flag)
+ #'char-fold-to-regexp)))
+
(defun replace-search (search-string limit regexp-flag delimited-flag
case-fold &optional backward)
"Search for the next occurrence of SEARCH-STRING to replace."
@@ -2778,6 +2810,26 @@ replace--push-stack
,search-str ,next-replace)
,stack))
+(defun replace--region-filter (bounds)
+ "Return a function that decides if a region is inside BOUNDS.
+BOUNDS is a list of cons cells of the form (START . END). The
+returned function takes as argument two buffer positions, START
+and END."
+ (let ((region-bounds
+ (mapcar (lambda (position)
+ (cons (copy-marker (car position))
+ (copy-marker (cdr position))))
+ bounds)))
+ (lambda (start end)
+ (delq nil (mapcar
+ (lambda (bounds)
+ (and
+ (>= start (car bounds))
+ (<= start (cdr bounds))
+ (>= end (car bounds))
+ (<= end (cdr bounds))))
+ region-bounds)))))
+
(defun perform-replace (from-string replacements
query-flag regexp-flag delimited-flag
&optional repeat-count map start end backward region-noncontiguous-p)
@@ -2862,22 +2914,9 @@ perform-replace
;; Unless a single contiguous chunk is selected, operate on multiple chunks.
(when region-noncontiguous-p
- (let ((region-bounds
- (mapcar (lambda (position)
- (cons (copy-marker (car position))
- (copy-marker (cdr position))))
- (funcall region-extract-function 'bounds))))
- (setq region-filter
- (lambda (start end)
- (delq nil (mapcar
- (lambda (bounds)
- (and
- (>= start (car bounds))
- (<= start (cdr bounds))
- (>= end (car bounds))
- (<= end (cdr bounds))))
- region-bounds))))
- (add-function :after-while isearch-filter-predicate region-filter)))
+ (setq region-filter (replace--region-filter
+ (funcall region-extract-function 'bounds)))
+ (add-function :after-while isearch-filter-predicate region-filter))
;; If region is active, in Transient Mark mode, operate on region.
(if backward
--
2.35.1
[-- Attachment #3: Type: text/plain, Size: 1630 bytes --]
>>> Please also note that condition-case can be replaced by
>>> a hook in minibuffer-exit-hook that can remove highlighting
>>> after exiting the minibuffer.
>>
>> If it was a `unwind-protect', I would agree. But I don't know how to
>> simulate a `condition-case'. Specifically, how can we determine if some
>> hook (the minibuffer-exit-hook in this case) is being run "normally" or
>> as part of the recovery from a signaled error?
>
> Shouldn't both cases clean up highlight from the buffer?
> Then I see no need to distinguish each case. Or if really needed,
> you can try to bind the cleanup to command-error-function.
My previous patch had only one case: if the user quits, we clean up the
highlighting.
I can only see one simpler alternative, which is to always
unconditionally clean up the highlight. This is not as nice, but if
keeping the code as simple as possible is important here, then I guess
this is the way forward. So that's what the current patch does.
I suspect people will see this as a bug, but maybe discussing this issue
by itself later will be easier.
>>> Alternatively, the same lambda above could be added to
>>>
>>> (add-hook 'minibuffer-setup-hook (lambda () ...))
>>
>> Why was it again that we want to avoid saying something like this?
>>
>> (let ((case-fold-search whatever)
>> (isearch-regexp regexp-flag))
>> (minibuffer-with-setup-hook #'minibuffer-lazy-highlight-init
>> (query-replace-read-from prompt regexp-flag)))
>
> 4 lines look nice, unlike 20 lines in one of your patches ;-)
When you add all the bells and whistles, 4 lines just won't do it.
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-07 19:32 ` Augusto Stoffel
@ 2022-04-08 7:32 ` Juri Linkov
2022-04-08 7:53 ` Augusto Stoffel
2022-04-09 11:06 ` Augusto Stoffel
0 siblings, 2 replies; 57+ messages in thread
From: Juri Linkov @ 2022-04-08 7:32 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
>> Looks good.
>
> Okay, I've refactored my code like this. I actually like it better that
> way. (As a downside, the stuff that was already merged to isearch.el is
> completely changed.)
Thanks, now it looks much better!
In `isearch-edit-string' you replaced:
(add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
with
(minibuffer-with-setup-hook (minibuffer-lazy-highlight-setup)
but the docstring of `minibuffer-lazy-highlight-setup' in your patch is:
This function return a closure intended to be added to
`minibuffer-setup-hook'.
Maybe either a typo that needs to mention `minibuffer-with-setup-hook',
or `minibuffer-setup-hook' needs to evaluate such closure with something like
(add-hook 'minibuffer-setup-hook (funcall 'minibuffer-lazy-highlight-setup))
But since this is more ugly, then using `minibuffer-with-setup-hook' is fine.
>> Shouldn't both cases clean up highlight from the buffer?
>> Then I see no need to distinguish each case. Or if really needed,
>> you can try to bind the cleanup to command-error-function.
>
> My previous patch had only one case: if the user quits, we clean up the
> highlighting.
>
> I can only see one simpler alternative, which is to always
> unconditionally clean up the highlight. This is not as nice, but if
> keeping the code as simple as possible is important here, then I guess
> this is the way forward. So that's what the current patch does.
>
> I suspect people will see this as a bug, but maybe discussing this issue
> by itself later will be easier.
Yep, let's do this later when people will ask for it.
>> 4 lines look nice, unlike 20 lines in one of your patches ;-)
>
> When you add all the bells and whistles, 4 lines just won't do it.
Now the parameters of minibuffer-lazy-highlight-setup look nice,
so line count doesn't matter when code keeps simplicity.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-08 7:32 ` Juri Linkov
@ 2022-04-08 7:53 ` Augusto Stoffel
2022-04-09 11:06 ` Augusto Stoffel
1 sibling, 0 replies; 57+ messages in thread
From: Augusto Stoffel @ 2022-04-08 7:53 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
On Fri, 8 Apr 2022 at 10:32, Juri Linkov <juri@linkov.net> wrote:
>>> Looks good.
>>
>> Okay, I've refactored my code like this. I actually like it better that
>> way. (As a downside, the stuff that was already merged to isearch.el is
>> completely changed.)
>
> Thanks, now it looks much better!
Great, then I'll test it some more and prepare properly formatted
patches.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-08 7:32 ` Juri Linkov
2022-04-08 7:53 ` Augusto Stoffel
@ 2022-04-09 11:06 ` Augusto Stoffel
2022-04-10 19:38 ` Juri Linkov
1 sibling, 1 reply; 57+ messages in thread
From: Augusto Stoffel @ 2022-04-09 11:06 UTC (permalink / raw)
To: Juri Linkov; +Cc: 53126
[-- Attachment #1: Type: text/plain, Size: 337 bytes --]
On Fri, 8 Apr 2022 at 10:32, Juri Linkov <juri@linkov.net> wrote:
>>> Looks good.
>>
>> Okay, I've refactored my code like this. I actually like it better that
>> way. (As a downside, the stuff that was already merged to isearch.el is
>> completely changed.)
>
> Thanks, now it looks much better!
Please find attached the patches.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Rewrite-the-minibuffer-lazy-highlight-feature.patch --]
[-- Type: text/x-patch, Size: 9429 bytes --]
From 5ed0e5a850992c62189c0dc74ef98b892522d4df Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 9 Apr 2022 12:38:14 +0200
Subject: [PATCH 1/2] Rewrite the minibuffer lazy highlight feature
The new API was discussed in bug#53126. It's more robust and easier
to use in complex cases like that of 'query-replace'.
* etc/NEWS: Amend the feature announcement
* lisp/isearch.el (isearch-edit-string): Use new API.
(minibuffer-lazy-highlight-transform,
minibuffer-lazy-highlight--overlay, minibuffer-lazy-highlight--count,
minibuffer-lazy-highlight--after-change,
minibuffer-lazy-highlight--exit) Remove helper functions, which are
now kept together with the lazy highlight configuration variables
within a closure.
(minibuffer-lazy-highlight-setup): This function now takes the lazy
highlighting configuration variables as argument, and returns a
closure that is intended to run as part of the minibuffer setup.
---
etc/NEWS | 5 +-
lisp/isearch.el | 148 ++++++++++++++++++++++++++++--------------------
2 files changed, 88 insertions(+), 65 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index 2fac893cc5..2cddfcc8db 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1593,9 +1593,8 @@ the clipboard, and insert it into the buffer.
---
** New function 'minibuffer-lazy-highlight-setup'.
-This function is intended to be added to 'minibuffer-setup-hook'.
-It sets up the minibuffer for lazy highlighting of matches
-in the original window.
+This function allows to set up the minibuffer so that lazy
+highlighting of its content is applied in the original window.
+++
** New text property 'inhibit-isearch'.
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 956b115ce4..168d71ada3 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1812,20 +1812,20 @@ isearch-edit-string
(minibuffer-history-symbol)
;; Search string might have meta information on text properties.
(minibuffer-allow-text-properties t))
- (when isearch-lazy-highlight
- (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup))
(setq isearch-new-string
- (read-from-minibuffer
- (isearch-message-prefix nil isearch-nonincremental)
- (cons isearch-string (1+ (or (isearch-fail-pos)
- (length isearch-string))))
- minibuffer-local-isearch-map nil
- (if isearch-regexp
- (cons 'regexp-search-ring
- (1+ (or regexp-search-ring-yank-pointer -1)))
- (cons 'search-ring
- (1+ (or search-ring-yank-pointer -1))))
- nil t)
+ (minibuffer-with-setup-hook
+ (minibuffer-lazy-highlight-setup)
+ (read-from-minibuffer
+ (isearch-message-prefix nil isearch-nonincremental)
+ (cons isearch-string (1+ (or (isearch-fail-pos)
+ (length isearch-string))))
+ minibuffer-local-isearch-map nil
+ (if isearch-regexp
+ (cons 'regexp-search-ring
+ (1+ (or regexp-search-ring-yank-pointer -1)))
+ (cons 'search-ring
+ (1+ (or search-ring-yank-pointer -1))))
+ nil t))
isearch-new-message
(mapconcat 'isearch-text-char-description
isearch-new-string "")))))
@@ -4361,57 +4361,81 @@ minibuffer-lazy-count-format
:group 'lazy-count
:version "29.1")
-(defvar minibuffer-lazy-highlight-transform #'identity
- "Function to transform minibuffer text into a `isearch-string' for highlighting.")
-
-(defvar minibuffer-lazy-highlight--overlay nil
- "Overlay for minibuffer prompt updates.")
-
-(defun minibuffer-lazy-highlight--count ()
- "Display total match count in the minibuffer prompt."
- (when minibuffer-lazy-highlight--overlay
- (overlay-put minibuffer-lazy-highlight--overlay
- 'before-string
- (and isearch-lazy-count-total
- (not isearch-error)
- (format minibuffer-lazy-count-format
- isearch-lazy-count-total)))))
-
-(defun minibuffer-lazy-highlight--after-change (_beg _end _len)
- "Update lazy highlight state in minibuffer selected window."
- (when isearch-lazy-highlight
- (let ((inhibit-redisplay t) ;; Avoid cursor flickering
- (string (minibuffer-contents)))
- (with-minibuffer-selected-window
- (setq isearch-string (funcall minibuffer-lazy-highlight-transform string))
- (isearch-lazy-highlight-new-loop)))))
-
-(defun minibuffer-lazy-highlight--exit ()
- "Unwind changes from `minibuffer-lazy-highlight-setup'."
- (remove-hook 'after-change-functions
- #'minibuffer-lazy-highlight--after-change)
- (remove-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
- (remove-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
- (setq minibuffer-lazy-highlight--overlay nil)
- (when lazy-highlight-cleanup
- (lazy-highlight-cleanup)))
-
-(defun minibuffer-lazy-highlight-setup ()
+(cl-defun minibuffer-lazy-highlight-setup
+ (&key (highlight isearch-lazy-highlight)
+ (cleanup lazy-highlight-cleanup)
+ (transform #'identity)
+ (filter nil)
+ (regexp isearch-regexp)
+ (regexp-function isearch-regexp-function)
+ (case-fold isearch-case-fold-search)
+ (lax-whitespace (if regexp
+ isearch-regexp-lax-whitespace
+ isearch-lax-whitespace)))
"Set up minibuffer for lazy highlight of matches in the original window.
-This function is intended to be added to `minibuffer-setup-hook'.
-Note that several other isearch variables influence the lazy
-highlighting, including `isearch-regexp',
-`isearch-lazy-highlight' and `isearch-lazy-count'."
- (remove-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
- (add-hook 'after-change-functions
- #'minibuffer-lazy-highlight--after-change)
- (add-hook 'lazy-count-update-hook #'minibuffer-lazy-highlight--count)
- (add-hook 'minibuffer-exit-hook #'minibuffer-lazy-highlight--exit)
- (setq minibuffer-lazy-highlight--overlay
- (and minibuffer-lazy-count-format
- (make-overlay (point-min) (point-min) (current-buffer) t)))
- (minibuffer-lazy-highlight--after-change nil nil nil))
+This function return a closure intended to be added to
+`minibuffer-setup-hook'. It accepts the following keyword
+arguments, all of which have a default based on the current
+isearch settings.
+
+HIGHLIGHT: Whether to perform lazy highlight.
+CLEANUP: Whether to clean up the lazy highlight when the minibuffer
+exits.
+TRANSFORM: A function taking one argument, the minibuffer contents,
+and returning the `isearch-string' to use for lazy highlighting.
+FILTER: A function to add to `isearch-filter-predicate'.
+REGEXP: The value of `isearch-regexp' to use for lazy highlighting.
+REGEXP-FUNCTION: The value of `isearch-regexp-function' to use for
+lazy highlighting.
+CASE-FOLD: The value of `isearch-case-fold' to use for lazy
+highlighting.
+LAX-WHITESPACE: The value of `isearch-lax-whitespace' and
+`isearch-regexp-lax-whitespace' to use for lazy highlighting."
+ (if (not highlight)
+ #'ignore
+ (let ((unwind (make-symbol "minibuffer-lazy-highlight--unwind"))
+ (after-change (make-symbol "minibuffer-lazy-highlight--after-change"))
+ (display-count (make-symbol "minibuffer-lazy-highlight--display-count"))
+ overlay)
+ (fset unwind
+ (lambda ()
+ (remove-function isearch-filter-predicate filter)
+ (remove-hook 'lazy-count-update-hook display-count)
+ (when overlay (delete-overlay overlay))
+ (remove-hook 'after-change-functions after-change)
+ (remove-hook 'minibuffer-exit-hook unwind)
+ (let ((lazy-highlight-cleanup cleanup))
+ (lazy-highlight-cleanup))))
+ (fset after-change
+ (lambda (_beg _end _len)
+ (let ((inhibit-redisplay t) ;; Avoid cursor flickering
+ (string (minibuffer-contents)))
+ (with-minibuffer-selected-window
+ (let* ((isearch-forward t)
+ (isearch-regexp regexp)
+ (isearch-regexp-function regexp-function)
+ (isearch-case-fold-search case-fold)
+ (isearch-lax-whitespace lax-whitespace)
+ (isearch-regexp-lax-whitespace lax-whitespace)
+ (isearch-string (funcall transform string)))
+ (isearch-lazy-highlight-new-loop))))))
+ (fset display-count
+ (lambda ()
+ (overlay-put overlay 'before-string
+ (and isearch-lazy-count-total
+ (not isearch-error)
+ (format minibuffer-lazy-count-format
+ isearch-lazy-count-total)))))
+ (lambda ()
+ (add-hook 'minibuffer-exit-hook unwind)
+ (add-hook 'after-change-functions after-change)
+ (when minibuffer-lazy-count-format
+ (setq overlay (make-overlay (point-min) (point-min) (current-buffer) t))
+ (add-hook 'lazy-count-update-hook display-count))
+ (when filter
+ (add-function :after-while isearch-filter-predicate filter))
+ (funcall after-change nil nil nil)))))
\f
(defun isearch-resume (string regexp word forward message case-fold)
--
2.35.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-lazy-highlight-when-reading-query-replace-argume.patch --]
[-- Type: text/x-patch, Size: 5305 bytes --]
From 432fcd0172a538713d85e3d4a64c2586ae1a9a5b Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 9 Apr 2022 12:47:28 +0200
Subject: [PATCH 2/2] Add lazy highlight when reading 'query-replace' arguments
* lisp/replace.el (query-replace-read-args): Use
'minibuffer-lazy-highlight-setup' to highlight the text to be replaced
in the original buffer (and a match count, if applicable).
(replace--region-filter): New function for code that
used to be inlined in perform-replace but is useful elsewhere.
(perform-replace): Use 'replace--region-filter'.
---
lisp/replace.el | 65 +++++++++++++++++++++++++++++++++++--------------
1 file changed, 47 insertions(+), 18 deletions(-)
diff --git a/lisp/replace.el b/lisp/replace.el
index e6f565d802..00d30d1e38 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -365,11 +365,33 @@ query-replace-read-args
(unless noerror
(barf-if-buffer-read-only))
(save-mark-and-excursion
- (let* ((from (query-replace-read-from prompt regexp-flag))
+ (let* ((delimited-flag (and current-prefix-arg
+ (not (eq current-prefix-arg '-))))
+ (from (minibuffer-with-setup-hook
+ (minibuffer-lazy-highlight-setup
+ :case-fold case-fold-search
+ :filter (when (use-region-p)
+ (replace--region-filter
+ (funcall region-extract-function 'bounds)))
+ :highlight query-replace-lazy-highlight
+ :regexp regexp-flag
+ :regexp-function (or replace-regexp-function
+ delimited-flag
+ (and replace-char-fold
+ (not regexp-flag)
+ #'char-fold-to-regexp))
+ :transform (lambda (string)
+ (let* ((split (query-replace--split-string string))
+ (from-string (if (consp split) (car split) split)))
+ (when (and case-fold-search search-upper-case)
+ (setq isearch-case-fold-search
+ (isearch-no-upper-case-p from-string regexp-flag)))
+ from-string)))
+ (query-replace-read-from prompt regexp-flag)))
(to (if (consp from) (prog1 (cdr from) (setq from (car from)))
(query-replace-read-to from prompt regexp-flag))))
(list from to
- (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
+ (or delimited-flag
(and (plist-member (text-properties-at 0 from) 'isearch-regexp-function)
(get-text-property 0 'isearch-regexp-function from)))
(and current-prefix-arg (eq current-prefix-arg '-))))))
@@ -2778,6 +2800,26 @@ replace--push-stack
,search-str ,next-replace)
,stack))
+(defun replace--region-filter (bounds)
+ "Return a function that decides if a region is inside BOUNDS.
+BOUNDS is a list of cons cells of the form (START . END). The
+returned function takes as argument two buffer positions, START
+and END."
+ (let ((region-bounds
+ (mapcar (lambda (position)
+ (cons (copy-marker (car position))
+ (copy-marker (cdr position))))
+ bounds)))
+ (lambda (start end)
+ (delq nil (mapcar
+ (lambda (bounds)
+ (and
+ (>= start (car bounds))
+ (<= start (cdr bounds))
+ (>= end (car bounds))
+ (<= end (cdr bounds))))
+ region-bounds)))))
+
(defun perform-replace (from-string replacements
query-flag regexp-flag delimited-flag
&optional repeat-count map start end backward region-noncontiguous-p)
@@ -2862,22 +2904,9 @@ perform-replace
;; Unless a single contiguous chunk is selected, operate on multiple chunks.
(when region-noncontiguous-p
- (let ((region-bounds
- (mapcar (lambda (position)
- (cons (copy-marker (car position))
- (copy-marker (cdr position))))
- (funcall region-extract-function 'bounds))))
- (setq region-filter
- (lambda (start end)
- (delq nil (mapcar
- (lambda (bounds)
- (and
- (>= start (car bounds))
- (<= start (cdr bounds))
- (>= end (car bounds))
- (<= end (cdr bounds))))
- region-bounds))))
- (add-function :after-while isearch-filter-predicate region-filter)))
+ (setq region-filter (replace--region-filter
+ (funcall region-extract-function 'bounds)))
+ (add-function :after-while isearch-filter-predicate region-filter))
;; If region is active, in Transient Mark mode, operate on region.
(if backward
--
2.35.1
[-- Attachment #4: Type: text/plain, Size: 2483 bytes --]
> In `isearch-edit-string' you replaced:
> (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
> with
> (minibuffer-with-setup-hook (minibuffer-lazy-highlight-setup)
> but the docstring of `minibuffer-lazy-highlight-setup' in your patch is:
>
> This function return a closure intended to be added to
> `minibuffer-setup-hook'.
>
> Maybe either a typo that needs to mention `minibuffer-with-setup-hook',
> or `minibuffer-setup-hook' needs to evaluate such closure with something like
>
> (add-hook 'minibuffer-setup-hook (funcall 'minibuffer-lazy-highlight-setup))
>
> But since this is more ugly, then using `minibuffer-with-setup-hook' is fine.
>
Well, as a matter of fact the closure is meant to run in the
'minibuffer-setup-hook'. This is not to say it's a good idea to use
'add-hook' to do add it there. One should always use
'minibuffer-with-setup-hook' for that kind of stuff, I guess.
Feel free to copy edit the docstring, but in light of the above I didn't
change it for now.
>>> Shouldn't both cases clean up highlight from the buffer?
>>> Then I see no need to distinguish each case. Or if really needed,
>>> you can try to bind the cleanup to command-error-function.
>>
>> My previous patch had only one case: if the user quits, we clean up the
>> highlighting.
>>
>> I can only see one simpler alternative, which is to always
>> unconditionally clean up the highlight. This is not as nice, but if
>> keeping the code as simple as possible is important here, then I guess
>> this is the way forward. So that's what the current patch does.
>>
>> I suspect people will see this as a bug, but maybe discussing this issue
>> by itself later will be easier.
>
> Yep, let's do this later when people will ask for it.
All right, I've removed all the "clever business" related to delayed
clean up of the lazy highlight for now.
>>> 4 lines look nice, unlike 20 lines in one of your patches ;-)
>>
>> When you add all the bells and whistles, 4 lines just won't do it.
>
> Now the parameters of minibuffer-lazy-highlight-setup look nice,
> so line count doesn't matter when code keeps simplicity.
So, my sample code from the previous message defined a function for this
snippet which already repeats 3 times
(or replace-regexp-function
delimited-flag
(and replace-char-fold
(not regexp-flag)
#'char-fold-to-regexp))
I've removed that function for now. We can reintroduce the function if
you deem it helpful.
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
2022-04-09 11:06 ` Augusto Stoffel
@ 2022-04-10 19:38 ` Juri Linkov
0 siblings, 0 replies; 57+ messages in thread
From: Juri Linkov @ 2022-04-10 19:38 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 53126
close 53126 29.0.50
thanks
> Please find attached the patches.
Thanks for working on this feature.
Now your patch provides a quote elegant code
with an easy-to-use function with simple parameters.
So now pushed to master.
>> Yep, let's do this later when people will ask for it.
>
> All right, I've removed all the "clever business" related to delayed
> clean up of the lazy highlight for now.
Please create a new request when you want to add more features.
^ permalink raw reply [flat|nested] 57+ messages in thread
end of thread, other threads:[~2022-04-10 19:38 UTC | newest]
Thread overview: 57+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-08 13:24 bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc Augusto Stoffel
2022-01-08 18:59 ` Juri Linkov
2022-01-08 19:35 ` Augusto Stoffel
2022-01-09 9:10 ` Juri Linkov
2022-01-09 10:02 ` Augusto Stoffel
2022-01-09 10:30 ` Augusto Stoffel
2022-01-09 18:58 ` Juri Linkov
2022-01-10 17:34 ` Augusto Stoffel
2022-01-10 19:09 ` Juri Linkov
2022-02-26 16:13 ` Augusto Stoffel
2022-03-15 17:09 ` Juri Linkov
2022-03-15 21:33 ` Augusto Stoffel
2022-03-16 18:56 ` Juri Linkov
2022-03-16 20:09 ` Augusto Stoffel
2022-03-17 17:09 ` Juri Linkov
2022-03-17 19:10 ` Augusto Stoffel
2022-03-17 20:40 ` Juri Linkov
2022-03-17 21:42 ` Augusto Stoffel
2022-03-20 9:38 ` Augusto Stoffel
2022-03-20 18:51 ` Juri Linkov
2022-03-24 19:03 ` Augusto Stoffel
2022-03-25 8:39 ` Juri Linkov
2022-03-25 9:43 ` Augusto Stoffel
2022-03-27 7:46 ` Juri Linkov
2022-04-01 9:06 ` Augusto Stoffel
2022-04-01 16:35 ` Juri Linkov
2022-04-01 18:12 ` Augusto Stoffel
2022-04-02 18:23 ` Juri Linkov
2022-04-03 8:32 ` Augusto Stoffel
2022-04-03 17:06 ` Juri Linkov
2022-04-04 16:37 ` Juri Linkov
2022-04-05 16:38 ` Augusto Stoffel
2022-04-05 17:12 ` Juri Linkov
2022-04-07 19:32 ` Augusto Stoffel
2022-04-08 7:32 ` Juri Linkov
2022-04-08 7:53 ` Augusto Stoffel
2022-04-09 11:06 ` Augusto Stoffel
2022-04-10 19:38 ` Juri Linkov
2022-03-15 17:24 ` Juri Linkov
2022-03-15 21:21 ` Augusto Stoffel
2022-03-16 19:02 ` Juri Linkov
2022-03-16 20:25 ` Augusto Stoffel
2022-03-17 17:05 ` Juri Linkov
2022-03-17 19:06 ` Augusto Stoffel
2022-03-20 19:24 ` Juri Linkov
2022-03-20 19:59 ` Augusto Stoffel
2022-03-20 20:29 ` Juri Linkov
2022-03-20 20:56 ` Augusto Stoffel
2022-03-23 18:20 ` Juri Linkov
2022-03-23 18:54 ` Augusto Stoffel
2022-03-23 19:17 ` Eli Zaretskii
2022-03-23 19:53 ` Juri Linkov
2022-03-23 20:06 ` Juri Linkov
2022-03-23 20:30 ` Augusto Stoffel
2022-03-23 20:43 ` Juri Linkov
2022-03-17 19:45 ` Augusto Stoffel
2022-03-17 20:43 ` 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).