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

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

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 10590 bytes --]

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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch.diff --]
[-- Type: text/x-diff, Size: 978 bytes --]

*** /home/micha/today/wdired-change-2/wdired.el	2012-09-08 18:47:46.000000000 +0200
--- /home/micha/today/wdired-change-2/wdired-patched.el	2012-09-08 18:47:16.000000000 +0200
***************
*** 506,512 ****
              (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
--- 506,513 ----
              (progn
                (setq done t)
                (let ((inhibit-read-only t))
!                 (goto-char (1+ (next-single-property-change
!                                 (point) 'old-name nil (line-end-position))))
                  (search-forward (wdired-get-filename t) nil t)
                  (replace-match (file-name-nondirectory filename-ori) t t))
                (dired-do-create-files-regexp

[-- Attachment #3: Type: text/plain, Size: 1959 bytes --]


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''

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

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> 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.

But wait.  There are many more problems with
`wdired-use-interactive-rename' non-nil.

All in emacs -Q:

1.  When I hit ! at the prompt after hitting C-c C-c to perform changes,
this has not the desired effect.  Emacs still asks me one time for every
remaining renaming action.

2.  If I hit n when Emacs prompts, I get this error:

dired-create-files: No buffer named *Dired log*

3.  I also noticed that `wdired-use-interactive-rename' non-nil is not
usable at all when circular renamings are to be done.  You end up with
files renamed to a temporary name.  E.g. try to switch the names of two
files, and see what you get.

To sum up, setting `wdired-use-interactive-rename' to a non-nil value
breaks many many things.

I also think that the questions you would get when performing circular
renamings would confuse many users, if we would get it work.

In general, IMHO `wdired-use-interactive-rename' is not very useful at
all.  I never used it, and the fact that all these problems were never
reported by anyone before indicates that not many people are using it.

I suggest to remove it completely.  Instead, it would be better to
ask the user in a way similar to e.g. C in dired, like

Really perform these renamings?

a -> b
b -> a
file-x -> file-y
...

But I'm open to other suggestions.  Any opinions?


Thanks,

Michael.

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

[Marcin Borkowski]

On 2012-09-09, at 17:19, Michael Heerdegen <michael_heerdegen@web.de> wrote:

> 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).

Confirmed in GNU Emacs 25.1.50.1.

-- 
mb

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

[Marcin Borkowski]

On 2012-09-11, at 21:40, Michael Heerdegen <michael_heerdegen@web.de> wrote:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> 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.
>
> But wait.  There are many more problems with
> `wdired-use-interactive-rename' non-nil.
>
> All in emacs -Q:
>
> 1.  When I hit ! at the prompt after hitting C-c C-c to perform changes,
> this has not the desired effect.  Emacs still asks me one time for every
> remaining renaming action.

Could not reproduce in GNU Emacs 25.1.50.1 - got an error even before
the prompt.

> In general, IMHO `wdired-use-interactive-rename' is not very useful at
> all.  I never used it, and the fact that all these problems were never
> reported by anyone before indicates that not many people are using it.
>
> I suggest to remove it completely.  Instead, it would be better to
> ask the user in a way similar to e.g. C in dired, like
>
> Really perform these renamings?
>
> a -> b
> b -> a
> file-x -> file-y
> ...
>
> But I'm open to other suggestions.  Any opinions?

+1.

> Thanks,
>
> Michael.

--
mb

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

[martin rudalics]

 >> In general, IMHO `wdired-use-interactive-rename' is not very useful at
 >> all.  I never used it, and the fact that all these problems were never
 >> reported by anyone before indicates that not many people are using it.
 >>
 >> I suggest to remove it completely.

IIUC this was broken by the introduction of the 'dired-filename' text
property.  There's hardly anything reasonable we can do about this.
That property should be front-sticky but wdired removes all text
properties it creates before doing the renaming so this seems hardly
reliable.

So we probably should obsolete ‘wdired-use-interactive-rename’ and not
call ‘wdired-search-and-rename’ any more in ‘wdired-do-renames’ in the
Emacs-25 branch.

 >> Instead, it would be better to
 >> ask the user in a way similar to e.g. C in dired, like
 >>
 >> Really perform these renamings?
 >>
 >> a -> b
 >> b -> a
 >> file-x -> file-y
 >> ...
 >>
 >> But I'm open to other suggestions.  Any opinions?
 >
 > +1.

Sounds TRT to me too.

martin

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

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> 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).

I'm unable to reproduce this bug on the master, so I'm going to go ahead
and guess that it's been fixed at some point in the eight years since
this was reported.  If the bug still exists, please respond to the
debbugs address, and we'll reopen.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no