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.
View this message in rfc822 format
From: Drew Adams <drew.adams <at> oracle.com> To: Tassilo Horn <tsdh <at> gnu.org> Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, "75626 <at> debbugs.gnu.org" <75626 <at> debbugs.gnu.org>, Eli Zaretskii <eliz <at> gnu.org> Subject: bug#75626: 31.0.50; Dired misses or double-processes files when auto-revert-mode is enabled Date: Mon, 20 Jan 2025 22:32:48 +0000
> >> > 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. Requiring a user to change the macro is not what I'd call "the code that invokes the macro" preventing XYZ. You seem to be saying that a user can always redefine the macro. That excuse could be offered for _any_ change to the Emacs code: don't like it, just change the code. > >> > 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. No. It gets two (possibly 3 or 4) arguments: BODY, ARG, and possibly SHOW-PROGRESS and DISTINGUISH-ONE-MARKED. `C-h f'-it, or check its definition in the code. > > 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. So you admit that a code invoking the macro can't override the behavior you're now hard-coding in it. > >> 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. We're (I'm) talking about user code, not user altering the behavior interactively. It's not about "manually revert"ing. It's about letting `auto-revert-mode' revert during `dired-map-over-marks'. And to _disallow_ reversion (auto or other) during `d-m-o-m' user code can, now, just bind `revert-buffer-function' to `ignore', around the call to `dired-map-over-marks'. If you change `dired-map-over-marks' then user code can't have any such effect - you're hard-coding prevention of reversion during `d-m-o-m'. > > 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. That might sound reasonable, IF you also provided a way for user code to override that prohibition of reverting during `d-m-o-m'. > 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. That's why I agreed that usually users will want to prohibit reversion during `d-m-o-m'. That's not the same thing as hard-coding that prohibition and not giving user code an easy way to override it. > >> 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. No. And I don't have a vehement disagreement. I'm not convinced that the solution being implemented is required. Seems like overkill, to me. At least provide a variable that the `d-m-o-m' will respect whose value you can change to not prohibit <whatever>, including reversion. That costs nothing, and you can even tell users why they generally won't want to change the variable value. > 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. The negative effect is in removing the possibility of allowing reversion during `d-m-o-m'. What's the argument for hard-coding disallowing it, not letting code allow it, conditionally or otherwise? I haven't seen an argument for that. I've seen only an argument (of which I'm convinced, and have said so several times) that most of the time it's better to disallow it. Disallowing it always is too rigid - not the same as disallowing it by default but allowing user code to do whatever it wants.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.