Subject: Re: bug#73018: 31.0.50; wdired + replace-regexp only modifies the visible portion of the buffer

From: Michael Heerdegen <[email-masked]>
Date: Mon, 09 Sep 2024 16:55:59 +0200

Juri Linkov <[email-masked]> writes:

> M-: (buffer-substring (- (point-max) 2) (- (point-max) 1))
> => #("7" 0 1 (fontified nil invisible dired-hide-details-link))
>
> M-> ;; (end-of-buffer)
>
> M-: (buffer-substring (- (point-max) 2) (- (point-max) 1))
> => #("7" 0 1 (face default dired-symlink-filename t fontified t
> invisible dired-hide-details-link))
>
> And indeed, after going to the end of the Dired buffer
> the last file gets an additional property `dired-symlink-filename'
> used by Isearch/Replace.

So, the properties we use are of different types: some are already
attached by `dired-insert-set-properties', others later by font-lock
(see `dired-font-lock-keywords').  AFAIR this is because some tests are
more expensive than others (e.g. the test whether a link is broken) and
are intentionally delayed.

Would something like this be good?

From c5ef8c8db7d8b2ab03998a0f3f2a1b820ff24e2a Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <[email-masked]>
Date: Mon, 9 Sep 2024 16:46:13 +0200
Subject: [PATCH] WIP: Bug#73018

---
 lisp/dired-aux.el | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index d79ec342435..77dde7147bc 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -3740,8 +3740,12 @@ dired-isearch-search-filenames
 The returned function narrows the search to match the search string
 only as part of a file name enclosed by the text property `dired-filename'.
 It's intended to override the default search function."
-  (isearch-search-fun-in-text-property
-   (funcall orig-fun) '(dired-filename dired-symlink-filename)))
+  (let ((search-fun
+         (isearch-search-fun-in-text-property
+          (funcall orig-fun) '(dired-filename dired-symlink-filename))))
+    (lambda (&rest args)
+      (font-lock-ensure)
+      (apply search-fun args))))

 ;;;###autoload
 (defun dired-isearch-filenames ()
--
2.39.2

A related question is whether everybody always wants to search in
symlink targets when isearching file names in dired... I don't.  Would
it be worth to add an option for that?  Currently the properties are
just hardcoded.

Then, in the above patch we could make the `font-lock-ensure' call
depend on the value of that option.

> Also noticed that doing the first replacement always raises an error:
>
> Debugger entered--Lisp error: (error "Match data clobbered by buffer
> modification hooks")
>   replace-match("!" nil nil)
>   replace-match-maybe-edit("!" nil nil nil (672 673 #<buffer char>) nil)
>   perform-replace("7" "!" t t nil nil nil nil nil nil nil)
>   query-replace-regexp("7" "!" nil nil nil nil nil)
>   funcall-interactively(query-replace-regexp "7" "!" nil nil nil nil nil)
>   command-execute(query-replace-regexp)

Do I interpret the code in replace_match correctly: this error doesn't
even mean the match data has been clobbered - only that modification
hooks called searching functions?  I don't know what the referenced
search_regs.num_regs exactly contains.  But we already seem to ensure
not to clobber match data.

Michael.