Package: emacs;
Reported by: Tassilo Horn <tsdh <at> gnu.org>
Date: Fri, 17 Jan 2025 07:43:01 UTC
Severity: normal
Found in version 31.0.50
Done: Tassilo Horn <tsdh <at> gnu.org>
Bug is archived. No further changes may be made.
Message #196 received at 75626 <at> debbugs.gnu.org (full text, mbox):
From: Tassilo Horn <tsdh <at> gnu.org> To: Michael Heerdegen <michael_heerdegen <at> web.de> Cc: 75626 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, michael.albinus <at> gmx.de Subject: Re: bug#75626: 31.0.50; Dired misses or double-processes files when auto-revert-mode is enabled Date: Wed, 22 Jan 2025 09:05:22 +0100
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Tassilo Horn <tsdh <at> gnu.org> writes:
>
>> 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):
>
> Unfortunately it doesn't apply here.
Yeah, because of the "git diff -b" flag omitting whitespace changes.
>> 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...)
>
> I think your reading is correct.
Thanks for validating.
>> 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.
>
> I hope it's not related to the auto-revert-mode -> nil binding?
Nope, that's what I reverted first to check if that was the culprit.
> Anyway, as Ship Mints already mentioned, you never created a buffer
> local binding. A `let' binds the visible binding which is in your
> case the global one - since there was no local variable binding yet.
Ah, indeed. So now I (setq inhibit-auto-revert nil) in dired-mode to
create a local variable in the buffer.
> So you bind the global variable - which is not what we want but that
> binding should be visible nonetheless. I'm not sure why it does not
> work as expected.
Me neither.
> In some situations it can help to use a variable watcher to log
> variable binding changes:
>
> (info "(elisp) Watching Variables")
Oh, that's a great hint! I've tried with this watch:
--8<---------------cut here---------------start------------->8---
(add-variable-watcher 'inhibit-auto-revert
(lambda (sym new-val op where)
(message "(%S %S %S) in %S"
op sym new-val where)))
--8<---------------cut here---------------end--------------->8---
And what should I say, it still didn't work in the beginning, i.e., the
previous version of my last mail + (setq inhibit-auto-revert nil) in
dired-mode to create a local variable. The watcher showed that after
confirming the compression of my 1000 test files, a set with value nil
was done but I have no clue from where.
Then I edited a bit back and forth and recompiled and tested (with emacs
-Q) after each edit without actually changing any significant part,
e.g., I tried to also let-bind inhibit-auto-revert to t in the function
dired-map-over-marks-check which uses the macro dired-map-over-marks.
Still not working. So reverting that again. And now I have the patch
at the end of this mail AND IT SUDDENLY WORKS.
The output including the watcher is this (commented by me):
;; That's for the confirmation query
(let inhibit-auto-revert t) in #<buffer test>
(unlet inhibit-auto-revert nil) in #<buffer test>
Compress or uncompress * [1000 files]? (y or n) y
;; That's the actual compression of the 1000 files
(let inhibit-auto-revert t) in #<buffer test>
(unlet inhibit-auto-revert nil) in #<buffer test>
Compress or uncompress: 1000 files.
Reverting buffer ‘test’
So exactly as one might expect. And the last line suggests that
inhibiting some auto-reverts is not bad either, it'll be reverted in the
next round.
But I really wonder what could have caused that it didn't work
initially. Yesterday, I've rebuilt emacs without native compiler in
order to rule that out as the culprit. Could it be that a
non-native-comp emacs still picks up outdated eln files? Well, then the
question would be why it doesn't anymore...
Anyway, below the current patch. It would be great if someone else
could test it, too.
Obviously, the new inhibit-auto-revert should be mentioned in NEWS and
the elisp info docs which I'll do and post the final patch for review
before committing.
Bye,
Tassilo
[inhibit-auto-revert.patch (text/x-patch, inline)]
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,25 +792,27 @@ auto-revert-handler
;; the values.
(remote-file-name-inhibit-cache t)
(revert
- (if buffer-file-name
- (and (or auto-revert-remote-files
- (not (file-remote-p buffer-file-name)))
- (or (not auto-revert-notify-watch-descriptor)
- auto-revert-notify-modified-p)
- (if auto-revert-tail-mode
- (and (file-readable-p buffer-file-name)
- (/= auto-revert-tail-pos
- (setq size
- (file-attribute-size
- (file-attributes buffer-file-name)))))
- (funcall (or buffer-stale-function
- #'buffer-stale--default-function)
- t)))
- (and (or auto-revert-mode
- global-auto-revert-non-file-buffers)
- (funcall (or buffer-stale-function
- #'buffer-stale--default-function)
- t))))
+ (and
+ (not inhibit-auto-revert)
+ (if buffer-file-name
+ (and (or auto-revert-remote-files
+ (not (file-remote-p buffer-file-name)))
+ (or (not auto-revert-notify-watch-descriptor)
+ auto-revert-notify-modified-p)
+ (if auto-revert-tail-mode
+ (and (file-readable-p buffer-file-name)
+ (/= auto-revert-tail-pos
+ (setq size
+ (file-attribute-size
+ (file-attributes buffer-file-name)))))
+ (funcall (or buffer-stale-function
+ #'buffer-stale--default-function)
+ t)))
+ (and (or auto-revert-mode
+ global-auto-revert-non-file-buffers)
+ (funcall (or buffer-stale-function
+ #'buffer-stale--default-function)
+ 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..eef1fdd50dc 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
@@ -2796,6 +2787,7 @@ dired-mode
mode-name "Dired"
;; case-fold-search nil
buffer-read-only t
+ inhibit-auto-revert nil
mode-line-buffer-identification
(propertized-buffer-identification "%17b"))
(add-to-invisibility-spec '(dired . t))
@@ -4089,13 +4081,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))
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.