Subject: Re: bug#54636: dired fails to update subdirs when files are created/deleted/renamed

From: Tassilo Horn <[email-masked]>
Date: Wed, 30 Mar 2022 16:30:22 +0200

Eli Zaretskii <[email-masked]> writes:

Hi Eli,

>> > I think I just fixed it there, please take a look.
>> 
>> That's what I was about to do, too, but that's not right, too.
>
> Well, it fixes the regression.

That's true.

>> Say you have dired buffers for
>> 
>>   ~/
>>   ~/foo/
>>   ~/foo/bar/
>> 
>> and then delete ~/foo from inside the ~/ dired buffer.  You'll be
>> asked if the buffers for foo should be delete, too, and when you
>> confirm, what is deleted are the buffers ~/ and ~/foo but ~/foo/bar
>> persists.  So the buffers are deleted downwards (to the root) instead
>> of upwards which is wrong.
>
> This means your fix for the "ask" part is incomplete, and should be
> improved.  But that is a new feature in Emacs 28, so it is not a
> catastrophe if it is imperfect.

It's still a major annoyance since the feature is not opt-in.

> Regressions in previously correct behavior are much worse.

Of course.

> Of course, if you can come up with a fix for the question-asking part
> that makes it delete all the relevant buffers, and if that fix is safe
> enough (a high bar at this late stage of the pretest), we can install
> that on the release branch.  Failing that, the fix for that will have
> to wait till Emacs 28.2 at the very least.

Here's a patch.  I've created a separate cond-arm for the SUBDIRS case,
so any callers which are not dired-clean-up-after-deletion (the only one
setting SUBDIRS) are not affected.

diff --git a/lisp/dired.el b/lisp/dired.el
index 75dcd33e67..972a0865f4 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2874,8 +2874,9 @@ dired-buffers-for-dir
   "Return a list of buffers for DIR (top level or in-situ subdir).
 If FILE is non-nil, include only those whose wildcard pattern (if any)
 matches FILE.
-If SUBDIRS is non-nil, also include the dired buffers of
-directories below DIR.
+If SUBDIRS is non-nil, include the dired buffers of DIR and the
+directories below DIR instead (but no dired buffers with in-situ
+subdir DIR).
 The list is in reverse order of buffer creation, most recent last.
 As a side effect, killed dired buffers for DIR are removed from
 `dired-buffers'."
@@ -2887,10 +2888,9 @@ dired-buffers-for-dir
        ((null (buffer-name buf))
 	;; Buffer is killed - clean up:
 	(setq dired-buffers (delq elt dired-buffers)))
-       ((dired-in-this-tree-p dir (car elt))
+       ((and (null subdirs) (dired-in-this-tree-p dir (car elt)))
 	(with-current-buffer buf
-          (when (and (or subdirs
-                         (assoc dir dired-subdir-alist))
+          (when (and (assoc dir dired-subdir-alist)
 	             (or (null file)
 		         (if (stringp dired-directory)
 		             (let ((wildcards (file-name-nondirectory
@@ -2900,7 +2900,9 @@ dired-buffers-for-dir
                                                    file)))
 		           (member (expand-file-name file dir)
 			           (cdr dired-directory)))))
-            (setq result (cons buf result)))))))
+            (setq result (cons buf result)))))
+       ((and subdirs (dired-in-this-tree-p (car elt) dir))
+        (setq result (cons buf result)))))
     result))
 
 (defun dired-glob-regexp (pattern)

Can we agree it is safe enough?

Bye,
Tassilo