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

From: Ship Mints <[email-masked]>
Date: Tue, 21 Jan 2025 15:26:06 -0500

If mode changes take place during processing, you may need to "brand"
inhibit-auto-revert

(put 'inhibit-auto-revert 'permanent-local t)

as mode changes clear impermanent locals.

On Tue, Jan 21, 2025 at 3:19 PM Tassilo Horn <[email-masked]> wrote:

> Tassilo Horn <[email-masked]> writes:
>
> > I'll make the change to the new more general inhibit-auto-revert when
> > time permits.
>
> I have to declare emacs programming bankruptcy.  The current version
> with dired--inhibit-auto-revert works fine.  Now I wanted to change that
> into a more general inhibit-auto-revert.  Here's the patch (with -b to
> make it easier to grasp):
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/lisp/autorevert.el b/lisp/autorevert.el
> index 1dcfe8e911f..0cd0623c59b 100644
> --- a/lisp/autorevert.el
> +++ b/lisp/autorevert.el
> @@ -778,6 +778,11 @@ auto-revert-active-p
>        auto-revert-tail-mode
>        auto-revert--global-mode))
>
> +(defvar-local inhibit-auto-revert nil
> +  "A non-nil value prevents `auto-revert-mode' from reverting the buffer.
> +Should be used in let-bindings to temporarily disable auto-reverts in a
> +buffer.")
> +
>  (defun auto-revert-handler ()
>    "Revert current buffer, if appropriate.
>  This is an internal function used by Auto-Revert Mode."
> @@ -787,6 +792,8 @@ auto-revert-handler
>           ;; the values.
>           (remote-file-name-inhibit-cache t)
>           (revert
> +          (and
> +           (not inhibit-auto-revert)
>             (if buffer-file-name
>                 (and (or auto-revert-remote-files
>                          (not (file-remote-p buffer-file-name)))
> @@ -805,7 +812,7 @@ auto-revert-handler
>                        global-auto-revert-non-file-buffers)
>                    (funcall (or buffer-stale-function
>                                 #'buffer-stale--default-function)
> -                          t))))
> +                           t)))))
>           eob eoblist)
>      (setq auto-revert-notify-modified-p nil
>            auto-revert--last-time (current-time))
> diff --git a/lisp/dired.el b/lisp/dired.el
> index d2071d80bf3..7c9d7310efb 100644
> --- a/lisp/dired.el
> +++ b/lisp/dired.el
> @@ -944,9 +944,6 @@ dired-mark-if
>                            ""))))
>      (and (> count 0) count)))
>
> -(defvar-local dired--inhibit-auto-revert nil
> -  "A non-nil value prevents `auto-revert-mode' from reverting the
> buffer.")
> -
>  (defmacro dired-map-over-marks (body arg &optional show-progress
>                                      distinguish-one-marked)
>    "Eval BODY with point on each marked line.  Return a list of BODY's
> results.
> @@ -983,8 +980,8 @@ dired-map-over-marks
>    ;;endless loop.
>    ;;This warning should not apply any longer, sk  2-Sep-1991 14:10.
>    `(prog1
> -       (let ((dired--inhibit-auto-revert t)
> -             (inhibit-read-only t)
> +       (let ((inhibit-read-only t)
> +             (inhibit-auto-revert t)
>               case-fold-search found results)
>          (if (and ,arg (not (eq ,arg 'marked)))
>              (if (integerp ,arg)
> @@ -1294,12 +1291,6 @@ 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, dired--inhibit-auto-revert 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 dired--inhibit-auto-revert)
>          (dired-directory-changed-p dirname))))
>
>  (defcustom dired-auto-revert-buffer nil
> @@ -4089,13 +4080,12 @@ dired-internal-do-deletions
>             (while l
>               (goto-char (marker-position (cdr (car l))))
>                (dired-move-to-filename)
> -             (let ((inhibit-read-only t))
> +             (let ((inhibit-read-only t)
> +                    ;; Temporarily prevent auto-revert while deleting
> +                    ;; entry in the dired buffer (bug#71264).
> +                    (inhibit-auto-revert t))
>                 (condition-case err
> -                   (let ((fn (car (car l)))
> -                          ;; Temporarily prevent auto-revert while
> -                          ;; deleting entry in the dired buffer
> -                          ;; (bug#71264).
> -                          (auto-revert-mode nil))
> +                   (let ((fn (car (car l))))
>                       (dired-delete-file fn dired-recursive-deletes trash)
>                       ;; if we get here, removing worked
>                       (setq succ (1+ succ))
> --8<---------------cut here---------------end--------------->8---
>
> That last hunk in dired-internal-do-deletions is due to a wrong fix for
> the bug#71264.  When auto-revert-mode itself is bound to nil when
> auto-revert kicks in, the buffer will be removed from
> auto-revert-buffer-list causing auto-revert to be disabled forever in
> that buffer.  (At least that's my reading of the code...)
>
> Anyway, the above solution with the new inhibit-auto-revert does not
> work.  auto-revert-buffers is called from a timer and eventually
> auto-revert-handler is called for my dired buffer where the compression
> of 1000 is still ongoing but inhibit-auto-revert is nil there and
> revert-buffer is called causing the issue of this bug again.
>
> I can't understand why it doesn't see the non-nil inhibit-auto-revert
> binding in the expansion of dired-map-over-marks.  Where is the
> difference to the current working solution with
> dired--inhibit-auto-revert?  It's bound the same way and accessed from
> auto-revert-handler via the funcall to the dired buffer's
> buffer-stale-function, i.e., dired-buffer-stale-p.
>
> Bye,
> Tassilo
>
>
>
>