Subject: Re: bug#75626: 31.0.50; Dired misses or double-processes files when auto-revert-mode is enabled

From: Tassilo Horn <[email-masked]>
Date: Mon, 20 Jan 2025 07:06:20 +0100

Michael Heerdegen <[email-masked]> writes:

> Tassilo Horn <[email-masked]> writes:
>
>> Good idea.  I've tried this simple patch and it fixes the issue for me.
>>
>> diff --git a/lisp/dired.el b/lisp/dired.el
>> index bab5e833a76..1152d85f149 100644
>> --- a/lisp/dired.el
>> +++ b/lisp/dired.el
>> @@ -1289,6 +1289,10 @@ dired-buffer-stale-p
>>          ;; Do not auto-revert when the dired buffer can be currently
>>          ;; written by the user as in `wdired-mode'.
>>          buffer-read-only
>> +         ;; When a dired operation using dired-map-over-marks is in
>> +         ;; progress, inhibit-read-only is set and we must not
>
> Better say "bound" please.
>
>> +         ;; auto-revert.
>> +         (null inhibit-read-only)
>
> And use `not' here, since we are testing a boolean valued flag.
>
>>          (dired-directory-changed-p dirname))))
>>
>>  (defcustom dired-auto-revert-buffer nil

So like this:

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/dired.el b/lisp/dired.el
index 2087a6f6f21..c78a13800e6 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -1289,6 +1289,12 @@ dired-buffer-stale-p
         ;; Do not auto-revert when the dired buffer can be currently
         ;; written by the user as in `wdired-mode'.
         buffer-read-only
+         ;; When a dired operation using dired-map-over-marks is in
+         ;; progress, inhibit-read-only is bound to some non-nil value
+         ;; and we must not auto-revert because that could change the
+         ;; order of files leading to skipping or double-processing (see
+         ;; bug#75626).
+         (not inhibit-read-only)
         (dired-directory-changed-p dirname))))
 
 (defcustom dired-auto-revert-buffer nil
--8<---------------cut here---------------end--------------->8---

Eli, good to go?  And if so, master or emacs-30?

>> Probably testing inhibit-read-only is not TRT but the expansion of
>> dired-map-over-marks should explicitly let-bind some new
>> dired--map-over-marks-in-progress variable to make it more
>> explicit...
>
> Yes.  OTOH it's not bad either.  `inhibit-read-only' bound (together
> with buffer-read-only which we already have) is a good indicator for
> that some operation is running and we should not auto revert.
>
> Unless I'm missing something I would prefer this solution.

Yes, you are right.  Whoever binds inhibit-read-only in a dired-buffer
is probably doing something that won't play well with auto-reverting in
between.

Bye,
Tassilo