Re: autorevert.el

[Luc Teirlinck]

Stefan Monnier wrote:

   Most of the patch looks right (the list-diff thing was just horrendous and
   showed a serious misunderstanding), except I'd try to merge a bit more the
   dired and non-dired checks since both check the modtime.

Not all parts of the function check the modtime and the two parts
where the modtime gets checked do it in different ways (dired can not
use `verify-visited-file-modtime', because it is not visiting a file).
It seems that the only thing that is done unconditionally in all cases
is check (buffer-modified-p), so I changed the function to first check
this before splitting into cases.

   Also, I'd use file-remote-p rather than (string-match "@"),
   especially since remote files might not include any @.

You are right.  I kept this from the original code.  I did not check
this carefully enough.  Note that I neither checked the code
of the vc-part of the original patch, not tested it.

   As for the auto-revert-flag thingy, I find it truly ugly because I don't
   like dired knowing about auto-revert just for the sake of verbosity control.
   There's got to be a better way.

Like what?  I need some way to tell not only dired, but other non-file
buffers as well, not to display the revert messages they usually do.
The only real way I see is using a flag variable and I need one
variable for all non-file buffers.  The only alternative I see would
be a similar variable inhibiting revert messages not "overtly" tied to
auto-revert.el.  I do not believe that would make a lot of difference
in practice, but it could be done.  Note that the code in question
first checks: (boundp 'auto-revert-flag), so if autoreverting is not
already in use, auto-revert.el will not be loaded (that would indeed
be terribly ugly).

I do not know whether you question the usefulness of the verbosity
control itself.  I implemented this because I felt a definite need for
it.  Constant gratuitous echo area messages are a distracting
nuisance, not to mention being overshouted by them while trying to use
the minibuffer and just hesitating slightly too long.

Here are the changes I propose to my original autorevert.el patch.
Essentially, I replaced the three separate (buffer-modified-p) checks
by one single one at the very start and used file-remote-p instead of
(string-match "@").  I could change the auto-revert-flag stuff once I
understand your concrete objections better.  Would a
not-officially-linked-to-autorevert.el flag variable disabling revert
messages (when set) in various non-file buffers, including dired
buffers, take care of your objections?  That would be easy to do.
However, I definitely want to keep the functionality as a user option,
disabled by default.

Diff with previous version of the patch:

===File ~/auto-revert-newdiff===============================
diff -c /home/teirllm/autorevert.prev.el 
/home/teirllm/emacscvsdir/emacs/lisp/autorevert.el
*** /home/teirllm/autorevert.prev.el    Wed Mar  3 17:38:24 2004
--- /home/teirllm/emacscvsdir/emacs/lisp/autorevert.el  Wed Mar  3 17:44:23 2004
***************
*** 278,312 ****
    "Check if current buffer should be reverted."
    ;;  - Always include VC buffers to list.  It would be too expensive
    ;;  to test the "revert" status here each time timer launches.
!   (or (and (eq major-mode 'dired-mode)
!          (or (and global-auto-revert-mode
!                   global-auto-revert-non-file-buffers)
!              auto-revert-mode)
!          (not (buffer-modified-p))
!          (stringp dired-directory)
!          ;; Exclude remote buffers, would be too slow for user
!          ;; modem, timeouts, network lag ... all is possible
!          (not (string-match "@" dired-directory))
!          (file-directory-p dired-directory)
!            (file-readable-p dired-directory)
!          (not (let ((attributes (file-attributes dired-directory))
!                     (modtime (visited-file-modtime)))
!                 (or (eq modtime 0)
!                     (not (eq (car attributes) t))
!                     (and (= (car (nth 5 attributes)) (car modtime))
!                          (= (nth 1 (nth 5 attributes)) (cdr modtime)))))))
!       (and (not (eq major-mode 'dired-mode))
!          (not (buffer-modified-p))
!          (auto-revert-vc-buffer-p))
!       (and (not (eq major-mode 'dired-mode))
!          (not (buffer-modified-p))
!          (if (buffer-file-name)
!              (and (file-readable-p (buffer-file-name))
!                   (not (verify-visited-file-modtime (current-buffer))))
!            (and revert-buffer-function
!                 (or (and global-auto-revert-mode
!                          global-auto-revert-non-file-buffers)
!                     auto-revert-mode))))))
  
  (defun auto-revert-vc-cvs-file-version (file)
    "Get version of FILE by reading control file on disk."
--- 278,310 ----
    "Check if current buffer should be reverted."
    ;;  - Always include VC buffers to list.  It would be too expensive
    ;;  to test the "revert" status here each time timer launches.
!   (unless (buffer-modified-p)
!     (or (and (eq major-mode 'dired-mode)
!            (or (and global-auto-revert-mode
!                     global-auto-revert-non-file-buffers)
!                auto-revert-mode)
!            (stringp dired-directory)
!            ;; Exclude remote buffers, would be too slow for user
!            ;; modem, timeouts, network lag ... all is possible
!            (not (file-remote-p dired-directory))
!            (file-directory-p dired-directory)
!            (file-readable-p dired-directory)
!            (not (let ((attributes (file-attributes dired-directory))
!                       (modtime (visited-file-modtime)))
!                   (or (eq modtime 0)
!                       (not (eq (car attributes) t))
!                       (and (= (car (nth 5 attributes)) (car modtime))
!                            (= (nth 1 (nth 5 attributes)) (cdr modtime)))))))
!       (and (not (eq major-mode 'dired-mode))
!            (auto-revert-vc-buffer-p))
!       (and (not (eq major-mode 'dired-mode))
!            (if (buffer-file-name)
!                (and (file-readable-p (buffer-file-name))
!                     (not (verify-visited-file-modtime (current-buffer))))
!              (and revert-buffer-function
!                   (or (and global-auto-revert-mode
!                            global-auto-revert-non-file-buffers)
!                       auto-revert-mode)))))))
  
  (defun auto-revert-vc-cvs-file-version (file)
    "Get version of FILE by reading control file on disk."

Diff finished.  Wed Mar  3 21:57:57 2004
============================================================