Subject: Re: bug#14013: 24.3.50; dired-isearch-filenames-regexp is matching text outside filenames

From: Juri Linkov <[email-masked]>
Date: Wed, 23 Feb 2022 20:53:48 +0200

>> >> +  (add-function :around (local 'isearch-search-fun-function)
>> >> +                #'dired-isearch-search-filenames)
>> >
>> > And: Is it intended that this is unconditional (I would expect a
>> > `dired-isearch-filenames-mode' test)?
>>
>> Currently it is unconditional when it's using isearch-filter-predicate.
>> So maybe it should be kept this way since it's what was used for many years?
>> Or is there a reason to change the current behavior?
>
> Ok, then better let's keep it.

For more customizability I added a new user option
'wdired-search-replace-filenames' enabled by default.

> A different thing: I found that
>
> | + (setq-local replace-re-search-function #'dired-isearch-search-filenames)
>
> is not correct - of course, `dired-isearch-search-filenames' is a higher
> order function (used for the around advice), not something suitable for
> searching.  With the patch installed query-replace in wdired errors.
>
> What's the correct value - the current binding of
> `isearch-search-fun-function'?

You are right, the correct value is the current binding of
`isearch-search-fun-function'.  Everything is fixed now
in this patch, i.e. search/replace works ok in dired/wdired buffers
(except ^ at the beginning of filenames that is a separate feature):

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 56897826cb..92f2848334 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -3147,11 +3147,11 @@ dired-isearch-filenames-mode
 When off, it uses the original predicate."
   :lighter nil
   (if dired-isearch-filenames-mode
-      (add-function :before-while (local 'isearch-filter-predicate)
-                    #'dired-isearch-filter-filenames
+      (add-function :around (local 'isearch-search-fun-function)
+                    #'dired-isearch-search-filenames
                     '((isearch-message-prefix . "filename ")))
-    (remove-function (local 'isearch-filter-predicate)
-                     #'dired-isearch-filter-filenames))
+    (remove-function (local 'isearch-search-fun-function)
+                     #'dired-isearch-search-filenames))
   (when isearch-mode
     (setq isearch-success t isearch-adjusted t)
     (isearch-update)))
@@ -3175,12 +3175,42 @@ dired-isearch-filenames-end
   (unless isearch-suspended
     (kill-local-variable 'dired-isearch-filenames)))
 
-(defun dired-isearch-filter-filenames (beg end)
+(defun dired-isearch-search-filenames (orig-fun)
   "Test whether some part of the current search match is inside a file name.
 This function returns non-nil if some part of the text between BEG and END
 is part of a file name (i.e., has the text property `dired-filename')."
-  (text-property-not-all (min beg end) (max beg end)
-			 'dired-filename nil))
+  (let ((search-fun (funcall orig-fun)))
+    (lambda (string &optional bound noerror count)
+      (let ((old-pos (point))
+            (beg (when (get-text-property
+                        (if isearch-forward (point) (max (1- (point)) (point-min)))
+                        'dired-filename)
+                   (point)))
+            end found)
+        (unless beg
+          (setq beg (if isearch-forward
+                        (next-single-property-change (point) 'dired-filename)
+                      (previous-single-property-change (point) 'dired-filename)))
+          (when beg (goto-char beg)))
+        (while (and beg (not found))
+          (setq end (if isearch-forward
+                        (next-single-property-change beg 'dired-filename)
+                      (previous-single-property-change beg 'dired-filename)))
+          (if (not end)
+              (setq beg nil)
+            (setq found (funcall
+                         search-fun string (if bound (if isearch-forward
+                                                         (min bound end)
+                                                       (max bound end))
+                                             end)
+                         noerror count))
+            (unless found
+              (setq beg (if isearch-forward
+                            (next-single-property-change end 'dired-filename)
+                          (previous-single-property-change end 'dired-filename)))
+              (when beg (goto-char beg)))))
+        (unless found (goto-char old-pos))
+        found))))
 
 ;;;###autoload
 (defun dired-isearch-filenames ()
diff --git a/lisp/wdired.el b/lisp/wdired.el
index ab3b91bbe5..229a266d33 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -155,6 +155,11 @@ wdired-create-parent-directories
   :version "26.1"
   :type 'boolean)
 
+(defcustom wdired-search-replace-filenames t
+  "Non-nil to search and replace in file names only."
+  :version "29.1"
+  :type 'boolean)
+
 (defvar-keymap wdired-mode-map
   :doc "Keymap used in `wdired-mode'."
   "C-x C-s" #'wdired-finish-edit
@@ -217,6 +222,7 @@ wdired-mode
   (error "This mode can be enabled only by `wdired-change-to-wdired-mode'"))
 (put 'wdired-mode 'mode-class 'special)
 
+(declare-function dired-isearch-search-filenames "dired-aux")
 
 ;;;###autoload
 (defun wdired-change-to-wdired-mode ()
@@ -237,9 +243,12 @@ wdired-change-to-wdired-mode
               (dired-remember-marks (point-min) (point-max)))
   (setq-local wdired--old-point (point))
   (wdired--set-permission-bounds)
-  (setq-local query-replace-skip-read-only t)
-  (add-function :after-while (local 'isearch-filter-predicate)
-                #'wdired-isearch-filter-read-only)
+  (when wdired-search-replace-filenames
+    (add-function :around (local 'isearch-search-fun-function)
+                  #'dired-isearch-search-filenames
+                  '((isearch-message-prefix . "filename ")))
+    (setq-local replace-search-function
+    (setq-local replace-re-search-function (funcall isearch-search-fun-function))))
   (use-local-map wdired-mode-map)
   (force-mode-line-update)
   (setq buffer-read-only nil)
@@ -319,11 +328,6 @@ wdired--before-change-fn
             ;; Is this good enough? Assumes no extra white lines from dired.
             (put-text-property (1- (point-max)) (point-max) 'read-only t)))))))
 
-(defun wdired-isearch-filter-read-only (beg end)
-  "Skip matches that have a read-only property."
-  (not (text-property-not-all (min beg end) (max beg end)
-			      'read-only nil)))
-
 ;; Protect the buffer so only the filenames can be changed, and put
 ;; properties so filenames (old and new) can be easily found.
 (defun wdired--preprocess-files ()
@@ -438,8 +442,11 @@ wdired-change-to-dired-mode
     (remove-text-properties
      (point-min) (point-max)
      '(front-sticky nil rear-nonsticky nil read-only nil keymap nil)))
-  (remove-function (local 'isearch-filter-predicate)
-                   #'wdired-isearch-filter-read-only)
+  (when wdired-search-replace-filenames
+    (remove-function (local 'isearch-search-fun-function)
+                     #'dired-isearch-search-filenames)
+    (kill-local-variable 'replace-search-function)
+    (kill-local-variable 'replace-re-search-function))
   (use-local-map dired-mode-map)
   (force-mode-line-update)
   (setq buffer-read-only t)