GNU bug report logs - #11795
24.1.50; wdired: C-c C-c loses marks and positions of renamed files

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11795 in the body.
You can then email your comments to 11795 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.1.50; wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 27 Jun 2012 01:53:07 +0200

Hi,

a short demonstration for emacs -Q:

1. M-x dired a directory containing some files

2. Go to the first file's line (not "." or ".."), and mark the file
with m

3. M-x wdired-change-to-wdired-mode

4. Edit the name of the first file: e.g. add a "z" to the front of the
   name so that the file will surely appear at another position after
   renaming

5. C-c C-c

You'll see that point is still at the first file's line, although the
renamed file is now displayed near the end of the buffer.  Even worse,
the mark of the file has disappeared.

Background: `wdired-finish-edit' calls `revert-buffer', which will
call `dired-revert'.  Before this is called, everything is ok.  But
`dired-revert' fails to remember the mark and the position of the
renamed file correctly.  I didn't dig any further, but I guess a
specialized revert function is needed for `wdired-finish-edit'.


Thanks,

Michael.


In GNU Emacs 24.1.50.1 (i486-pc-linux-gnu, GTK+ Version 3.4.2)
 of 2012-06-22 on zelenka, modified by Debian
 (emacs-snapshot package, version 2:20120622-1)
Windowing system distributor `The X.Org Foundation', version 11.0.11201902
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.1.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.1.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''

Message #8 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: <michael_heerdegen <at> web.de>, <11795 <at> debbugs.gnu.org>
Subject: RE: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Tue, 26 Jun 2012 17:57:35 -0700

Hi Michael,

I haven't looked at the bug report much, but are you aware of user option
`dired-keep-marker-rename'.  Dunno whether it is involve here, but it generally
controls whether a marked file keeps its mark after renaming.

HTH - Drew

> Background: `wdired-finish-edit' calls `revert-buffer', which will
> call `dired-revert'.  Before this is called, everything is ok.  But
> `dired-revert' fails to remember the mark and the position of the
> renamed file correctly.  I didn't dig any further, but I guess a
> specialized revert function is needed for `wdired-finish-edit'.

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Schmidt <christopher <at> ch.ristopher.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 27 Jun 2012 02:27:00 +0100 (BST)

[Message part 1 (text/plain, inline)]

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Background: `wdired-finish-edit' calls `revert-buffer', which will
> call `dired-revert'.  Before this is called, everything is ok.  But
> `dired-revert' fails to remember the mark and the position of the
> renamed file correctly.  I didn't dig any further, but I guess a
> specialized revert function is needed for `wdired-finish-edit'.

This patch for wdired.el fixes the problem here (GNU Emacs 24.1.1).

[wdired-finish-edit-remeber-marks.diff (text/x-diff, inline)]

diff --git a/wdired.el b/wdired-fixed.el
index fe5b4ef..d241d2e 100644
--- a/wdired.el
+++ b/wdired-fixed.el
@@ -350,7 +350,8 @@ non-nil means return old filename."
   (interactive)
   (wdired-change-to-dired-mode)
   (let ((changes nil)
-	(errors 0)
+	(errors 0)	
+	(marks (dired-remember-marks (point-min) (point-max)))
 	files-deleted
 	files-renamed
 	some-file-names-unchanged
@@ -376,6 +377,9 @@ non-nil means return old filename."
             (setq changes t)
             (if (not file-new)		;empty filename!
                 (push file-old files-deleted)
+	      (push (cons (substitute-in-file-name file-new)
+			  (cdr (assoc file-old marks)))
+		    marks)
               (push (cons file-old (substitute-in-file-name file-new))
                     files-renamed))))
 	(forward-line -1)))
@@ -393,7 +397,8 @@ non-nil means return old filename."
 		     (= (length files-renamed) 1))
 	    (setq dired-directory (cdr (car files-renamed))))
 	  ;; Re-sort the buffer.
-	  (revert-buffer))
+	  (revert-buffer)
+	  (dired-mark-remembered marks))
       (let ((inhibit-read-only t))
 	(remove-text-properties (point-min) (point-max)
 				'(old-name nil end-name nil old-link nil

[Message part 3 (text/plain, inline)]

        Christopher

Message #14 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Christopher Schmidt'" <christopher <at> ristopher.com>,
	<11795 <at> debbugs.gnu.org>
Subject: RE: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 27 Jun 2012 07:20:13 -0700

> > Background: `wdired-finish-edit' calls `revert-buffer', which will
> > call `dired-revert'.  Before this is called, everything is ok.  But
> > `dired-revert' fails to remember the mark and the position of the
> > renamed file correctly.  I didn't dig any further, but I guess a
> > specialized revert function is needed for `wdired-finish-edit'.
> 
> This patch for wdired.el fixes the problem here (GNU Emacs 24.1.1).

But shouldn't Wdired respect the user's setting of option
`dired-keep-marker-rename'?  IOW, shouldn't users be able to control whether
exiting wdired removes marks?

Message #17 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Schmidt <christopher <at> ch.ristopher.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 27 Jun 2012 15:47:18 +0100 (BST)

[Message part 1 (text/plain, inline)]

"Drew Adams" <drew.adams <at> oracle.com> writes:

>> This patch for wdired.el fixes the problem here (GNU Emacs 24.1.1).
>
> But shouldn't Wdired respect the user's setting of option
> `dired-keep-marker-rename'?  IOW, shouldn't users be able to control
> whether exiting wdired removes marks?

Right.

[wdired-finish-edit-remember-marks.diff (text/x-diff, inline)]

diff --git a/wdired.el b/wdired-fixed.el
index fe5b4ef..c5956c5 100644
--- a/wdired.el
+++ b/wdired-fixed.el
@@ -351,6 +351,7 @@ non-nil means return old filename."
   (wdired-change-to-dired-mode)
   (let ((changes nil)
 	(errors 0)
+	(marks (dired-remember-marks (point-min) (point-max)))
 	files-deleted
 	files-renamed
 	some-file-names-unchanged
@@ -376,6 +377,12 @@ non-nil means return old filename."
             (setq changes t)
             (if (not file-new)		;empty filename!
                 (push file-old files-deleted)
+	      (when dired-keep-marker-rename
+		(push (cons (substitute-in-file-name file-new)
+			    (if (integerp dired-keep-marker-rename)
+				dired-keep-marker-rename
+			      (cdr (assoc file-old marks))))
+		      marks))
               (push (cons file-old (substitute-in-file-name file-new))
                     files-renamed))))
 	(forward-line -1)))
@@ -393,7 +400,9 @@ non-nil means return old filename."
 		     (= (length files-renamed) 1))
 	    (setq dired-directory (cdr (car files-renamed))))
 	  ;; Re-sort the buffer.
-	  (revert-buffer))
+	  (revert-buffer)
+	  (let ((inhibit-read-only t))
+	    (dired-mark-remembered marks)))
       (let ((inhibit-read-only t))
 	(remove-text-properties (point-min) (point-max)
 				'(old-name nil end-name nil old-link nil

[Message part 3 (text/plain, inline)]

        Christopher

Message #20 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Christopher Schmidt'" <christopher <at> ristopher.com>,
	<11795 <at> debbugs.gnu.org>
Subject: RE: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 27 Jun 2012 07:59:56 -0700

> >> This patch for wdired.el fixes the problem here (GNU Emacs 24.1.1).
> >
> > But shouldn't Wdired respect the user's setting of option
> > `dired-keep-marker-rename'?  IOW, shouldn't users be able to control
> > whether exiting wdired removes marks?
> 
> Right.

Looks OK to me.

A question is whether we should bother to invoke `dired-remember-marks' if
`dired-keep-marker-rename' is nil.

This patch is local to `wdired-finish-edit', which I think is intended to be
more or less atomic (a transaction), so I guess the answer is no (a user cannot
change the option value during `wdired-finish-edit').

Not calling `dired-remember-marks' if `dired-keep-marker-rename' is nil would
save a little time (not important), and I don't think we would lose anything
doing that.  But it's a question.

BTW, I wonder if there isn't some part of `wdired-finish-edit' that should be in
an `unwind-protect'.  Not thinking of anything in particular - just a feeling...

Message #23 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Cc: christopher <at> ch.ristopher.com
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 27 Jun 2012 19:39:44 +0200

Hi Christopher,

the patch indeed fixes the problem, thanks!

But there is a problem: if I now rename a file that isn't marked, I get the
following error:

Debugger entered--Lisp error: (wrong-type-argument char-or-string-p nil)
  dired-mark-remembered((("/home/micha/Treasure/Trash/bug")))
  (let ((inhibit-read-only t)) (dired-mark-remembered marks))
  (progn (when (and (stringp dired-directory) (not (file-directory-p dired-directory)) (null some-file-names-unchanged) (= (length files-renamed) 1)) (setq dired-directory (cdr (car files-renamed)))) (revert-buffer) (let ((inhibit-read-only t)) (dired-mark-remembered marks)))
  (if changes (progn (when (and (stringp dired-directory) (not (file-directory-p dired-directory)) (null some-file-names-unchanged) (= (length files-renamed) 1)) (setq dired-directory (cdr (car files-renamed)))) (revert-buffer) (let ((inhibit-read-only t)) (dired-mark-remembered marks))) (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)")))
  (let ((changes nil) (errors 0) (marks (dired-remember-marks (point-min) (point-max))) files-deleted files-renamed some-file-names-unchanged file-old file-new tmp-value) (save-excursion (when (and wdired-allow-to-redirect-links (fboundp (quote make-symbolic-link))) (setq tmp-value (wdired-do-symlink-changes)) (setq errors (cdr tmp-value)) (setq changes (car tmp-value))) (when (and wdired-allow-to-change-permissions (boundp (quote wdired-col-perm))) (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)) (when file-old (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) (push file-old files-deleted) (when dired-keep-marker-rename (push ... marks)) (push (cons file-old ...) files-renamed)))) (forward-line -1))) (when files-renamed (setq errors (+ errors (wdired-do-renames files-renamed)))) (if changes (progn (when (and (stringp dired-directory) (not (file-directory-p dired-directory)) (null some-file-names-unchanged) (= (length files-renamed) 1)) (setq dired-directory (cdr (car files-renamed)))) (revert-buffer) (let ((inhibit-read-only t)) (dired-mark-remembered marks))) (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)"))) (when files-deleted (wdired-flag-for-deletion files-deleted)) (when (> errors 0) (dired-log-summary (format "%d rename actions failed" errors) nil)))
  wdired-finish-edit()
  call-interactively(wdired-finish-edit nil nil)

`dired-mark-remembered' does not accept files that are not marked, see
the docstring.  I guess this can easily be fixed by just removing those
from the alist before calling `dired-mark-remembered'.


Regards,

Michael.


> diff --git a/wdired.el b/wdired-fixed.el
> index fe5b4ef..c5956c5 100644
> --- a/wdired.el
> +++ b/wdired-fixed.el
> @@ -351,6 +351,7 @@ non-nil means return old filename."
>    (wdired-change-to-dired-mode)
>    (let ((changes nil)
>  	(errors 0)
> +	(marks (dired-remember-marks (point-min) (point-max)))
>  	files-deleted
>  	files-renamed
>  	some-file-names-unchanged
> @@ -376,6 +377,12 @@ non-nil means return old filename."
>              (setq changes t)
>              (if (not file-new)		;empty filename!
>                  (push file-old files-deleted)
> +	      (when dired-keep-marker-rename
> +		(push (cons (substitute-in-file-name file-new)
> +			    (if (integerp dired-keep-marker-rename)
> +				dired-keep-marker-rename
> +			      (cdr (assoc file-old marks))))
> +		      marks))
>                (push (cons file-old (substitute-in-file-name file-new))
>                      files-renamed))))
>  	(forward-line -1)))
> @@ -393,7 +400,9 @@ non-nil means return old filename."
>  		     (= (length files-renamed) 1))
>  	    (setq dired-directory (cdr (car files-renamed))))
>  	  ;; Re-sort the buffer.
> -	  (revert-buffer))
> +	  (revert-buffer)
> +	  (let ((inhibit-read-only t))
> +	    (dired-mark-remembered marks)))
>        (let ((inhibit-read-only t))
>  	(remove-text-properties (point-min) (point-max)
>  				'(old-name nil end-name nil old-link nil
>
>
>         Christopher

Message #26 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Schmidt <christopher <at> ch.ristopher.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 27 Jun 2012 19:02:47 +0100 (BST)

[Message part 1 (text/plain, inline)]

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> But there is a problem: if I now rename a file that isn't marked, I
> get the following error:
[...]
> `dired-mark-remembered' does not accept files that are not marked, see
> the docstring.  I guess this can easily be fixed by just removing
> those from the alist before calling `dired-mark-remembered'.

Here is a fix for the fix.

[wdired-finish-edit-remember-marks-fixed.diff (text/x-diff, inline)]

diff --git a/wdired.el b/wdired-fixed-fixed.el
index fe5b4ef..045fe91 100644
--- a/wdired.el
+++ b/wdired-fixed-fixed.el
@@ -351,6 +351,7 @@ non-nil means return old filename."
   (wdired-change-to-dired-mode)
   (let ((changes nil)
 	(errors 0)
+	(marks (dired-remember-marks (point-min) (point-max)))
 	files-deleted
 	files-renamed
 	some-file-names-unchanged
@@ -376,6 +377,12 @@ non-nil means return old filename."
             (setq changes t)
             (if (not file-new)		;empty filename!
                 (push file-old files-deleted)
+	      (when dired-keep-marker-rename
+		(let ((mark (if (integerp dired-keep-marker-rename)
+				dired-keep-marker-rename
+			      (cdr (assoc file-old marks)))))
+		  (when mark
+		    (push (cons (substitute-in-file-name file-new) mark) marks))))
               (push (cons file-old (substitute-in-file-name file-new))
                     files-renamed))))
 	(forward-line -1)))
@@ -393,7 +400,9 @@ non-nil means return old filename."
 		     (= (length files-renamed) 1))
 	    (setq dired-directory (cdr (car files-renamed))))
 	  ;; Re-sort the buffer.
-	  (revert-buffer))
+	  (revert-buffer)
+	  (let ((inhibit-read-only t))
+	    (dired-mark-remembered marks)))
       (let ((inhibit-read-only t))
 	(remove-text-properties (point-min) (point-max)
 				'(old-name nil end-name nil old-link nil

[Message part 3 (text/plain, inline)]

        Christopher

Message #29 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Cc: Christopher Schmidt <christopher <at> ch.ristopher.com>
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 27 Jun 2012 23:05:47 +0200

Hi Christopher,

thanks for working on this.

Your patch looks and works quite well so far - but one more detail:

-	  (revert-buffer))
+	  (revert-buffer)
+	  (let ((inhibit-read-only t))
+	    (dired-mark-remembered marks)))

The call to `dired-mark-remembered' should be wrapped in a
`save-excursion', else it may move point to another position.


Regards,

Michael

Message #32 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: 11795 <at> debbugs.gnu.org
Cc: Christopher Schmidt <christopher <at> ch.ristopher.com>
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 04 Jul 2012 06:02:33 +0200

[Message part 1 (text/plain, inline)]

Hello again,

> The call to `dired-mark-remembered' should be wrapped in a
> `save-excursion', else it may move point to another position.

So, this is the fixed patch:

[wdired-finish-edit-remember-marks-fixed.diff (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

I made a lot of tests, and it seems to work well now.  Please, somebody
have a look, and apply it if it's ok.


Thanks,

Michael.

Message #35 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Christopher Schmidt <christopher <at> ch.ristopher.com>, 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;	wdired: C-c C-c loses marks and positions
	of renamed files
Date: Wed, 04 Jul 2012 11:18:49 +0200

>> The call to `dired-mark-remembered' should be wrapped in a
>> `save-excursion', else it may move point to another position.

Can someone explain why `dired-mark-remembered' cannot do:

(defun dired-mark-remembered (alist)
  "Mark all files remembered in ALIST.
Each element of ALIST looks like (FILE . MARKERCHAR)."
  (let (elt fil chr)
    (save-excursion
      (while alist
	(setq elt (car alist)
	      alist (cdr alist)
	      fil (car elt)
	      chr (cdr elt))
	(when (dired-goto-file fil)
	  (beginning-of-line)
	  (delete-char 1)
	  (insert chr))))))

martin

Message #38 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: martin rudalics <rudalics <at> gmx.at>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
	Christopher Schmidt <christopher <at> ch.ristopher.com>, 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 04 Jul 2012 08:29:57 -0400

>>> The call to `dired-mark-remembered' should be wrapped in a
>>> `save-excursion', else it may move point to another position.
> Can someone explain why `dired-mark-remembered' cannot do:

> (defun dired-mark-remembered (alist)
>   "Mark all files remembered in ALIST.
> Each element of ALIST looks like (FILE . MARKERCHAR)."
>   (let (elt fil chr)
>     (save-excursion
>       (while alist
> 	(setq elt (car alist)
> 	      alist (cdr alist)
> 	      fil (car elt)
> 	      chr (cdr elt))
> 	(when (dired-goto-file fil)
> 	  (beginning-of-line)
> 	  (delete-char 1)
> 	  (insert chr))))))

I have no idea, but you could rewrite the above as:

  (require 'pcase)
  (defun dired-mark-remembered (alist)
    "Mark all files remembered in ALIST.
  Each element of ALIST looks like (FILE . MARKERCHAR)."
    (save-excursion
      (pcase-dolist (`(,fil . ,chr) alist)
        (when (dired-goto-file fil)
          (beginning-of-line)
          (delete-char 1)
          (insert chr)))))


-- Stefan "official pcase promoter"

Message #41 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: martin rudalics <rudalics <at> gmx.at>
Cc: Christopher Schmidt <christopher <at> ch.ristopher.com>, 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 22 Aug 2012 06:44:58 +0200

martin rudalics <rudalics <at> gmx.at> writes:

> >> The call to `dired-mark-remembered' should be wrapped in a
> >> `save-excursion', else it may move point to another position.
>
> Can someone explain why `dired-mark-remembered' cannot do:
>
> (defun dired-mark-remembered (alist)
>   "Mark all files remembered in ALIST.
> Each element of ALIST looks like (FILE . MARKERCHAR)."
>   (let (elt fil chr)
>     (save-excursion
>       (while alist
> 	(setq elt (car alist)
> 	      alist (cdr alist)
> 	      fil (car elt)
> 	      chr (cdr elt))
> 	(when (dired-goto-file fil)
> 	  (beginning-of-line)
> 	  (delete-char 1)
> 	  (insert chr))))))
>
> martin

I guess nobody can explain it because your version simply is better and
more correct.

So, it would be cool if the patch could be applied.


Thanks,

Michael.

Message #44 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Christopher Schmidt <christopher <at> ch.ristopher.com>, 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;	wdired: C-c C-c loses marks and positions
	of renamed files
Date: Wed, 22 Aug 2012 09:08:33 +0200

> So, it would be cool if the patch could be applied.

I hopefully applied the change now.  Please check whether it DTRT.

Thanks, martin

Message #47 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Schmidt <christopher <at> ch.ristopher.com>
To: bug-gnu-emacs <at> gnu.org
Cc: martin rudalics <rudalics <at> gmx.at>,
	Michael Heerdegen <heerdegen.michael <at> googlemail.com>
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 29 Aug 2012 16:19:37 +0100 (BST)

martin rudalics <rudalics <at> gmx.at> writes:
> I hopefully applied the change now.  Please check whether it DTRT.

It looks like the first two hunks of Michael's patch were not applied.
(The patch looks fishy - it actually removes the new code rather than
adding it.)

        Christopher

Message #50 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Cc: martin rudalics <rudalics <at> gmx.at>,
	Christopher Schmidt <christopher <at> ristopher.com>,
	Michael Heerdegen <heerdegen.michael <at> googlemail.com>
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 29 Aug 2012 18:15:01 +0200

Christopher Schmidt <christopher <at> ch.ristopher.com> writes:

> martin rudalics <rudalics <at> gmx.at> writes:
> > I hopefully applied the change now.  Please check whether it DTRT.
>
> It looks like the first two hunks of Michael's patch were not applied.
> (The patch looks fishy - it actually removes the new code rather than
> adding it.)

Actually, nothing was applied at all to wdired.el.

Martin, please wait a moment.  I'll prepare a patch that DTRT.


Thanks,

Michael.

Message #53 received at submit <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: bug-gnu-emacs <at> gnu.org, Christopher Schmidt <christopher <at> ristopher.com>,
	Michael Heerdegen <heerdegen.michael <at> googlemail.com>
Subject: Re: bug#11795: 24.1.50; wdired: C-c C-c loses marks and positions
	of renamed files
Date: Wed, 29 Aug 2012 19:22:54 +0200

> Actually, nothing was applied at all to wdired.el.
> 
> Martin, please wait a moment.  I'll prepare a patch that DTRT.

Probably my bad.  Take all the time you need to DTRT.

martin

Message #56 received at submit <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: bug-gnu-emacs <at> gnu.org, Christopher Schmidt <christopher <at> ristopher.com>,
	Michael Heerdegen <heerdegen.michael <at> googlemail.com>
Subject: Re: bug#11795: 24.1.50; wdired: C-c C-c loses marks and positions
	of renamed files
Date: Wed, 03 Oct 2012 11:14:21 +0200

> Actually, nothing was applied at all to wdired.el.
> 
> Martin, please wait a moment.  I'll prepare a patch that DTRT.

Any chances for a fix to this in the next days?

martin

Message #59 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Cc: Martin Rudalics <rudalics <at> gmx.at>
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Fri, 05 Oct 2012 01:52:01 +0200

martin rudalics <rudalics <at> gmx.at> writes:

> > Martin, please wait a moment.  I'll prepare a patch that DTRT.
>
> Any chances for a fix to this in the next days?

Martin, I would rather like to discuss first what we do with bug#12394.
Can you please have a look?  It would be good to know what we want to
change for that bug before applying a patch for this.


Thanks,

Michael.

Message #62 received at submit <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11795: 24.1.50; wdired: C-c C-c loses marks and positions
	of renamed files
Date: Fri, 05 Oct 2012 09:05:10 +0200

> Martin, I would rather like to discuss first what we do with bug#12394.
> Can you please have a look?  It would be good to know what we want to
> change for that bug before applying a patch for this.

IIRC `wdired' is full of such niceties.  Here the problem is that
`dired-move-to-filename' moves to the start of the old filename,
skipping the prepended string.  Maybe it's sufficient to remove that
call as in the patch below since the subsequent

(search-forward (wdired-get-filename t) nil t)

should move to the right position anyway.  If not, we'll have to rewrite
this function from scratch.

martin

*** lisp/wdired.el	2012-08-05 04:19:21 +0000
--- lisp/wdired.el	2012-10-05 06:36:43 +0000
***************
*** 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,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

Message #65 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: martin rudalics <rudalics <at> gmx.at>
Cc: christopher <at> ristopher.com, 11795 <at> debbugs.gnu.org,
	heerdegen.michael <at> googlemail.com
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Tue, 09 Oct 2012 02:41:42 +0200

[Message part 1 (text/plain, inline)]

Hello Martin,

> Any chances for a fix to this in the next days?

Sorry for the delay.

The patch we had so far for this had several problems:

- It sometimes failed to collect file marks because it did that when the
  buffer had already been modified.

- It failed to restore the marks of files that couldn't be renamed
  (e.g. in read-only dirs)

- It didn't propagate the name changes to other dired buffers

I created a new fix that should take care of these problems.  I wanted
to make some more sanity tests today, unfortunately I'm ill and don't
feel well.  I'll try to do this tomorrow.

So, this is the new patch:

[wdired.patch (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

Regards,

Michael.

Message #68 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Tue, 16 Oct 2012 01:01:36 +0200

[Message part 1 (text/plain, inline)]

martin rudalics <rudalics <at> gmx.at> writes:

> Any chances for a fix to this in the next days?

After performing different tests, my patch looks sane so far.  IMHO it
can be installed now.

[wdired.patch (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

Michael.

Message #71 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Christopher Schmidt <christopher <at> ch.ristopher.com>, 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;	wdired: C-c C-c loses marks and positions
	of renamed files
Date: Tue, 16 Oct 2012 11:39:50 +0200

>> Any chances for a fix to this in the next days?
> 
> After performing different tests, my patch looks sane so far.  IMHO it
> can be installed now.

Christopher can you check Michael's patch?  Pretty please.

Thanks, martin

Message #76 received at 11795-done <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: 11795-done <at> debbugs.gnu.org
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>
Subject: Re: bug#11795: 24.1.50;	wdired: C-c C-c loses marks and positions
	of renamed files
Date: Wed, 17 Oct 2012 11:37:24 +0200

> After performing different tests, my patch looks sane so far.  IMHO it
> can be installed now.

Installed as revision 110564 on trunk.  Bug closed.

Thanks, martin

Message #79 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Schmidt <christopher <at> ch.ristopher.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 17 Oct 2012 10:51:27 +0100 (BST)

martin rudalics <rudalics <at> gmx.at> writes:
> Christopher can you check Michael's patch?  Pretty please.
>
> Thanks, martin

martin rudalics <rudalics <at> gmx.at> writes:
>> After performing different tests, my patch looks sane so far.  IMHO
>> it can be installed now.
>
> Installed as revision 110564 on trunk.  Bug closed.

FWIW I did some very basic testing yesterday.  Everything looked fine to
me.

Michael, Martin, thanks for your work on this issue.

        Christopher

Message #82 received at submit <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11795: 24.1.50;	wdired: C-c C-c loses marks and positions
	of renamed files
Date: Thu, 18 Oct 2012 11:15:51 +0200

> FWIW I did some very basic testing yesterday.  Everything looked fine to
> me.
> 
> Michael, Martin, thanks for your work on this issue.

Thanks to you for testing.

martin

Message #85 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Tue, 23 Oct 2012 11:05:04 +0300

> But shouldn't Wdired respect the user's setting of option
> `dired-keep-marker-rename'?  IOW, shouldn't users be able to control whether
> exiting wdired removes marks?

When `dired-keep-marker-rename' is customized to a character like ?R or ?M,
now `wdired-finish-edit' adds a new marker on the files renamed by wdired.
This is new behavior.  Strictly speaking, it is consistent with the meaning
of `dired-keep-marker-rename', so I'm not sure if a new option is necessary
to control this.  But at least when someone wants to put markers only on the
files moved by `dired-do-rename', but not on the files renamed by wdired,
this is still possible to do by adding to ~/.emacs something like:

  (defadvice wdired-finish-edit (around my-wdired-finish-edit act)
    (let ((dired-keep-marker-rename t))
      ad-do-it))

Message #88 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Juri Linkov <juri <at> jurta.org>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Wed, 24 Oct 2012 15:31:33 +0200

Juri Linkov <juri <at> jurta.org> writes:

> > But shouldn't Wdired respect the user's setting of option
> > `dired-keep-marker-rename'?  IOW, shouldn't users be able to control
> > whether
> > exiting wdired removes marks?
>
> When `dired-keep-marker-rename' is customized to a character like ?R or ?M,
> now `wdired-finish-edit' adds a new marker on the files renamed by wdired.
> This is new behavior.  Strictly speaking, it is consistent with the meaning
> of `dired-keep-marker-rename', so I'm not sure if a new option is necessary
> to control this.  But at least when someone wants to put markers only on the
> files moved by `dired-do-rename', but not on the files renamed by wdired,
> this is still possible to do by adding to ~/.emacs something like:
>
>   (defadvice wdired-finish-edit (around my-wdired-finish-edit act)
>     (let ((dired-keep-marker-rename t))
>       ad-do-it))

I think you're right, some people may want to have a behavior different
than that defined by `dired-keep-marker-rename'.  And we should avoid
the necessity for users to use something nontrivial like `defadvice' to
control this.

So, introducing an new option `wdired-keep-marker-rename' would IMHO make
sense.  The default value could be t, at least that's what I think would
be most intuitive.


Michael.

Message #91 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Thu, 25 Oct 2012 12:03:35 +0300

> So, introducing an new option `wdired-keep-marker-rename' would IMHO make
> sense.  The default value could be t, at least that's what I think would
> be most intuitive.

Yes, I agree that introducing `wdired-keep-marker-rename' would be
the best way to fix this.

Message #94 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Sat, 27 Oct 2012 12:18:34 +0300

> So, introducing an new option `wdired-keep-marker-rename' would IMHO make
> sense.  The default value could be t, at least that's what I think would
> be most intuitive.

I fixed this regression by adding `wdired-keep-marker-rename'.
Thanks for the suggestion.

When testing it, I noticed a random behavior of `wdired-do-renames' when
the marker is customized to a character.  The decision what character to put
depends on the position of point.  When point is at the top of the Dired
buffer, it uses a customized marker character.  When point is at the bottom,
it restores an old marker character.

The problem is that `dired-remove-file' doesn't remove file entries
(and neither `dired-rename-file') because files already have new renamed names
in the Dired buffer.  Whereas `dired-add-entry' adds duplicated file entries.

Fortunately, `revert-buffer' (called at the end of `wdired-finish-edit')
sorts out this mess caused by `dired-add-entry'.  But in `dired-revert'
it uses `dired-remember-marks' and `dired-restore-positions' to restore
markers.  So when duplicated file entries were added by `dired-add-entry'
at the top of the Dired buffer (in front of file entries edited by WDired),
it uses a customized marker character.  When duplicated file entries
were added at the bottom, it restores an old marker character because
`dired-restore-positions' in `dired-revert' gets markers from the
first duplicated file entry.

Do you have an idea how to fix this problem?  One way would be
instead of using `dired-add-file' to change markers in the remembered
variable `wdired-old-marks' and just call `dired-mark-remembered'
(before `revert-buffer' in `wdired-finish-edit') with the modified
list of markers for renamed files.

Message #97 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Juri Linkov <juri <at> jurta.org>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Sat, 27 Oct 2012 14:55:40 +0200

Hello Juri,

> > So, introducing an new option `wdired-keep-marker-rename' would IMHO make
> > sense.  The default value could be t, at least that's what I think would
> > be most intuitive.
>
> I fixed this regression by adding `wdired-keep-marker-rename'.
> Thanks for the suggestion.

Good - thanks!

> When testing it, I noticed a random behavior of `wdired-do-renames'
> when the marker is customized to a character.  The decision what
> character to put depends on the position of point.  When point is at
> the top of the Dired buffer, it uses a customized marker character.
> When point is at the bottom, it restores an old marker character.
>
> The problem is that `dired-remove-file' doesn't remove file entries
> (and neither `dired-rename-file') because files already have new
> renamed names
> in the Dired buffer.  Whereas `dired-add-entry' adds duplicated file
> entries.
>
> Fortunately, `revert-buffer' (called at the end of
> `wdired-finish-edit') sorts out this mess caused by `dired-add-entry'.
> But in `dired-revert' it uses `dired-remember-marks' and
> `dired-restore-positions' to restore markers.  So when duplicated file
> entries were added by `dired-add-entry' at the top of the Dired buffer
> (in front of file entries edited by WDired), it uses a customized
> marker character.  When duplicated file entries were added at the
> bottom, it restores an old marker character because
> `dired-restore-positions' in `dired-revert' gets markers from the
> first duplicated file entry.

Thanks for finding this, and for your analysis.

> Do you have an idea how to fix this problem?  One way would be instead
> of using `dired-add-file' to change markers in the remembered variable
> `wdired-old-marks' and just call `dired-mark-remembered' (before
> `revert-buffer' in `wdired-finish-edit') with the modified list of
> markers for renamed files.

I chose to use `dired-add-file' in my patch so that name changes are
propagated also to other dired buffers where these files may also be
shown.  This is consistent with `dired-do-rename'.

WRT your suggestion: I see two problems:

1. Calling `dired-mark-remembered' before `revert-buffer' is not
reliable, because it may fail to get the right names from places where
changes were made.  I.e., it my not work for the same reason as
`dired-remove-file' doesn't work.

2. Even if this worked well, the remaining old lines of renamed files
are still lying around.  They can still cause trouble.  When
`revert-buffer' is called, these lines are still included for
remembering marks.  This is problematic: there is a chance that the user
has renamed another file in that buffer to that collected name.

My suggestion would be to just delete the according lines so that they
cannot cause trouble, simply with `delete-region'.  That should not do
any harm.

What do you think?


Regards,

Michael.

Message #100 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Sat, 27 Oct 2012 17:38:58 +0300

> My suggestion would be to just delete the according lines so that they
> cannot cause trouble, simply with `delete-region'.  That should not do
> any harm.

I'm afraid `delete-region' will do harm especially at this stage closer
to the release because it needs heavy testing for possible failures.

Wdired worked correctly for a long time without `dired-add-file', so
it's better to avoid using `dired-add-file' and low-level `delete-region'.

`revert-buffer' already does its work well and propagates name changes
to other dired buffers where these files may also be shown.

The original request was about keeping markers of renamed files,
so what is needed is to use just `dired-remember-marks' and
`dired-mark-remembered' (and also another pair of functions
`dired-save-positions' and `dired-restore-positions').

There is no problem of getting the right names from places where
changes were made.  This should be implemented in the same place
in `wdired-do-renames' that currently has information about old and new
file names and their markers.  The same place in `wdired-do-renames'
can modify the remembered list of markers accordingly.
IOW, keep the same logic in `wdired-do-renames' as it has now,
but modify the `wdired-old-marks' variable instead of operating
on the dired buffer.

Message #103 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Thu, 22 Nov 2012 11:12:12 +0200

reopen 11795
stop

This bug is a regression against Emacs 23.1+.

Actually I found out that Christopher already proposed a patch
that fixes this bug.  I see no reason not to use it.  I propose
to do this on the Emacs-24 release branch before the pretest:
to revert the ill-behaved part of revno:110564 and install
Christopher's patch that works correctly in my tests and
has no bad effects caused by unnecessary direct operations
on the dired buffer.

Below is a complete patch adapted for the Emacs-24 release branch:

=== modified file 'lisp/wdired.el'
--- lisp/wdired.el	2012-10-27 09:17:14 +0000
+++ lisp/wdired.el	2012-11-22 09:09:30 +0000
@@ -399,6 +397,15 @@ (defun wdired-finish-edit ()
             (setq changes t)
             (if (not file-new)		;empty filename!
                 (push file-old files-deleted)
+	      (when wdired-keep-marker-rename
+		(let ((mark (cond ((integerp wdired-keep-marker-rename)
+				   wdired-keep-marker-rename)
+				  (wdired-keep-marker-rename
+				   (cdr (assoc file-old wdired-old-marks)))
+				  (t nil))))
+		  (when mark
+		    (push (cons (substitute-in-file-name file-new) mark)
+			  wdired-old-marks))))
               (push (cons file-old (substitute-in-file-name file-new))
                     files-renamed))))
 	(forward-line -1)))
@@ -416,7 +423,9 @@ (defun wdired-finish-edit ()
 		     (= (length files-renamed) 1))
 	    (setq dired-directory (cdr (car files-renamed))))
 	  ;; Re-sort the buffer.
-	  (revert-buffer))
+	  (revert-buffer)
+	  (let ((inhibit-read-only t))
+	    (dired-mark-remembered wdired-old-marks)))
       (let ((inhibit-read-only t))
 	(remove-text-properties (point-min) (point-max)
 				'(old-name nil end-name nil old-link nil
@@ -430,8 +439,6 @@ (defun wdired-finish-edit ()
   (set-buffer-modified-p nil)
   (setq buffer-undo-list nil))
 
-(declare-function dired-add-entry "dired-aux" (filename &optional marker-char relative))
-
 (defun wdired-do-renames (renames)
   "Perform RENAMES in parallel."
   (let ((residue ())
@@ -473,8 +480,7 @@ (defun wdired-do-renames (renames)
               (push (cons tmp file-new) residue))))
          (t
           (setq progress t)
-          (let* ((file-ori (car rename))
-                 (old-mark (cdr (assoc file-ori wdired-old-marks))))
+          (let* ((file-ori (car rename)))
             (if wdired-use-interactive-rename
                 (wdired-search-and-rename file-ori file-new)
               ;; If dired-rename-file autoloads dired-aux while
@@ -485,20 +491,12 @@ (defun wdired-do-renames (renames)
               (condition-case err
                   (let ((dired-backup-overwrite nil))
                     (dired-rename-file file-ori file-new
-                                       overwrite)
-                    (dired-remove-file file-ori)
-                    (dired-add-file
-		     file-new
-		     (cond ((integerp wdired-keep-marker-rename)
-			    wdired-keep-marker-rename)
-			   (wdired-keep-marker-rename old-mark)
-			   (t nil))))
+                                       overwrite))
                 (error
                  (setq errors (1+ errors))
                  (dired-log (concat "Rename `" file-ori "' to `"
                                     file-new "' failed:\n%s\n")
-                            err)
-                 (dired-add-entry file-ori old-mark)))))))))
+                            err)))))))))
     errors))
 
 

Message #108 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Thu, 22 Nov 2012 10:28:43 -0500

> Below is a complete patch adapted for the Emacs-24 release branch:

Please install it,


        Stefan

Message #111 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Juri Linkov <juri <at> jurta.org>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Fri, 23 Nov 2012 01:04:13 +0100

Hello Juri,

> reopen 11795
> stop
>
> This bug is a regression against Emacs 23.1+.
>
> Actually I found out that Christopher already proposed a patch
> that fixes this bug.  I see no reason not to use it.  I propose
> to do this on the Emacs-24 release branch before the pretest:
> to revert the ill-behaved part of revno:110564 and install
> Christopher's patch that works correctly in my tests and
> has no bad effects caused by unnecessary direct operations
> on the dired buffer.

Thanks for your work.

But I see some problems in your patch that I already had fixed in my
version.

1.  When I prepend a letter z to the first file in a buffer, so that it
will be under the last files after reverting, and I hit C-c C-c, then
point is at another file afterwards.  This was an issue of the original
report.

2.  If renamed files are also visible in another dired buffer, they
just disappear there.  That makes no sense.


Regards,

Michael.

Message #114 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Fri, 23 Nov 2012 09:29:30 +0200

>> Below is a complete patch adapted for the Emacs-24 release branch:
>
> Please install it,

Installed in the emacs-24 branch.

Message #117 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Fri, 23 Nov 2012 09:30:31 +0200

> 1.  When I prepend a letter z to the first file in a buffer, so that it
> will be under the last files after reverting, and I hit C-c C-c, then
> point is at another file afterwards.  This was an issue of the original
> report.

This is not a regression, so it could be fixed in the trunk
in the same way as for `wdired-old-marks', i.e. to store the original
value returned from `dired-save-positions', changing original filenames
to renamed filenames in the returned value, and calling
`dired-restore-positions' with the new value after `revert-buffer'.
A simple patch below does this.

> 2.  If renamed files are also visible in another dired buffer, they
> just disappear there.  That makes no sense.

The problem where other Dired buffers are not updated
in `wdired-finish-edit' is not a regression too.
WDired worked this way from its first version.

If you want, you could try to rewrite WDired so that it correctly
maintains consistency with renamed files without using `revert-buffer'
and by using `dired-add-file' like `dired-do-rename' does.

Your previous patch doesn't do this correctly - it inserts
duplicate file lines, so Dired displays two versions of the same file,
but fortunately `revert-buffer' fixes this mess.

Instead of this, much simpler would be to keep the current design
of WDired that uses `revert-buffer' to maintain consistency with renames.
Then a simpler fix would be to call `revert-buffer' in all other
Dired buffers that display renamed files by using

  (dired-fun-in-all-buffers dired-directory nil (function revert-buffer))

This patch for the trunk demonstrates how these problems could be fixed.
Do you see more problems with this approach?

=== modified file 'lisp/wdired.el'
--- lisp/wdired.el	2012-11-21 10:29:30 +0000
+++ lisp/wdired.el	2012-11-23 07:28:39 +0000
@@ -194,6 +194,7 @@ (defvar wdired-mode-hook nil
 (defvar wdired-col-perm) ;; Column where the permission bits start
 (defvar wdired-old-content)
 (defvar wdired-old-point)
+(defvar wdired-old-positions)
 (defvar wdired-old-marks)
 
 (defun wdired-mode ()
@@ -237,6 +238,8 @@ (defun wdired-change-to-wdired-mode ()
        (buffer-substring (point-min) (point-max)))
   (set (make-local-variable 'wdired-old-marks)
        (dired-remember-marks (point-min) (point-max)))
+  (set (make-local-variable 'wdired-old-positions)
+       (dired-save-positions))
   (set (make-local-variable 'wdired-old-point) (point))
   (set (make-local-variable 'query-replace-skip-read-only) t)
   (add-hook 'isearch-filter-predicates 'wdired-isearch-filter-read-only nil t)
@@ -406,6 +409,8 @@ (defun wdired-finish-edit ()
 		  (when mark
 		    (push (cons (substitute-in-file-name file-new) mark)
 			  wdired-old-marks))))
+	      (when (equal (cadr (car wdired-old-positions)) file-old)
+		(setcar (cdr (car wdired-old-positions)) file-new))
               (push (cons file-old (substitute-in-file-name file-new))
                     files-renamed))))
 	(forward-line -1)))
@@ -423,9 +428,12 @@ (defun wdired-finish-edit ()
 		     (= (length files-renamed) 1))
 	    (setq dired-directory (cdr (car files-renamed))))
 	  ;; Re-sort the buffer.
-	  (revert-buffer)
+	  (if (stringp dired-directory)
+	      (dired-fun-in-all-buffers dired-directory nil (function revert-buffer))
+	    (revert-buffer))
 	  (let ((inhibit-read-only t))
-	    (dired-mark-remembered wdired-old-marks)))
+	    (dired-mark-remembered wdired-old-marks))
+	  (dired-restore-positions wdired-old-positions))
       (let ((inhibit-read-only t))
 	(remove-text-properties (point-min) (point-max)
 				'(old-name nil end-name nil old-link nil

Message #120 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Sun, 25 Nov 2012 11:12:20 +0200

> Instead of this, much simpler would be to keep the current design
> of WDired that uses `revert-buffer' to maintain consistency with renames.
> Then a simpler fix would be to call `revert-buffer' in all other
> Dired buffers that display renamed files by using
>
>   (dired-fun-in-all-buffers dired-directory nil (function revert-buffer))

After looking more at this, I noticed a problem that no previous patch
was able to fix.

The question is what to do with marks in other Dired buffers?

One option is to copy marks of renamed files from the current Dired buffer
to other Dired buffers that display the same set of renamed files.
But I think that more correct would be to remember marks in all other
Dired buffers, and in `wdired-finish-edit' restore them in other Dired
buffer to their previous marks.

This means that `wdired-old-marks' should be an associative list
where the key is a Dired buffer and the value is the returned value
from `dired-remember-marks' remembered for each Dired buffer.

Message #123 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Sat, 19 Jan 2013 07:32:10 +0100

Hello Juri,

sorry, again, for the long delay.

> > 1.  When I prepend a letter z to the first file in a buffer, so that it
> > will be under the last files after reverting, and I hit C-c C-c, then
> > point is at another file afterwards.  This was an issue of the original
> > report.
>
> This is not a regression, so it could be fixed in the trunk
> in the same way as for `wdired-old-marks', i.e. to store the original
> value returned from `dired-save-positions', changing original filenames
> to renamed filenames in the returned value, and calling
> `dired-restore-positions' with the new value after `revert-buffer'.
> A simple patch below does this.

Ok, that seems reasonable to me.

> > 2.  If renamed files are also visible in another dired buffer, they
> > just disappear there.  That makes no sense.
>
> The problem where other Dired buffers are not updated
> in `wdired-finish-edit' is not a regression too.
> WDired worked this way from its first version.
>
> Instead of this, much simpler would be to keep the current design
> of WDired that uses `revert-buffer' to maintain consistency with renames.
> Then a simpler fix would be to call `revert-buffer' in all other
> Dired buffers that display renamed files by using
>
>   (dired-fun-in-all-buffers dired-directory nil (function revert-buffer))
>
> This patch for the trunk demonstrates how these problems could be fixed.
> Do you see more problems with this approach?

I must say that I'm a bit reluctant to force a revert in other buffers.
Because some aspects of the these buffers may be lost that maybe were
important for the user - like killed files (they reappear when
reverting) or any marks (e.g. belonging to positions stored in a
register).  That's why I tried to avoid reverting.

WRT to the marks in other buffers (see your followup
<87sj7yavw7.fsf <at> mail.jurta.org>) - I agree we should restore (keep) the
marks in those other buffers when renaming files, and not export the
marks form the buffer where wdired was used.


Thanks,

Michael.

Message #126 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Sat, 19 Jan 2013 12:37:23 +0200

> I must say that I'm a bit reluctant to force a revert in other buffers.
> Because some aspects of the these buffers may be lost that maybe were
> important for the user - like killed files (they reappear when
> reverting) or any marks (e.g. belonging to positions stored in a
> register).  That's why I tried to avoid reverting.

Not reverting means a lot of trouble.  For example, you have to detect
the situation when a file is in the killed state, so not to add its
renamed file line to the Dired buffer in this case, etc.

A less error-prone approach would be to allow a revert to avoid
all these nuances.

Message #129 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Juri Linkov <juri <at> jurta.org>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50;
	wdired: C-c C-c loses marks and positions of renamed files
Date: Sun, 20 Jan 2013 06:15:53 +0100

> > I must say that I'm a bit reluctant to force a revert in other buffers.
> > Because some aspects of the these buffers may be lost that maybe were
> > important for the user - like killed files (they reappear when
> > reverting) or any marks (e.g. belonging to positions stored in a
> > register).  That's why I tried to avoid reverting.
>
> Not reverting means a lot of trouble.  For example, you have to detect
> the situation when a file is in the killed state, so not to add its
> renamed file line to the Dired buffer in this case, etc.

Agreed.

> A less error-prone approach would be to allow a revert to avoid
> all these nuances.

Ok.  So, do you want to write a patch to implement this?


Thanks,

Michael.

Message #134 received at 11795 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 11795 <at> debbugs.gnu.org
Subject: Re: bug#11795: 24.1.50; wdired: C-c C-c loses marks and positions
 of renamed files
Date: Mon, 21 Sep 2020 16:58:21 +0200

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> a short demonstration for emacs -Q:
>
> 1. M-x dired a directory containing some files
>
> 2. Go to the first file's line (not "." or ".."), and mark the file
> with m
>
> 3. M-x wdired-change-to-wdired-mode
>
> 4. Edit the name of the first file: e.g. add a "z" to the front of the
>    name so that the file will surely appear at another position after
>    renaming
>
> 5. C-c C-c
>
> You'll see that point is still at the first file's line, although the
> renamed file is now displayed near the end of the buffer.  Even worse,
> the mark of the file has disappeared.

A long discussion then followed, which I've only skimmed.  The bug
itself was fixed at the time, though, so I'm closing this bug report.
If there's anything further to be done in this area, a new bug report
could perhaps be opened.

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.