* bug#55110: 29.0.50; Regression in query-replace prompt @ 2022-04-25 15:37 Juri Linkov 2022-04-26 19:42 ` Augusto Stoffel 0 siblings, 1 reply; 5+ messages in thread From: Juri Linkov @ 2022-04-25 15:37 UTC (permalink / raw) To: 55110; +Cc: augusto stoffel X-Debbugs-Cc: Augusto Stoffel <arstoffel@gmail.com> I noticed a recent regression after minibuffer-lazy-highlight changes: 0. emacs -Q 1. perform one query-replace to add some FROM and TO to its history: M-% a RET b RET 2. check that isearch still works in the minibuffer history: M-% C-r a 3. activate the region, e.g. C-SPC and move point 4. try to isearch in the history again: M-% C-r a It fails with: [Failing I-search backward: a] Maybe this is caused by minibuffer-lazy-highlight-setup that sets filter to replace--region-filter in the minibuffer instead of the original buffer? ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#55110: 29.0.50; Regression in query-replace prompt 2022-04-25 15:37 bug#55110: 29.0.50; Regression in query-replace prompt Juri Linkov @ 2022-04-26 19:42 ` Augusto Stoffel 2022-04-27 7:44 ` Juri Linkov 0 siblings, 1 reply; 5+ messages in thread From: Augusto Stoffel @ 2022-04-26 19:42 UTC (permalink / raw) To: Juri Linkov; +Cc: 55110 Thanks for the heads up, I'll take a look at this some time soon. On Mon, 25 Apr 2022 at 18:37, Juri Linkov <juri@linkov.net> wrote: > Maybe this is caused by minibuffer-lazy-highlight-setup > that sets filter to replace--region-filter in the minibuffer > instead of the original buffer? Most likely, yes. `replace--region-filter' is modified globally, so a similar problem should happen if you temporarily leave the minibuffer and do Isearch in any other buffer. If that's the case, I think we would have two options: 1) Add a quick fix for the minibuffer Isearch only. 2) A more complicated change that solves the issue generally by saving the region filter in the fashion of isearch-lazy-highlight-regexp et alii. WDYT? ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#55110: 29.0.50; Regression in query-replace prompt 2022-04-26 19:42 ` Augusto Stoffel @ 2022-04-27 7:44 ` Juri Linkov 2022-05-14 16:02 ` Augusto Stoffel 0 siblings, 1 reply; 5+ messages in thread From: Juri Linkov @ 2022-04-27 7:44 UTC (permalink / raw) To: Augusto Stoffel; +Cc: 55110 >> Maybe this is caused by minibuffer-lazy-highlight-setup >> that sets filter to replace--region-filter in the minibuffer >> instead of the original buffer? > > Most likely, yes. `replace--region-filter' is modified globally, so a > similar problem should happen if you temporarily leave the minibuffer > and do Isearch in any other buffer. > > If that's the case, I think we would have two options: > > 1) Add a quick fix for the minibuffer Isearch only. > > 2) A more complicated change that solves the issue generally by saving > the region filter in the fashion of isearch-lazy-highlight-regexp et > alii. > > WDYT? Recently we fixed a similar problem in `perform-replace' by creating a dynamically bound value in `let': (let ((opos (point-marker)) ;; Restore original isearch filter to allow ;; using isearch in a recursive edit even ;; when perform-replace was started from ;; `xref--query-replace-1' that let-binds ;; `isearch-filter-predicate' (bug#53758). (isearch-filter-predicate #'isearch-filter-visible)) So maybe a buffer-local value of `isearch-filter-predicate' in the minibuffer would help. Also I recommend to make all hooks in `minibuffer-lazy-highlight-setup' local by adding the argument LOCAL to add-hook/remove-hook. ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#55110: 29.0.50; Regression in query-replace prompt 2022-04-27 7:44 ` Juri Linkov @ 2022-05-14 16:02 ` Augusto Stoffel 2022-05-15 18:45 ` Juri Linkov 0 siblings, 1 reply; 5+ messages in thread From: Augusto Stoffel @ 2022-05-14 16:02 UTC (permalink / raw) To: Juri Linkov; +Cc: 55110 [-- Attachment #1: Type: text/plain, Size: 1944 bytes --] On Wed, 27 Apr 2022 at 10:44, Juri Linkov <juri@linkov.net> wrote: >>> Maybe this is caused by minibuffer-lazy-highlight-setup >>> that sets filter to replace--region-filter in the minibuffer >>> instead of the original buffer? >> >> Most likely, yes. `replace--region-filter' is modified globally, so a >> similar problem should happen if you temporarily leave the minibuffer >> and do Isearch in any other buffer. >> >> If that's the case, I think we would have two options: >> >> 1) Add a quick fix for the minibuffer Isearch only. >> >> 2) A more complicated change that solves the issue generally by saving >> the region filter in the fashion of isearch-lazy-highlight-regexp et >> alii. >> >> WDYT? > > Recently we fixed a similar problem in `perform-replace' > by creating a dynamically bound value in `let': > > (let ((opos (point-marker)) > ;; Restore original isearch filter to allow > ;; using isearch in a recursive edit even > ;; when perform-replace was started from > ;; `xref--query-replace-1' that let-binds > ;; `isearch-filter-predicate' (bug#53758). > (isearch-filter-predicate #'isearch-filter-visible)) > > So maybe a buffer-local value of `isearch-filter-predicate' > in the minibuffer would help. Yes, that indeed solves the problem. See attached patch. > Also I recommend to make all hooks in `minibuffer-lazy-highlight-setup' > local by adding the argument LOCAL to add-hook/remove-hook. Indeed, the minibuffer lazy highlight feature is currently incompatible with recursive minibuffers. The patch fixes that as well. There's a caveat, though: isearch in a recursive minibuffer is again affected by the presence of an inappropriate filter function. Fixing that in a robust way might require a bigger refactoring of the lazy highlight feature, I think. Another option might be to make `replace--region-filter' also check for the value of `(current-buffer)'. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Make-minibuffer-lazy-highlight-setup-buffer-local-wh.patch --] [-- Type: text/x-patch, Size: 1599 bytes --] From 37de4e53fce6adbf5e52f3184c163574e3129def Mon Sep 17 00:00:00 2001 From: Augusto Stoffel <arstoffel@gmail.com> Date: Sat, 14 May 2022 17:21:27 +0200 Subject: [PATCH] Make minibuffer lazy highlight setup buffer-local where appropriate * lisp/isearch.el (minibuffer-lazy-highlight-setup): Modify hooks buffer-locally, so that recursive minibuffers are not affected by the special behavior of lazy-highlight. Also make 'isearch-filter-predicate' buffer-local, so that isearch in the minibuffer is not affected by the region filter (bug#55110). --- lisp/isearch.el | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lisp/isearch.el b/lisp/isearch.el index 96168f94bd..3e1dab4d15 100644 --- a/lisp/isearch.el +++ b/lisp/isearch.el @@ -4441,12 +4441,13 @@ minibuffer-lazy-highlight-setup (format minibuffer-lazy-count-format isearch-lazy-count-total))))) (lambda () - (add-hook 'minibuffer-exit-hook unwind) - (add-hook 'after-change-functions after-change) + (add-hook 'minibuffer-exit-hook unwind nil t) + (add-hook 'after-change-functions after-change nil t) (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 + (make-local-variable 'isearch-filter-predicate) (add-function :after-while isearch-filter-predicate filter)) (funcall after-change nil nil nil))))) -- 2.36.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* bug#55110: 29.0.50; Regression in query-replace prompt 2022-05-14 16:02 ` Augusto Stoffel @ 2022-05-15 18:45 ` Juri Linkov 0 siblings, 0 replies; 5+ messages in thread From: Juri Linkov @ 2022-05-15 18:45 UTC (permalink / raw) To: Augusto Stoffel; +Cc: 55110 >> So maybe a buffer-local value of `isearch-filter-predicate' >> in the minibuffer would help. > > Yes, that indeed solves the problem. See attached patch. Thanks, pushed. >> Also I recommend to make all hooks in `minibuffer-lazy-highlight-setup' >> local by adding the argument LOCAL to add-hook/remove-hook. > > Indeed, the minibuffer lazy highlight feature is currently incompatible > with recursive minibuffers. The patch fixes that as well. > > There's a caveat, though: isearch in a recursive minibuffer is again > affected by the presence of an inappropriate filter function. Fixing > that in a robust way might require a bigger refactoring of the lazy > highlight feature, I think. Another option might be to make > `replace--region-filter' also check for the value of `(current-buffer)'. I see no problem to fix this because every minibuffer uses own buffer, so they have separate buffer-local variables. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-15 18:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-04-25 15:37 bug#55110: 29.0.50; Regression in query-replace prompt Juri Linkov 2022-04-26 19:42 ` Augusto Stoffel 2022-04-27 7:44 ` Juri Linkov 2022-05-14 16:02 ` Augusto Stoffel 2022-05-15 18:45 ` Juri Linkov
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.