bug#457: wdired and auto-revert-mode

bug#457: wdired and auto-revert-mode

[Joe Wells]

martin rudalics <rudalics@gmx.at> writes:

>> Changing to wdired should temporarily turn off `auto-revert-mode' if
>> it was enabled in the corresponding dired buffer. It is very
>> frustrating when the buffer reverts while editing file names.

The correct way to handle this is to mark the buffer as modified
during the period for which it should not be auto-reverted.  The
function auto-revert-handler refuses to do anything with buffers that
are marked as modified.  The first thing auto-revert-handler does is
check the condition (buffer-modified-p).

-- 
Joe

> Could you please try whether the attached patch handles the problem.
>
> Thanks, martin.
>
> *** dired.el.~1.406.~	2008-07-30 09:30:57.453125000 +0200
> --- dired.el	2008-08-03 10:43:25.921875000 +0200
> ***************
> *** 717,722 ****
> --- 717,725 ----
>       (and (stringp dirname)
>   	 (not (when noconfirm (file-remote-p dirname)))
>   	 (file-readable-p dirname)
> + 	 ;; Don't auto-revert when the dired buffer can be currently
> + 	 ;; written by the user (as in `wdired-mode', see bug#457).
> + 	 buffer-read-only
>   	 (dired-directory-changed-p dirname))))
>   
>   ;; Separate function from dired-noselect for the sake of dired-vms.el.


-- 
Heriot-Watt University is a Scottish charity
registered under charity number SC000278.

bug#457: wdired and auto-revert-mode

[martin rudalics]

 > The correct way to handle this is to mark the buffer as modified
 > during the period for which it should not be auto-reverted.

No.  `buffer-modified-p' is busy telling whether the user has modified
the buffer with `wdired'.  It's used many times in wdired.el.

 > The
 > function auto-revert-handler refuses to do anything with buffers that
 > are marked as modified.  The first thing auto-revert-handler does is
 > check the condition (buffer-modified-p).

martin

bug#457: wdired and auto-revert-mode

[Joe Wells]

martin rudalics <rudalics@gmx.at> writes:

>> The correct way to handle this is to mark the buffer as modified
>> during the period for which it should not be auto-reverted.
>
> No.  `buffer-modified-p' is busy telling whether the user has modified
> the buffer with `wdired'.  It's used many times in wdired.el.

Just make sure to leave the buffer marked as modified as long as there
are changes that should not be lost by reverting.  If you do that,
then auto-revert-mode will not lose changes.

Is something in wdired doing (set-buffer-modified-p nil) *before* the
changes have been recorded?

-- 
Joe

>> The
>> function auto-revert-handler refuses to do anything with buffers that
>> are marked as modified.  The first thing auto-revert-handler does is
>> check the condition (buffer-modified-p).
>
> martin


-- 
Heriot-Watt University is a Scottish charity
registered under charity number SC000278.

bug#457: wdired and auto-revert-mode

[martin rudalics]

 > Just make sure to leave the buffer marked as modified as long as there
 > are changes that should not be lost by reverting.  If you do that,
 > then auto-revert-mode will not lose changes.

At the very moment the OP got "frustrated", the buffer was apparently
_unmodified_ since otherwise Emacs would have refused to auto-revert it.
Hence, the problem seems not in losing any changes.

 > Is something in wdired doing (set-buffer-modified-p nil) *before* the
 > changes have been recorded?

Yes, but why should that matter here?  BTW, `wdired' also resets
`buffer-undo-list'.  The important aspect is that as long as a user is
in `wdired-mode', buffer-modified status and `buffer-undo-list' should
faithfully reflect the user's editing steps.

Did you try my `buffer-read-only' patch?  I use neither `dired' nor
`auto-revert-mode'.

martin

bug#457: wdired and auto-revert-mode

[Joe Wells]

martin rudalics <rudalics@gmx.at> writes:

>> Just make sure to leave the buffer marked as modified as long as there
>> are changes that should not be lost by reverting.  If you do that,
>> then auto-revert-mode will not lose changes.
>
> At the very moment the OP got "frustrated", the buffer was apparently
> _unmodified_ since otherwise Emacs would have refused to auto-revert it.
> Hence, the problem seems not in losing any changes.

The original bug report stated this:

  “It is very frustrating when the buffer reverts while editing file
   names.”

This seems to indicate that Michael was complaining about the buffer
getting reverted while he was in the middle of editing file names.
I'm not sure how this can happen unless something is clearing the
buffer-modified flag, because auto-revert-handler refuses to do
anything if the buffer-modified flag is set.

Perhaps Michael can clarify what exactly the bad behavior was?

>> Is something in wdired doing (set-buffer-modified-p nil) *before* the
>> changes have been recorded?
>
> Yes, but why should that matter here?  BTW, `wdired' also resets
> `buffer-undo-list'.  The important aspect is that as long as a user is
> in `wdired-mode', buffer-modified status and `buffer-undo-list' should
> faithfully reflect the user's editing steps.

As long as the buffer is marked as modified, it will not be
auto-reverted.  If it only gets marked unmodified after the user is
done editing file names, then I don't see what the problem is.

It is hard to make any recommendation without a clearer description of
how to reproduce the problem, whatever the problem might be.

> Did you try my `buffer-read-only' patch?

No.

> I use neither `dired' nor `auto-revert-mode'.

-- 
Joe


-- 
Heriot-Watt University is a Scottish charity
registered under charity number SC000278.

bug#457: wdired and auto-revert-mode

[martin rudalics]

 > This seems to indicate that Michael was complaining about the buffer
 > getting reverted while he was in the middle of editing file names.
 > I'm not sure how this can happen unless something is clearing the
 > buffer-modified flag, because auto-revert-handler refuses to do
 > anything if the buffer-modified flag is set.

The only way this can happen is after starting `wdired', when a user has
undone his editing steps (`primitive-undo' may clear it), or when a user
explicitly resets it.

 > As long as the buffer is marked as modified, it will not be
 > auto-reverted.  If it only gets marked unmodified after the user is
 > done editing file names, then I don't see what the problem is.

See above.

 >>Did you try my `buffer-read-only' patch?
 >
 > No.

Hmmm ...

 >>I use neither `dired' nor `auto-revert-mode'.

... so I'm still looking for someone who does,

martin

bug#457: wdired and auto-revert-mode

[Michael Heerdegen]

I'll try to make clear what happened to the best of my recollection:


(0) C-x d my-dir

(1) I marked some files with % *, m, u etc.

(2) I killed the formerly marked files in the dired buffer (k).

(3) I pressed r (`wdired-change-to-wdired-mode') (resulting in making
the buffer non-modified again).

(4) Directly after that, M-x query-replace-regexp. While I was
specifying the regexp, the buffer reverted, and I had to restart with (1).

bug#457: wdired and auto-revert-mode

[Joe Wells]

Michael Heerdegen <michaelh@minet.uni-jena.de> writes:

> I'll try to make clear what happened to the best of my recollection:
>
> (0) C-x d my-dir
>
> (1) I marked some files with % *, m, u etc.
>
> (2) I killed the formerly marked files in the dired buffer (k).
>
> (3) I pressed r (`wdired-change-to-wdired-mode') (resulting in making
> the buffer non-modified again).

Ahh!  That's the bug: wdired-change-to-wdired-mode does
(set-buffer-modified-p nil), which incorrectly forgets about
modifications made by other commands.

In this case, (buffer-modified-p) is a poor proxy for “there exist
uncommitted changes that the user probably does not want to lose”.
The buffer-modified flag is mostly good enough for buffers visiting
ordinary files, but is hard to keep correct for fancy buffers like
Dired buffers.  Of course, this wasn't so important before
auto-revert-mode existed.

-- 
Joe

> (4) Directly after that, M-x query-replace-regexp. While I was
> specifying the regexp, the buffer reverted, and I had to restart with (1).

bug#457: wdired and auto-revert-mode

[Michael Heerdegen]

Joe Wells writes:
 > In this case, (buffer-modified-p) is a poor proxy for ^[$,1r|^[(Bthere exist
 > uncommitted changes that the user probably does not want to lose^[$,1r}^[(B.
 > The buffer-modified flag is mostly good enough for buffers visiting
 > ordinary files, but is hard to keep correct for fancy buffers like
 > Dired buffers.  Of course, this wasn't so important before
 > auto-revert-mode existed.

Yes. And this is also the reason for another, much worse bug which
makes auto-revert-mode in dired nearly useless:

If you copy files etc. (what else should you do with dired?), your
buffer gets buffer-modified-p, which means that auto-reverting doesn't
take place any more until you revert manually with g.

bug#457: wdired and auto-revert-mode

[martin rudalics]

 >  > The buffer-modified flag is mostly good enough for buffers visiting
 >  > ordinary files, but is hard to keep correct for fancy buffers like
 >  > Dired buffers.  Of course, this wasn't so important before
 >  > auto-revert-mode existed.
 >
 > Yes. And this is also the reason for another, much worse bug which
 > makes auto-revert-mode in dired nearly useless:
 >
 > If you copy files etc. (what else should you do with dired?), your
 > buffer gets buffer-modified-p, which means that auto-reverting doesn't
 > take place any more until you revert manually with g.

The buffer-modified status of dired-buffers has no significance: Do
`dired-mark' for some file and undo that via `dired-unmark' or plain
`undo'.  The buffer will remain to appear modified.  Do `dired-unmark'
for some file in a fresh dired buffer.  The buffer will appear modified.
We'd have to specify the semantics of modifiedness for dired buffers
first.  Then we could decide what to do when a directory is modified by
external intervention and autorevert picks up those changes.

OTOH wdired DTRT WRT `undo' and we probably shouldn't let autorevert
interfere with this.

martin

bug#457: wdired and auto-revert-mode

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

 > Changing to wdired should temporarily turn off `auto-revert-mode' if
 > it was enabled in the corresponding dired buffer. It is very
 > frustrating when the buffer reverts while editing file names.

Could you please try whether the attached patch handles the problem.

Thanks, martin.

[-- Attachment #2: 457.patch --]
[-- Type: text/plain, Size: 534 bytes --]

*** dired.el.~1.406.~	2008-07-30 09:30:57.453125000 +0200
--- dired.el	2008-08-03 10:43:25.921875000 +0200
***************
*** 717,722 ****
--- 717,725 ----
      (and (stringp dirname)
  	 (not (when noconfirm (file-remote-p dirname)))
  	 (file-readable-p dirname)
+ 	 ;; Don't auto-revert when the dired buffer can be currently
+ 	 ;; written by the user (as in `wdired-mode', see bug#457).
+ 	 buffer-read-only
  	 (dired-directory-changed-p dirname))))
  
  ;; Separate function from dired-noselect for the sake of dired-vms.el.