bug#70593: 30.0.50; Dired: buffers of renamed dirs are broken

bug#70593: 30.0.50; Dired: buffers of renamed dirs are broken

[Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hello,

I'm really, really happy that I finally nailed down this not
insignificant dired problem:

When you revert a dired buffer whose default-directory has been renamed,
the buffer is erased and an error message is displayed.  Marks and stuff
are permanently lost.  This even happens when the directory had been
renamed with means of dired.

It follows a recipe for emacs -Q; feel free to skip to the following
analysis of the problem... (we care about this case, but the code is
broken - see below)

[ Recipe:

I have two dired buffers, one showing "~", the other one a subdirectory
"~/test" in the other window.  You can put some trash files into that
directory and mark some of them.

Now I select the "~" dired buffer and enable wdired.  I rename "test"
to, say, "test1", and confirm.

Now select the other dired buffer.  The directory name in the first line
has even been updated.  But when I hit g, the buffer is erased and an
error is raised:

| insert-directory: Setting current directory: No such file or directory,
| /home/micha/test2/

Recipe end ]

The problem is that we partially fail to update the affected buffers.

The code wants to do this in `dired-rename-subdir-1':

    (if (equal dir default-directory)
	;; if top level directory was renamed, lots of things have to be
	;; updated:

DIR is the old directory name.  But the `equality' test fails simply
because default-directory is "~/test/" but DIR has the tilde expanded.


TIA,

Michael.

bug#70593: 30.0.50; Dired: buffers of renamed dirs are broken

[Eli Zaretskii]

> Date: Fri, 26 Apr 2024 13:59:16 +0200
> From:  Michael Heerdegen via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> The problem is that we partially fail to update the affected buffers.
> 
> The code wants to do this in `dired-rename-subdir-1':
> 
>     (if (equal dir default-directory)
> 	;; if top level directory was renamed, lots of things have to be
> 	;; updated:
> 
> DIR is the old directory name.  But the `equality' test fails simply
> because default-directory is "~/test/" but DIR has the tilde expanded.

So all we need to do is call expand-file-name on both of them?  Or do
we need also to call file-truename?

bug#70593: 30.0.50; Dired: buffers of renamed dirs are broken

[Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

> > The code wants to do this in `dired-rename-subdir-1':
> >
> >     (if (equal dir default-directory)
> > 	;; if top level directory was renamed, lots of things have to be
> > 	;; updated:
> >
> > DIR is the old directory name.  But the `equality' test fails simply
> > because default-directory is "~/test/" but DIR has the tilde expanded.
>
> So all we need to do is call expand-file-name on both of them?  Or do
> we need also to call file-truename?

A sign of life with intermediate results:

Turns out this is a difficult and not even the only question (see
below).  It looks to me like this branch of the code (after the always
failing test) was never in use, or only a very long time ago.  Because
that unreachable branch is broken in several ways.

I needed to add a `file-name-as-directory' wrapper, else dired wouldn't
recognize the new directory as a directory and treats the file name as a
wildcard instead.

I would also add `abbreviate-file-name' calls so that the format of the
file names is like before (abbreviated).  Finally I don't understand why
the existing code computes `default-directory' and `dired-directory'
differently, so I reduced that to one computation for now, but also see
below for more.

This took me to this first actually working version for the
simplest case:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-Try-to-fix-70593.patch --]
[-- Type: text/x-diff, Size: 1912 bytes --]

From aa6146e69d8d9d97424ccaf4d69db9eea64d0011 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Mon, 29 Apr 2024 16:10:29 +0200
Subject: [PATCH] WIP: Try to fix #70593

---
 lisp/dired-aux.el | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index e340f98a551..60a0f16bfe4 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -2331,18 +2331,21 @@ dired-rename-subdir-1
       (if (dired-in-this-tree-p (car elt) expanded-dir)
 	  ;; ELT's subdir is affected by the rename
 	  (dired-rename-subdir-2 elt dir to)))
-    (if (equal dir default-directory)
+    (if (string= dir (expand-file-name default-directory))
 	;; if top level directory was renamed, lots of things have to be
 	;; updated:
 	(progn
 	  (dired-unadvertise dir)	; we no longer dired DIR...
-	  (setq default-directory to
-		dired-directory (expand-file-name;; this is correct
-				 ;; with and without wildcards
-				 (file-name-nondirectory (if (stringp dired-directory)
-                                                             dired-directory
-                                                           (car dired-directory)))
-				 to))
+	  (setq dired-directory
+                (abbreviate-file-name
+                 (file-name-as-directory
+                  (expand-file-name ;; this is correct
+		   ;; with and without wildcards
+		   (file-name-nondirectory (if (stringp dired-directory)
+                                               dired-directory
+                                             (car dired-directory)))
+		   to)))
+                default-directory dired-directory)
 	  (let ((new-name (file-name-nondirectory
 			   (directory-file-name (if (stringp dired-directory)
                                                     dired-directory
--
2.39.2


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



Open questions:

(1) What do we need to do we do when `dired-directory' is a cons
(i.e. dir along with a file list)?  The existing code just threw away
the file list (that's why I unified the cases for now - the code didn't
make sense to me).

(2) What to do when symlinks are involved?  This question is also not
trivial.  It might be necessary to handle different cases and change
more than one part of the code.

Just calling `file-truename' before comparing as you (Eli) suggested
might fix more valid cases but will also resolve symlinks as side
effect: the dired buffer will afterwards visit a directory behind a
(probably unrelated) symlink when the visited directory name had not
been unresolved before.  This is not what we want.  Hope it's
understandable what I mean.

Another tricky case is when the user accidentally broke a symlink
and a dired buffer we handle visits a directory with a name that can't
be resolved any more.


Michael.