Subject: 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 20:39:58 +0100

Drew Adams <[email-masked]> writes:

>> > That shouldn't happen, IMO.  Too general, and I doubt it's needed.
>> 
>> This bug contains a recipe showing at least one ocassion where it is
>> needed.
>
> It's needed for the _macro_ to do?  I don't see
> that demonstrated.
>
> An occasion where the macro is used and you want
> to prevent XYZ should be handled by the _code
> that invokes the macro_, not by the macro itself,
> i.e., not by expanding the macro.

That would be possible, too.  The difference is just one change to the
macro vs. N changes to different functions like dired-do-compress.

>> > One might very well want to allow reversion
>> > during some particular operation on marked
>> > files.  Let's not assume otherwise.
>> 
>> Sure, and that's still allowed, e.g., the code given as BODY of
>> dired-map-over-marks could explicitly call revert-buffer if it can
>> handle the result.
>
> When you say BODY, do you mean the _function_
> passed to the macro as its ARG, or the BODY
> argument?

It doesn't get a function, it gets a form like (funcall
#'dired-compress) in the case of dired-do-compress.  You could give it
somithing like (progn (funcall #'whatever) (revert-buffer)) when you are
certain that reverting after the operation is safe.

> In any case, how is an invocation of the macro
> supposed to override the denial of reversion?
> Can it simply let-bind a variable around the
> macro call?  I was guessing that, with your
> change the macro code itself would override
> that, e.g., with its own such binding, making
> it impossible to control the behavior from
> _around_ the macro call.

That's true.

>> The point is that auto-revert-mode reverts at _unpredictable_ moments
>> where chances are high that the dired buffer contents change in a way
>> that the processing logic goes wrong, e.g., a marked and not yet
>> processed file is now before point and will be skipped, or the other
>> way round, an already processed file is now after point and will be
>> processed again.
>
> Yes, I made clear that I understand that.
> And I explicitly agreed that that's a no-no.
>
> My point was that, until now, it was up to
> a _user_ to just _not do that_, i.e., not
> to shoot herself in the foot.  IIUC, Emacs is
> now preventing her from reverting the buffer,
> including, but not limited to, via
> `auto-revert-mode'.

That's not true.  The user could not manually revert before because as
Eli explained, Emacs isn't processing key events during the long-running
operation on marked files through dired-map-over-marks.  In contrast,
auto-revert-mode reverts from the Emacs main loop.

> If so, I'd prefer the original, more general
> behavior: leave it up to the _calling_ code to
> decide whether to limit the behavior in that
> way (or in any other way).  If it's important
> for the particular use case to prevent doing
> XYZ then the _calling code_ can, and should,
> prevent doing XYZ.  The macro shouldn't try to
> guess what should be prevented - even in the
> case of buffer reversion, which, I agree, is
> usually something to be prevented.
>> 
>> I don't see what feature you think I have stolen from you.  We just
>> prevent auto-revert-mode from reverting the dired buffer as long as
>> an operation on marked files is in progress.
>
> Why?  Because usually that's a good thing to
> prevent?  Not good/general enough.  Leave it up
> to calling code to prevent that.  Add a note
> about this to the doc string, if you like.  But
> why have the _macro_ prevent it?

Because it catches a category of (potential) errors at a central
location.  It's not unthinkable that users wrote their own processing
functions which are also vulnerable to misbehave if auto-revert-mode is
activated.

And keep in mind that the changes that make auto-revert-mode revert
don't need to be "our" changes.  For example, assume you have a dired
buffer for /tmp, do some operation on marked files, and some other
process creates files in /tmp causing auto-reverts.  That will produce
new lines at random locations in your dired buffer which is currently
processed marked line by marked line with code that relies on the
position of point in that buffer.  How should processing code handle
that?  I'd argue it can't and we should make sure the buffer stays
stable during the operation.

>> Progress is still visible
>> (SHOW-PROGRESS arg of dired-map-over-marks), i.e., the dired buffer is
>> periodically redisplayed showing the changes so far because that has
>> nothing to do with auto-revert-mode.
>
> No one questioned visibility of progress.
> Dunno why you mention that.

I had the impression that this could be the reason for you vehement
disagreement with the change.

Anyway, can you come up with some concrete scenario where inhibiting
auto-revert-mode for a dired marked files operation could do harm or
have any negative effect?  That's the thing I don't get.

Bye,
Tassilo