bug#12394: 24.2.50; wdired: error when wdired-use-interactive-rename is non-nil

[Michael Heerdegen]

Please describe exactly what actions triggered the bug, and
the precise symptoms of the bug.  If you can, give a recipe
starting from `emacs -Q':

Hello,

when you have set `wdired-use-interactive-rename' to non-nil, and you
add something to the front of a filename when in wdired-mode, you get
an error after hitting C-c C-c.  Here is a recipe for emacs -Q:

- dired a directory where you are allowed to change file names
- M-: (progn (require 'wdired) (setq wdired-use-interactive-rename t)) RET
- M-x wdired-change-to-wdired-mode
- go to any file
- prepend any string to its name
- C-c C-c

This will fail either with an error, or an attempt to rename a
non-existent file (whose name is a substring of the file you actually
wanted to rename).

Here is a backtrace where I got an error:

Debugger entered--Lisp error: (args-out-of-range 0 0)
  replace-match("newlines.el" t 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))
  (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))
  (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)) (forward-line -1))
  (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)) (forward-line -1)))
  (let ((done nil) 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)) (forward-line -1))))
  (save-excursion (goto-char (point-max)) (forward-line -1) (let ((done nil) 
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)) (forward-line 
-1)))))
  wdired-search-and-rename("/home/micha/today/Test/newlines.el" 
"/home/micha/today/Test/znewlines.el")
  (if wdired-use-interactive-rename (wdired-search-and-rename file-ori 
file-new) (require (quote dired-aux)) (condition-case err (let 
((dired-backup-overwrite nil)) (dired-rename-file file-ori file-new overwrite)) 
(error (setq errors (1+ errors)) (dired-log (concat "Rename `" file-ori "' to 
`" file-new "' failed:\n%s\n") err))))
  (let ((file-ori (car rename))) (if wdired-use-interactive-rename 
(wdired-search-and-rename file-ori file-new) (require (quote dired-aux)) 
(condition-case err (let ((dired-backup-overwrite nil)) (dired-rename-file 
file-ori file-new overwrite)) (error (setq errors (1+ errors)) (dired-log 
(concat "Rename `" file-ori "' to `" file-new "' failed:\n%s\n") err)))))
  (cond ((rassoc file-new renames) (error "Trying to rename 2 files to the same 
name")) ((assoc file-new renames) (setq residue (cons rename residue))) ((and 
(assoc file-new residue) (file-exists-p file-new)) (if (or progress renames) 
(setq residue (cons rename residue)) (message "Circular renaming: using 
temporary file name") (let ((tmp (make-temp-name file-new))) (setq renames 
(cons (cons (car rename) tmp) renames)) (setq residue (cons (cons tmp file-new) 
residue))))) (t (setq progress t) (let ((file-ori (car rename))) (if 
wdired-use-interactive-rename (wdired-search-and-rename file-ori file-new) 
(require (quote dired-aux)) (condition-case err (let ((dired-backup-overwrite 
nil)) (dired-rename-file file-ori file-new overwrite)) (error (setq errors (1+ 
errors)) (dired-log (concat "Rename `" file-ori "' to `" file-new "' 
failed:\n%s\n") err)))))))
  (let* ((rename (car (prog1 renames (setq renames (cdr renames))))) (file-new 
(cdr rename))) (cond ((rassoc file-new renames) (error "Trying to rename 2 
files to the same name")) ((assoc file-new renames) (setq residue (cons rename 
residue))) ((and (assoc file-new residue) (file-exists-p file-new)) (if (or 
progress renames) (setq residue (cons rename residue)) (message "Circular 
renaming: using temporary file name") (let ((tmp (make-temp-name file-new))) 
(setq renames (cons (cons ... tmp) renames)) (setq residue (cons (cons tmp 
file-new) residue))))) (t (setq progress t) (let ((file-ori (car rename))) (if 
wdired-use-interactive-rename (wdired-search-and-rename file-ori file-new) 
(require (quote dired-aux)) (condition-case err (let (...) (dired-rename-file 
file-ori file-new overwrite)) (error (setq errors ...) (dired-log ... 
err))))))))
  (while (or renames (prog1 (setq renames residue) (setq progress nil) (setq 
residue nil))) (let* ((rename (car (prog1 renames (setq renames (cdr 
renames))))) (file-new (cdr rename))) (cond ((rassoc file-new renames) (error 
"Trying to rename 2 files to the same name")) ((assoc file-new renames) (setq 
residue (cons rename residue))) ((and (assoc file-new residue) (file-exists-p 
file-new)) (if (or progress renames) (setq residue (cons rename residue)) 
(message "Circular renaming: using temporary file name") (let ((tmp ...)) (setq 
renames (cons ... renames)) (setq residue (cons ... residue))))) (t (setq 
progress t) (let ((file-ori (car rename))) (if wdired-use-interactive-rename 
(wdired-search-and-rename file-ori file-new) (require (quote dired-aux)) 
(condition-case err (let ... ...) (error ... ...))))))))
  (let ((residue nil) (progress nil) (errors 0) (overwrite (or (not 
wdired-confirm-overwrite) 1))) (while (or renames (prog1 (setq renames residue) 
(setq progress nil) (setq residue nil))) (let* ((rename (car (prog1 renames 
(setq renames ...)))) (file-new (cdr rename))) (cond ((rassoc file-new renames) 
(error "Trying to rename 2 files to the same name")) ((assoc file-new renames) 
(setq residue (cons rename residue))) ((and (assoc file-new residue) 
(file-exists-p file-new)) (if (or progress renames) (setq residue (cons rename 
residue)) (message "Circular renaming: using temporary file name") (let (...) 
(setq renames ...) (setq residue ...)))) (t (setq progress t) (let ((file-ori 
...)) (if wdired-use-interactive-rename (wdired-search-and-rename file-ori 
file-new) (require ...) (condition-case err ... ...))))))) errors)
  wdired-do-renames((("/home/micha/today/Test/newlines.el" . 
"/home/micha/today/Test/znewlines.el")))
  (+ errors (wdired-do-renames files-renamed))
  (setq errors (+ errors (wdired-do-renames files-renamed)))
  (progn (setq errors (+ errors (wdired-do-renames files-renamed))))
  (if files-renamed (progn (setq errors (+ errors (wdired-do-renames 
files-renamed)))))
  (let ((changes nil) (errors 0) files-deleted files-renamed 
some-file-names-unchanged file-old file-new tmp-value) (save-excursion (if (and 
wdired-allow-to-redirect-links (fboundp (quote make-symbolic-link))) (progn 
(setq tmp-value (wdired-do-symlink-changes)) (setq errors (cdr tmp-value)) 
(setq changes (car tmp-value)))) (if (and wdired-allow-to-change-permissions 
(boundp (quote wdired-col-perm))) (progn (setq tmp-value 
(wdired-do-perm-changes)) (setq errors (+ errors (cdr tmp-value))) (setq 
changes (or changes (car tmp-value))))) (goto-char (point-max)) (while (not 
(bobp)) (setq file-old (wdired-get-filename nil t)) (if file-old (progn (setq 
file-new (wdired-get-filename)) (if (equal file-new file-old) (setq 
some-file-names-unchanged t) (setq changes t) (if (not file-new) (setq 
files-deleted ...) (setq files-renamed ...))))) (forward-line -1))) (if 
files-renamed (progn (setq errors (+ errors (wdired-do-renames 
files-renamed))))) (if changes (progn (if (and (stringp dired-directory) (not 
(file-directory-p dired-directory)) (null some-file-names-unchanged) (= (length 
files-renamed) 1)) (progn (setq dired-directory (cdr (car files-renamed))))) 
(revert-buffer)) (let ((inhibit-read-only t)) (remove-text-properties 
(point-min) (point-max) (quote (old-name nil end-name nil old-link nil end-link 
nil end-perm nil old-perm nil perm-changed nil))) (message "(No changes to be 
performed)"))) (if files-deleted (progn (wdired-flag-for-deletion 
files-deleted))) (if (> errors 0) (progn (dired-log-summary (format "%d rename 
actions failed" errors) nil))))
  wdired-finish-edit()
  call-interactively(wdired-finish-edit nil nil)

I located the problem in `wdired-search-and-rename':

(defun wdired-search-and-rename (filename-ori filename-new)
  (save-excursion
    (goto-char (point-max))
    (forward-line -1)
    (let ((done nil)
          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)  ;; <--------------- here
                (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))
          (forward-line -1))))))

It is an error to use `dired-move-to-filename' in this context.
`dired-move-to-filename' looks for text with the `dired-filename' text
property.  But this is wrong if you have added text to the beginning
of a filename, because then, the first appearance of the
`dired-filename' text property is not the beginning of the edited
filename.  In the backtrace above, the `search-forward' fails, and
thus `replace-match' uses invalid match-data.


Fix: The following patch works for me:

patch.diff
Description: Text Data

It replaces the `dired-move-to-filename' call with this:

(goto-char (1+ (next-single-property-change
  (point) 'old-name nil (line-end-position))))

This works because when you invoke `wdired-change-to-wdired-mode',
`wdired-preprocess-files' is called and puts the text-property
`old-name' to the character _before_ the beginning of the (unedited)
filename.  So, whatever changes you make, the (edited) filename always
starts directly after the character with this property.  This is how
`wdired-get-filename' works.

BTW: I found this problem while writing a patch for bug#11795.  Those
bugs are completely unrelated, but I would prefer if this problem here
could be fixed first.


Thanks,

Michael.


If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
For information about debugging Emacs, please read the file
/usr/share/emacs/24.2.50/etc/DEBUG.


In GNU Emacs 24.2.50.1 (i486-pc-linux-gnu, GTK+ Version 3.4.2)
 of 2012-09-05 on dex, modified by Debian
 (emacs-snapshot package, version 2:20120905-1)
Windowing system distributor `The X.Org Foundation', version 11.0.11203000
Configured using:
 `configure '--build' 'i486-linux-gnu' '--host' 'i486-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 
'--enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/24.2.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.2.50/site-lisp:/usr/share/emacs/site-lisp'
 '--without-compress-info' '--with-crt-dir=/usr/lib/i386-linux-gnu/'
 '--with-x=yes' '--with-x-toolkit=gtk3' '--with-imagemagick=yes'
 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu'
 'CFLAGS=-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000 -g -O2' 'LDFLAGS=-g
 -Wl,--as-needed -znocombreloc' 'CPPFLAGS=-D_FORTIFY_SOURCE=2''