bug#32173: 26.1; wdired: broken 'wdired-use-interactive-rename'

[Stephen Berman <stephen.berman@gmx.net>]

On Mon, 16 Jul 2018 15:28:29 +0200 Enrico Scholz <enrico.scholz@ensc.de> wrote:

> Hi,
>
> wdired seems to misbehave when 'wdired-use-interactive-rename' is
> active:
>
> 1. create scratch directory with a file
>
>    mkdir /tmp/test
>    cd /tmp/test
>    touch foo.c
>
> 2. start emacs
>
>    LC_ALL=C emacs -Q -nw
>
> 3. set option above
>
>    M-: (setq wdired-use-interactive-rename t)
>
> 4. go into the folder
>
>    C-x C-f /tmp/test
>
>    emacs will show
>
>    | /tmp/test:
>    | total used in directory 0 available 4023272
>    | drwxrwxr-x.  2 ensc ensc  60 Jul 16 15:16 .
>    | drwxrwxrwt. 18 root root 600 Jul 16 15:17 ..
>    | -rw-rw-r--.  1 ensc ensc   0 Jul 16 14:58 foo.c
>
> 5. enter wdired mode
>
>    C-x C-q
>
> 6. replace 'foo' with 'test'; e.g.
>
>    test M-d
>
> 7. commit it
>
>    C-c C-c
>
>
> ---> emacs asks
>
> | Move `c' to `test.c'? [Type yn!q or C-h]
>
> or
>
> | Move `.' to `test.c'? [Type yn!q or C-h]
>
> (seems to differ slightly when repeating step 6).  Buffer content is
> malformed too (first two lines are merged, or whitespace between time
> and filename is removed).
>
>
> Confirming with 'y' will make the operation fail because the requested
> source file does not exist.
>
>
> Without the interactive rename, things seem to be fine.

The bug is in wdired-search-and-rename (called when
wdired-use-interactive-rename is non-nil) and is triggered by leaving
part of the original file name unmodified (in the above case, the
extension '.c' was not altered), which leaves the dired-filename text
property, which dired-move-to-filename, called by
wdired-search-and-rename, finds and moves point there, causing the
subsequent search for the file name to fail.  The patch below fixes this
bug, according to my tests, but maybe I overlooked some corner cases.
Can you try it and see if it works for you?

Steve Berman

diff --git a/lisp/wdired.el b/lisp/wdired.el
index bb60e77776..968aac0149 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -543,19 +543,41 @@ wdired-search-and-rename
     (goto-char (point-max))
     (forward-line -1)
     (let ((done nil)
+	  (failed t)
 	  curr-filename)
       (while (and (not done) (not (bobp)))
         (setq curr-filename (wdired-get-filename nil t))
         (if (equal curr-filename filename-ori)
-            (progn
-              (setq done t)
-              (let ((inhibit-read-only t))
-                (dired-move-to-filename)
-                (search-forward (wdired-get-filename t) nil t)
-                (replace-match (file-name-nondirectory filename-ori) t t))
-              (dired-do-create-files-regexp
-               (function dired-rename-file)
-               "Move" 1 ".*" filename-new nil t))
+	    (unwind-protect
+		(progn
+		  (setq done t)
+		  (let ((inhibit-read-only t))
+                    ;; If part of filename-ori is unmodified,
+                    ;; dired-move-to-filename moves point there, which
+                    ;; causes the search to fail (bug#32173).
+                    ;; Removing the dired-filename text property
+                    ;; prevents this (the text property is added again
+                    ;; when renaming succeeds).
+		    (remove-text-properties
+		     (line-beginning-position) (line-end-position)
+		     '(dired-filename nil))
+		    (dired-move-to-filename)
+		    (search-forward (wdired-get-filename t) nil t)
+		    (replace-match (file-name-nondirectory filename-ori) t t))
+		  (dired-do-create-files-regexp
+		   (function dired-rename-file)
+		   "Move" 1 ".*" filename-new nil t)
+		  (setq failed nil))
+              ;; If user quits before renaming succeeds, restore the
+              ;; dired-filename text property.
+	      (when failed
+		(beginning-of-line)
+		(let ((beg (re-search-forward
+			    directory-listing-before-filename-regexp
+			    (line-end-position) t))
+		      (end (dired-move-to-end-of-filename))
+		      (inhibit-read-only t))
+		  (add-text-properties beg end '(dired-filename t)))))
 	  (forward-line -1))))))
 
 ;; marks a list of files for deletion