autorevert.el

autorevert.el

[Luc Teirlinck]

This is an amended copy of something I sent to emacs-devel on
Thursday.  I do not expect the original to ever arrive.

Any objections to me applying the patch below?  I have been using it
without any problems for two weeks now.

The purpose is to be able to autorevert dired buffers without
excessive CPU-usage and without continuously being overshouted by
revert messages when trying to use the minibuffer.

If one customizes dired in the "wrong" way (say, not using the "a"
switch) or uses the "wrong" dired functionality (for instance, listing
subdirectories) this is currently not possible (without my patch
applied).

Also, without my patch applied, various other dired functionality, for
instance dired-undo, no longer properly works if one tries to
auto-revert dired buffers.

There is one addition to the patch I proposed (but did not install)
two weeks ago.  That is the new user-option
`auto-revert-no-secondary-messages' which allows to suppress the usual
"Reading directory %s...done" type messages.  The default is nil, do
not suppress them.  Without this option or with the option set to nil,
one still (even after applying my previous patch) can get overshouted
in the minibuffer while, say, bootstrapping or stuff like that.  With
the variable set to t, one can visit every single directory and
subdirectory in the Emacs CVS tree, so they all will get updated, and
then bootstrap (or do similar stuff that continuously updates the
directories), without any minibuffer-overshouting or CPU-usage
problems.

===File ~/autorevert.el-diff================================
*** autorevert.el.~1.21.~	Mon Feb 16 20:04:45 2004
--- autorevert.el	Thu Feb 26 20:43:07 2004
***************
*** 158,164 ****
  When non-nil, both file buffers and buffers with a custom
  `revert-buffer-function' are reverted by Global Auto-Revert Mode.
  
! Use this option with care since it could lead to excessive reverts."
    :group 'auto-revert
    :type 'boolean)
  
--- 158,178 ----
  When non-nil, both file buffers and buffers with a custom
  `revert-buffer-function' are reverted by Global Auto-Revert Mode.
  
! Use this option with care since it could lead to excessive reverts.
! Note also that for some non-file buffers the check whether the
! buffer needs updating may be imperfect, due to efficiency
! considerations, and may not take all information listed in the
! buffer into account.  Hence, a non-nil value for this option does
! not necessarily make manual updates useless for non-file buffers."
!   :group 'auto-revert
!   :type 'boolean)
! 
! (defcustom auto-revert-no-secondary-messages nil
!   "If non-nil, Auto-Revert Mode suppresses secondary revert messages.
! These are messages generated by a non-file buffer's `revert-buffer-function'.
! This variable only works for buffers whose`revert-buffer-function'
! supports it.  To suppress revert messages generated by Auto-Revert Mode
! itself, use `auto-revert-verbose'."
    :group 'auto-revert
    :type 'boolean)
  
***************
*** 260,312 ****
         (not (memq major-mode
  		  global-auto-revert-ignore-modes)))))
  
- (defun auto-revert-list-diff (a b)
-   "Check if strings in list A differ from list B."
-   (when (and a b)
-     (setq a (sort a 'string-lessp))
-     (setq b (sort b 'string-lessp))
-     (let (elt1 elt2)
-       (catch 'break
- 	(while (and (setq elt1 (and a (pop a)))
- 		    (setq elt2 (and b (pop b))))
- 	  (if (not (string= elt1 elt2))
- 	      (throw 'break t)))))))
- 
- (defun auto-revert-dired-file-list ()
-   "Return list of dired files."
-   (let (file list)
-     (save-excursion
-       (goto-char (point-min))
-       (while (not (eobp))
- 	(if (setq file (dired-get-filename t t))
- 	    (push file list))
- 	(forward-line 1)))
-     list))
- 
- (defun auto-revert-dired-changed-p ()
-   "Check if dired buffer has changed."
-   (when (and (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))
-     (let ((files (directory-files dired-directory))
- 	  (dired (auto-revert-dired-file-list)))
-       (or (not (eq (length files) (length dired)))
- 	  (auto-revert-list-diff files dired)))))
- 
  (defun auto-revert-buffer-p ()
    "Check if current buffer should be reverted."
!   ;;  - Always include dired buffers to list.  It would be too expensive
    ;;  to test the "revert" status here each time timer launches.
-   ;;  - Same for VC buffers.
    (or (and (eq major-mode 'dired-mode)
  	   (or (and global-auto-revert-mode
  		    global-auto-revert-non-file-buffers)
! 	       auto-revert-mode))
!       (and (not (buffer-modified-p))
  	   (auto-revert-vc-buffer-p))
!       (and (not (buffer-modified-p))
  	   (if (buffer-file-name)
  	       (and (file-readable-p (buffer-file-name))
  		    (not (verify-visited-file-modtime (current-buffer))))
--- 274,305 ----
         (not (memq major-mode
  		  global-auto-revert-ignore-modes)))))
  
  (defun auto-revert-buffer-p ()
    "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))))
***************
*** 369,382 ****
  	  ;; TODO:
  	  ))))))
  
  (defun auto-revert-handler ()
    "Revert current buffer."
    (let (revert)
      (cond
       ((eq major-mode 'dired-mode)
        ;;  Dired includes revert-buffer-function
!       (when (and revert-buffer-function
! 		 (auto-revert-dired-changed-p))
  	(setq revert t)))
       ((auto-revert-vc-buffer-p)
        (when (auto-revert-handler-vc)
--- 362,379 ----
  	  ;; TODO:
  	  ))))))
  
+ (defvar auto-revert-flag nil
+   "Bound to t by `auto-revert-handler'.
+ If non-nil, the current function was called as part of auto-reverting
+ a buffer.  This is used to implement `auto-revert-no-secondary-messages'.")
+ 
  (defun auto-revert-handler ()
    "Revert current buffer."
    (let (revert)
      (cond
       ((eq major-mode 'dired-mode)
        ;;  Dired includes revert-buffer-function
!       (when revert-buffer-function
  	(setq revert t)))
       ((auto-revert-vc-buffer-p)
        (when (auto-revert-handler-vc)
***************
*** 385,395 ****
  	  revert-buffer-function)
        (setq revert t)))
      (when revert
!       (revert-buffer 'ignore-auto 'dont-ask 'preserve-modes)
!       (if (eq revert 'vc)
! 	  (vc-mode-line buffer-file-name))
!       (if auto-revert-verbose
! 	  (message "Reverting buffer `%s'." (buffer-name))))))
  
  (defun auto-revert-buffers ()
    "Revert buffers as specified by Auto-Revert and Global Auto-Revert Mode.
--- 382,393 ----
  	  revert-buffer-function)
        (setq revert t)))
      (when revert
!       (let ((auto-revert-flag t))
! 	(revert-buffer 'ignore-auto 'dont-ask 'preserve-modes)
! 	(if (eq revert 'vc)
! 	    (vc-mode-line buffer-file-name))
! 	(if auto-revert-verbose
! 	    (message "Reverting buffer `%s'." (buffer-name)))))))
  
  (defun auto-revert-buffers ()
    "Revert buffers as specified by Auto-Revert and Global Auto-Revert Mode.
============================================================

===File ~/dired-diff========================================
*** dired.el.~1.272.~	Tue Feb  3 15:42:36 2004
--- dired.el	Thu Feb 26 20:04:48 2004
***************
*** 618,623 ****
--- 618,625 ----
  \f
  ;; Read in a new dired buffer
  
+ (defvar auto-revert-flag)
+ 
  ;; dired-readin differs from dired-insert-subdir in that it accepts
  ;; wildcards, erases the buffer, and builds the subdir-alist anew
  ;; (including making it buffer-local and clearing it first).
***************
*** 634,640 ****
        ;; based on dired-directory, e.g. with ange-ftp to a SysV host
        ;; where ls won't understand -Al switches.
        (run-hooks 'dired-before-readin-hook)
!       (message "Reading directory %s..." dirname)
        (if (consp buffer-undo-list)
  	  (setq buffer-undo-list nil))
        (let (buffer-read-only
--- 636,645 ----
        ;; based on dired-directory, e.g. with ange-ftp to a SysV host
        ;; where ls won't understand -Al switches.
        (run-hooks 'dired-before-readin-hook)
!       (unless (and (boundp 'auto-revert-flag)
! 		   auto-revert-flag
! 		   auto-revert-no-secondary-messages)
! 	(message "Reading directory %s..." dirname))
        (if (consp buffer-undo-list)
  	  (setq buffer-undo-list nil))
        (let (buffer-read-only
***************
*** 643,649 ****
  	(widen)
  	(erase-buffer)
  	(dired-readin-insert))
!       (message "Reading directory %s...done" dirname)
        (goto-char (point-min))
        ;; Must first make alist buffer local and set it to nil because
        ;; dired-build-subdir-alist will call dired-clear-alist first
--- 648,657 ----
  	(widen)
  	(erase-buffer)
  	(dired-readin-insert))
!       (unless (and (boundp 'auto-revert-flag)
! 		   auto-revert-flag
! 		   auto-revert-no-secondary-messages)
! 	(message "Reading directory %s...done" dirname))
        (goto-char (point-min))
        ;; Must first make alist buffer local and set it to nil because
        ;; dired-build-subdir-alist will call dired-clear-alist first
============================================================

Re: autorevert.el

[Stefan Monnier]

> Any objections to me applying the patch below?  I have been using it
> without any problems for two weeks now.

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.

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.

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


        Stefan

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
============================================================

Re: autorevert.el

[Luc Teirlinck]

>From my previous message:

   Note that I neither checked the code of the vc-part of the original
   patch, not tested it.

Meant was "nor tested it".

Sincerely,

Luc.

Re: autorevert.el

[Stefan Monnier]

> 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.

Too bad we can't do better, but thanks for double checking for me.

>    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.

Well, you've already done more than most of us.

>    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?

Dunno!

> 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.

Like which others?  And why does dired display a message anyway?
It seems just wrong to me, since the user knows he reverted the buffer if
he did M-x revert-buffer (and indeed M-x revert-buffer in a file buffer
does not output any message).  It seems these messages should be output
(when needed) by the code that calls revert-buffer-function.
As does auto-revert, for example.
I.e. I suggest to just throw out the dired messages.

> I do not know whether you question the usefulness of the verbosity
> control itself.

No, your example of `make recompile' was pretty compelling already ;-)


        Stefan

Re: autorevert.el

[Luc Teirlinck]

Stefan Monnier wrote:

   Too bad we can't do better, but thanks for double checking for me.

I am sure we could save a few lines of duplicate code by rewriting and
/or recombining auto-revert-active-p, auto-revert-buffer-p and
auto-revert-handler, as well as maybe some other functions.  I do not
know whether that would be worth the trouble given that there appears
to be plenty of other more user-visible unfinished business.

   > 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.

   Like which others?

Well, I know none at the moment, but the non-file autoreverting
applies to all non-file buffers with a revert-buffer function, so
there is plenty of "opportunity" for further verbosity problems,
given the generality.

   And why does dired display a message anyway?
   It seems just wrong to me, since the user knows he reverted the buffer if
   he did M-x revert-buffer (and indeed M-x revert-buffer in a file buffer
   does not output any message).  It seems these messages should be output
   (when needed) by the code that calls revert-buffer-function.
   As does auto-revert, for example.
   I.e. I suggest to just throw out the dired messages.

(As a side remark, reverting a dired buffer can be done with the `g'
command.)  I would personally favor getting rid of the messages, but I
do not know whether other people would agree with us on this.  It is
possible that the message gives some reassurance to some users if the
dired buffer did not visibly change.  It is also possible that on very
slow machines reverting a really huge dired buffer may take a
non-trivial amount of time.  But displaying an hourglass or so might
convey the same "Emacs is busy" message anyway.

Sincerely,

Luc.

Re: autorevert.el

[Luc Teirlinck]

While looking at autorevert.el I noticed another problem.

The docstring of `auto-revert-interval' is wrong.  It says that
setting the variable will apply to buffers put in auto-revert mode
afterwards, but not to buffers already in auto-revert mode.  That
would be bad enough if it were true, but the actual behavior is worse.
The new value does not apply to _any_ buffers until Auto Revert mode
calls `auto-revert-set-timer' for internal reasons, after which it
applies to _all_ buffers.  This is hyper-confusing and makes no sense.
If I remember well, then back in December or so, when this first came
up, you remarked that the behavior, rather than the docstring should
be changed, but apparently the opposite was done.  I did not realize
at the time how bad the actual behavior really was, nor that the new
docstring was wrong.

Below is a patch mending the situation.  Are there any objections
against me applying this patch?

While trying out my patch, I noticed another problem, however.  The
new value will take effect immediately when set through Custom, but
not when set via `set-variable' (unless the user does 
`M-x auto-revert-set-timer' afterwards).  Yet back in November we
reached pretty much a consensus on the following conclusion (quoting
Richard):

    So I guess we have to keep set-variable separate.  However,
    when it operates on a variable that is customizable, and when
    the binding it sets is a global binding, then it could work
    through the Custom mechanism.

This does not seem to have been implemented.  Is anybody currently
working or planning to work on this?

===File ~/autorevert-timer-diff=============================
diff -c /home/teirllm/autorevert-pretimer.el /home/teirllm/emacscvsdir/emacs/lisp/autorevert.el
*** /home/teirllm/autorevert-pretimer.el	Thu Mar  4 17:21:53 2004
--- /home/teirllm/emacscvsdir/emacs/lisp/autorevert.el	Thu Mar  4 20:38:13 2004
***************
*** 103,115 ****
  Never set this variable directly, use the command `auto-revert-mode' instead.")
  (put 'auto-revert-mode 'permanent-local t)
  
  (defcustom auto-revert-interval 5
    "Time, in seconds, between Auto-Revert Mode file checks.
! Setting this variable has no effect on buffers that are already in
! auto-revert-mode; it only affects buffers that are put into
! auto-revert-mode afterwards."
!   :group 'auto-revert
!   :type 'integer)
  
  (defcustom auto-revert-stop-on-user-input t
    "When non-nil Auto-Revert Mode stops checking files on user input."
--- 103,129 ----
  Never set this variable directly, use the command `auto-revert-mode' instead.")
  (put 'auto-revert-mode 'permanent-local t)
  
+ (defvar auto-revert-timer nil
+   "Timer used by Auto-Revert Mode.")
+ 
  (defcustom auto-revert-interval 5
    "Time, in seconds, between Auto-Revert Mode file checks.
! The value may be an integer or floating point number.
! 
! If a timer is already active, there are two ways to make sure
! that the new value will take effect immediately.  You can set
! this variable through Custom or you can call the command
! `auto-revert-set-timer' after setting the variable.  Otherwise,
! the new value will take effect the first time Auto Revert Mode
! calls `auto-revert-set-timer' for internal reasons or in your
! next editing session."
!   :group 'auto-revert
!   :type 'number
!   :set (lambda (variable value)
! 	 (set-default variable value)
! 	 (and (boundp 'auto-revert-timer)
! 	      auto-revert-timer
! 	      (auto-revert-set-timer))))
  
  (defcustom auto-revert-stop-on-user-input t
    "When non-nil Auto-Revert Mode stops checking files on user input."
***************
*** 205,213 ****
  The timer function `auto-revert-buffers' is responsible for purging
  the list of old buffers.")
  
- (defvar auto-revert-timer nil
-   "Timer used by Auto-Revert Mode.")
- 
  (defvar auto-revert-remaining-buffers '()
    "Buffers not checked when user input stopped execution.")
  
--- 219,224 ----
***************
*** 256,261 ****
--- 267,273 ----
  
  (defun auto-revert-set-timer ()
    "Restart or cancel the timer."
+   (interactive)
    (if (timerp auto-revert-timer)
        (cancel-timer auto-revert-timer))
    (setq auto-revert-timer

Diff finished.  Thu Mar  4 21:14:43 2004
============================================================

Re: autorevert.el

[Luc Teirlinck]

>From my previous message:

	So I guess we have to keep set-variable separate.  However,
	when it operates on a variable that is customizable, and when
	the binding it sets is a global binding, then it could work
	through the Custom mechanism.

    This does not seem to have been implemented.  Is anybody currently
    working or planning to work on this?

I could take a look at it otherwise.

Sincerely,

Luc.

Re: autorevert.el

[Luc Teirlinck]

Three weeks ago a (without further changes) for all practical purposes
unusable, patch to autorevert.el, to make dired buffers as well as
some other non-file buffers autorevert, was applied.  I proposed
changes that would make the dired part completely functional.  I have
tested those changes for three weeks now, not only in routine, but
also in various rather extreme situations and I never encountered any
problems.  I do find autoreverting dired buffers useful.

Nevertheless, there _are_ potential severe problems with autoreverting
non-file, non-vc, non-dired buffers.  *Only* the fact that many
non-file buffers mark themselves modified even though the _user_ never
modified them, saves the non-file autoreverting from complete disaster
(by preventing it from kicking in).  Currently non-file autoreverting
applies to all non-file buffers with a revert-buffer-function.  But
many revert-buffer-function's were not written to be called by
auto-revert.  Some pop up buffers.  For instance the buffer-menu uses
a revert-buffer-function that is just `list-buffers' with two dummy
arguments.  *Fortunately*, the buffer-menu marks its buffer modified
so it does _not_ get auto-reverted.  I tried to change the buffer-menu
so it no longer marked its buffer modified.  With auto-reverting of
non-file buffers enabled, Emacs became unusable because *Buffer List*
kept continuously popping up.

Stefan had one objection to my original patch.  The fact that I used a
variable, `auto-revert-flag' to prevent dired from displaying its
usual "Reading directory /home/teirllm/...done" messages which
otherwise can occasionally get so noisy that they are not only
excessively distracting but interfere with normal minibuffer usage.

Stefan proposed to instead get rid of these messages completely.

So a first question is:

What do people think of just completely getting rid of these dired
"Reading directory /home/teirllm/...done" messages?

The second question is:

How do we prevent autorevert.el from using inappropriate
revert-buffer-function's?

One, trivial to implement, possibility, would be an
`auto-revert-buffer-function' for non-file, non-vc, non-dired buffers,
which could always, of course, just be set to the regular
revert-buffer-function if appropriate.  The buffer would only get
auto-reverted by global-auto-revert-mode if
`auto-revert-buffer-function' is non-nil.  Or else we could just stick
to autoreverting dired and vc-buffers and give up on other non-file
buffers altogether.  But the current situation is just to dangerous.
There are disasters waiting to happen.

If we are actually going to seriously implement auto-reverting for
non-file, non-vc, non-dired buffers (actually it _already_ is
implemented right now in a very dangerous way) then I probably might
eventually need that `auto-revert-flag' variable Stefan objects to
anyway, in order to use the same function as both
revert-buffer-function and auto-revert-buffer-function.

Would anybody actually find auto-reverting the buffer-menu useful?  It
might be a good test case, but I do not know whether it really would
be useful enough.  Or are there any other non-file, non-vc, non-dired
buffers people would like to autorevert?

Sincerely,

Luc.

Re: autorevert.el

[Kai Grossjohann]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> What do people think of just completely getting rid of these dired
> "Reading directory /home/teirllm/...done" messages?

Good idea.

> How do we prevent autorevert.el from using inappropriate
> revert-buffer-function's?

I'm not sure.  Maybe the way is to make all revert-buffer-functions be
appropriate.

If I understand your message correctly, then the problem with the
buffer menu revert function is that it pops up the window even if it
wasn't visible before.  Then, for this case, one approach would be to
change the revert function to just update the contents of the buffer
menu buffer, instead of also popping up the window.

> Would anybody actually find auto-reverting the buffer-menu useful?

Oh, yes!  Quite often, I'm wondering about the strange list of
buffers, because I'm not aware that I need to revert the buffer.
(Though I use ibuffer these days.)

> It might be a good test case, but I do not know whether it really
> would be useful enough.  Or are there any other non-file, non-vc,
> non-dired buffers people would like to autorevert?

The *compilation* buffers currently have a function that reruns the
compilation, I think.  Maybe it would be good to just remove that, and
require people to use M-x recompile RET instead.  Hooking
recompilation into reverting seems wrong to me, anyhow.  But that's
just my taste.

Kai

Re: autorevert.el

[Stefan Monnier]

> How do we prevent autorevert.el from using inappropriate
> revert-buffer-function's?

How does the code figure out whether auto-reverting the buffer is needed?
I mean, there's no visited-time to check, right?
So maybe we should add a `auto-revert-stale-function' which dired and
buffer-menu can set.  This will clean up the current staleness checking
function since it will only have to work for file-buffers, and will
make sure that revert-buffer-function is only called by auto-revert
for modes which have auto-revert support.


        Stefan

Re: autorevert.el

[Luc Teirlinck]

Stefan Monnier wrote:

   How does the code figure out whether auto-reverting the buffer is needed?
   I mean, there's no visited-time to check, right?

For dired buffers, visited-file-modtime works, even though
verify-visited-file-modtime does not.  For non-file, non-vc, non-dired
buffers, the current code just reverts every auto-revert-interval
seconds without checking whether reverting is actually needed.  For
most of these buffers we might actually have to keep things that way,
since checking whether the buffer needs reverting could easily take
longer than reverting it.

   So maybe we should add a `auto-revert-stale-function' which dired and
   buffer-menu can set.  This will clean up the current staleness checking
   function since it will only have to work for file-buffers, and will
   make sure that revert-buffer-function is only called by auto-revert
   for modes which have auto-revert support.

I will try to make the buffer-menu auto-revert in a non-obnoxious way
and see how things work out.

Sincerely,

Luc.

Re: autorevert.el

[Stefan Monnier]

> For dired buffers, visited-file-modtime works, even though
> verify-visited-file-modtime does not.  For non-file, non-vc, non-dired
> buffers, the current code just reverts every auto-revert-interval
> seconds without checking whether reverting is actually needed.

That's just wrong in most cases.  So we indeed need an
`auto-revert-stale-function' which modes can set to (lambda () t) if they
want, but they'll have to do it explicitly to say "yes I know it's
suboptimal but for this case it's good enough".

> For most of these buffers we might actually have to keep things that way,
> since checking whether the buffer needs reverting could easily take longer
> than reverting it.

PCL-CVS offers a revert-buffer-function and it better not be refreshed
every 5 seconds unless maybe you're working on a small project hosted locally.


        Stefan

Re: autorevert.el

[Eli Zaretskii]

> Date: Sun, 14 Mar 2004 18:08:03 -0600 (CST)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
> Stefan Monnier wrote:
> 
>    How does the code figure out whether auto-reverting the buffer is needed?
>    I mean, there's no visited-time to check, right?
> 
> For dired buffers, visited-file-modtime works

Only on Posix systems, btw.  On MS-Windows, the directory's time
stamp does not change when you add or remove files.

Re: autorevert.el

[Luc Teirlinck]

Eli Zaretskii wrote:

   > For dired buffers, visited-file-modtime works

   Only on Posix systems, btw.  On MS-Windows, the directory's time
   stamp does not change when you add or remove files.

That would mean that on MS-Windows, you also would not always (or even
rarely) get the "Directory has changed on disk; type g to update Dired"
massages by which dired warns you that the information you are getting
is obsolete.  Indeed, the way Auto Revert Mode would check whether a
dired buffer needs reverting _after my patch is applied_ would be
identical to the way Dired decides whether it needs to print that message.

Anyway, I believe I have no choice but to change the current handling
of dired buffers by autorevert, because the present code is wrong and
chews up tons of CPU.  I have the impression that the current code
takes substantially more time checking whether a dired buffer needs
reverting than it would take to actually revert it.  On top of that,
the results of the check are completely incorrect.

What this could mean is that autoreverting of dired buffers may not
work on MS-Windows or maybe that, on MS-windows, one could just
automatically autorevert dired buffers every auto-revert-interval
seconds, assuming one has a reasonably fast computer (admittedly a
very suboptimal solution).

Once I have actually applied my patch, people could experiment and see
what really happens on various operating systems.

Sincerely,

Luc.

Re: autorevert.el

[Luc Teirlinck]

>From my previous message:

   Once I have actually applied my patch, people could experiment and
   see what really happens on various operating systems.

I forgot to say that I will post the revised version of my patch on
emacs-devel, to let people take a look at it, before applying it.

Sincerely,

Luc.

Re: autorevert.el

[Eli Zaretskii]

> Date: Mon, 15 Mar 2004 22:56:31 -0600 (CST)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
> What this could mean is that autoreverting of dired buffers may not
> work on MS-Windows or maybe that, on MS-windows, one could just
> automatically autorevert dired buffers every auto-revert-interval
> seconds, assuming one has a reasonably fast computer (admittedly a
> very suboptimal solution).

Both of these alternatives sound wrong to me.  I think on Windows
Emacs should listen to the special message sent by the OS whenever the
contents of some directory changes.  I believe this is how the Windows
Explorer and other programs know when to refresh their views of files
in a directory.

Re: autorevert.el

[Luc Teirlinck]

Eli Zaretskii wrote:

   Both of these alternatives sound wrong to me.  I think on Windows
   Emacs should listen to the special message sent by the OS whenever the
   contents of some directory changes.  I believe this is how the Windows
   Explorer and other programs know when to refresh their views of files
   in a directory.

I do not know anything about non-POSIX operating systems and hence
will not be able to do this myself.  I also do not have access to
computers running these operating systems with CVS Emacs installed, so
I could not even check my code, if I were able to write it.  To avoid
misunderstanding, I am _not_ refusing to do it for ideological
reasons, I just am unable to do it.  I could make things convenient
for anybody wanting to do it, in particular there would be no
necessity for that person to know anything about auto-revert.el.

I have not yet completely decided on how to do things, but for the
moment it seems that there might be a `buffer-stale-function' or such
(concrete name still undecided), as Stefan suggested.  In that case,
one would have the choice between adapting the POSIX function, or
giving buffer-stale-function an operating system specific value.

Note that I am not creating a portability problem, I am inheriting a
portability problem from dired.  Any problem that would occur for
auto-revert already occurs now in as far as the "Directory has changed
on disk; type g to update Dired" messages dired prints are concerned.
Any solution to the auto-revert portability problem would also be a
solution to that current portability problem.  If there is no current
portability problem, then there will be no auto-revert portability
problem either.  (I can not check.  In particular, it really would be
hard for me to solve a problem without even being able to check
whether the problem really occurs.)

Sincerely,

Luc.

Re: autorevert.el

[Stefan Monnier]

>    Both of these alternatives sound wrong to me.  I think on Windows
>    Emacs should listen to the special message sent by the OS whenever the
>    contents of some directory changes.  I believe this is how the Windows
>    Explorer and other programs know when to refresh their views of files
>    in a directory.

Currently there's no way to do it in elisp and the problem happens as much
for dired than for auto-revert, so it's really not specific to auto-revert.

> I do not know anything about non-POSIX operating systems and hence
> will not be able to do this myself.  I also do not have access to

Don't worry about it.  Please install your autorevert-stale-function (or
whatever the name) patch as soon as it's ready.  All it has to do is to get
rid of the current autorevert-dired horror.


        Stefan

Re: autorevert.el

[Kim F. Storm]

"Eli Zaretskii" <eliz@elta.co.il> writes:

> > Date: Mon, 15 Mar 2004 22:56:31 -0600 (CST)
> > From: Luc Teirlinck <teirllm@dms.auburn.edu>
> > 
> > What this could mean is that autoreverting of dired buffers may not
> > work on MS-Windows or maybe that, on MS-windows, one could just
> > automatically autorevert dired buffers every auto-revert-interval
> > seconds, assuming one has a reasonably fast computer (admittedly a
> > very suboptimal solution).
> 
> Both of these alternatives sound wrong to me.  I think on Windows
> Emacs should listen to the special message sent by the OS whenever the
> contents of some directory changes.  I believe this is how the Windows
> Explorer and other programs know when to refresh their views of files
> in a directory.

I had the same problem in ido-mode, which wants to cache directory
contents to speed up operations.  On Windoze, this doesn't work
because cached directories don't get refreshed automatically.

My (primitive) solution was to let the user refresh directories on
demand (with C-l).  If this could be automated, it would be fine.

However this would still be pretty useless (or make things perform
worse), if the effect of receiving a "some directory updated" message
would be to clear the whole directory cache (in case the message
doesn't indicate which directory changed -- does it?).

If somebody wants to work on adding that message functionality,
I will adapt ido to use it.

I suppose adding a w32-directory-changed-functions hook variable
would be the proper mechanism; members on the hook should be functions
with one arg -- the file name of directory which changed (if available).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: autorevert.el

[Eli Zaretskii]

> From: storm@cua.dk (Kim F. Storm)
> Date: 19 Mar 2004 11:19:22 +0100
> 
> (in case the message doesn't indicate which directory changed -- does it?).

I don't know, sorry.  Perhaps Jason could help us out here.

Re: autorevert.el

[Luc Teirlinck]

Stefan Monnier wrote:

   Please install your autorevert-stale-function (or whatever the
   name) patch as soon as it's ready.  All it has to do is to get rid
   of the current autorevert-dired horror.

Here is the proposed patch.  I actually took a look at the code
duplication in the current code and there appeared to be quite some.
My patch gets rid of one more function.

I _did_ get rid of my auto-revert-flag variable.  However, I am not
sure that as people try to make more and more modes suitable for
auto-reverting the need for that variable will not arise again.  If
people would object against the modifications I made to the
revert-buffer-functions of dired (silencing the revert message) or the
buffer-menu (see a later message), then I would need it right now.

Some things about the *-stale-function may be up for debate.  I
called it buffer-stale-function and gave it an optional argument
telling it that it is called by auto-revert or something else that
does not ask for confirmation.  The idea is that the same function
also could be useful to warn the user that the buffer is stale, if
auto-revert mode is _not_ enabled.  Since here the user makes an
explicit decision, the behavior could be different for remote files
and other time confusing stuff.  So the function would (like
revert-buffer-function) not necessarily be auto-revert specific.
Alternatively, one could make it an auto-revert specific function,
call it auto-revert-stale-function and get rid of the optional argument.

I will send supporting patches for dired.el and buff-menu.el separately.

===File ~/autorevert-diff===================================
diff -c /home/teirllm/autorevert.old.el /home/teirllm/emacscvsdir/emacs/lisp/autorevert.el
*** /home/teirllm/autorevert.old.el	Sat Mar 20 19:01:28 2004
--- /home/teirllm/emacscvsdir/emacs/lisp/autorevert.el	Sat Mar 20 18:56:29 2004
***************
*** 172,178 ****
  When non-nil, both file buffers and buffers with a custom
  `revert-buffer-function' are reverted by Global Auto-Revert Mode.
  
! Use this option with care since it could lead to excessive reverts."
    :group 'auto-revert
    :type 'boolean)
  
--- 172,183 ----
  When non-nil, both file buffers and buffers with a custom
  `revert-buffer-function' are reverted by Global Auto-Revert Mode.
  
! Use this option with care since it could lead to excessive reverts.
! Note also that for some non-file buffers the check whether the
! buffer needs updating may be imperfect, due to efficiency
! considerations, and may not take all information listed in the
! buffer into account.  Hence, a non-nil value for this option does
! not necessarily make manual updates useless for non-file buffers."
    :group 'auto-revert
    :type 'boolean)
  
***************
*** 193,198 ****
--- 198,215 ----
  This variable becomes buffer local when set in any fashion.")
  (make-variable-buffer-local 'global-auto-revert-ignore-buffer)
  
+ (defvar buffer-stale-function nil
+   "Function to check whether a non-file buffer needs reverting.
+ This should be a function with one optional argument NOCONFIRM.
+ Auto Revert Mode sets NOCONFIRM to t.  The function should return
+ non-nil if the buffer should be reverted.  The buffer is current
+ when this function is called.
+ 
+ The idea behind the NOCONFIRM argument is that the same function
+ can also be used to ask the user whether the buffer should be
+ reverted.  In such a situation one has to be less careful about,
+ say, reverting remote files, than if the function ia called at
+ regular intervals by Auto Revert Mode.")
  
  ;; Internal variables:
  
***************
*** 272,332 ****
         (not (memq major-mode
  		  global-auto-revert-ignore-modes)))))
  
- (defun auto-revert-list-diff (a b)
-   "Check if strings in list A differ from list B."
-   (when (and a b)
-     (setq a (sort a 'string-lessp))
-     (setq b (sort b 'string-lessp))
-     (let (elt1 elt2)
-       (catch 'break
- 	(while (and (setq elt1 (and a (pop a)))
- 		    (setq elt2 (and b (pop b))))
- 	  (if (not (string= elt1 elt2))
- 	      (throw 'break t)))))))
- 
- (defun auto-revert-dired-file-list ()
-   "Return list of dired files."
-   (let (file list)
-     (save-excursion
-       (goto-char (point-min))
-       (while (not (eobp))
- 	(if (setq file (dired-get-filename t t))
- 	    (push file list))
- 	(forward-line 1)))
-     list))
- 
- (defun auto-revert-dired-changed-p ()
-   "Check if dired buffer has changed."
-   (when (and (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))
-     (let ((files (directory-files dired-directory))
- 	  (dired (auto-revert-dired-file-list)))
-       (or (not (eq (length files) (length dired)))
- 	  (auto-revert-list-diff files dired)))))
- 
- (defun auto-revert-buffer-p ()
-   "Check if current buffer should be reverted."
-   ;;  - Always include dired buffers to list.  It would be too expensive
-   ;;  to test the "revert" status here each time timer launches.
-   ;;  - Same for VC buffers.
-   (or (and (eq major-mode 'dired-mode)
- 	   (or (and global-auto-revert-mode
- 		    global-auto-revert-non-file-buffers)
- 	       auto-revert-mode))
-       (and (not (buffer-modified-p))
- 	   (auto-revert-vc-buffer-p))
-       (and (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."
    (let* ((control "CVS/Entries")
--- 289,294 ----
***************
*** 383,407 ****
  
  (defun auto-revert-handler ()
    "Revert current buffer."
!   (let (revert)
!     (cond
!      ((eq major-mode 'dired-mode)
!       ;;  Dired includes revert-buffer-function
!       (when (and revert-buffer-function
! 		 (auto-revert-dired-changed-p))
  	(setq revert t)))
!      ((auto-revert-vc-buffer-p)
!       (when (auto-revert-handler-vc)
! 	(setq revert 'vc)))
!      ((or (buffer-file-name)
! 	  revert-buffer-function)
!       (setq revert t)))
!     (when revert
!       (revert-buffer 'ignore-auto 'dont-ask 'preserve-modes)
!       (if (eq revert 'vc)
! 	  (vc-mode-line buffer-file-name))
!       (if auto-revert-verbose
! 	  (message "Reverting buffer `%s'." (buffer-name))))))
  
  (defun auto-revert-buffers ()
    "Revert buffers as specified by Auto-Revert and Global Auto-Revert Mode.
--- 345,371 ----
  
  (defun auto-revert-handler ()
    "Revert current buffer."
!   (unless (buffer-modified-p)
!     (let (revert)
!       (cond
!        ((auto-revert-vc-buffer-p)
! 	(when (auto-revert-handler-vc)
! 	  (setq revert 'vc)))
!        ((or (and (buffer-file-name)
! 		 (file-readable-p (buffer-file-name))
! 		 (not (verify-visited-file-modtime (current-buffer))))
! 	    (and global-auto-revert-non-file-buffers
! 		 revert-buffer-function
! 		 (boundp 'buffer-stale-function)
! 		 (functionp buffer-stale-function)
! 		 (funcall buffer-stale-function t)))
  	(setq revert t)))
!       (when revert
! 	(revert-buffer 'ignore-auto 'dont-ask 'preserve-modes)
! 	(if (eq revert 'vc)
! 	    (vc-mode-line buffer-file-name))
! 	(if auto-revert-verbose
! 	    (message "Reverting buffer `%s'." (buffer-name)))))))
  
  (defun auto-revert-buffers ()
    "Revert buffers as specified by Auto-Revert and Global Auto-Revert Mode.
***************
*** 453,460 ****
  		       (memq buf auto-revert-buffer-list))
  		  (setq auto-revert-buffer-list
  			(delq buf auto-revert-buffer-list)))
! 	      (when (and (auto-revert-active-p)
! 			 (auto-revert-buffer-p))
  		(auto-revert-handler)
  		;; `preserve-modes' avoids changing the (minor) modes.  But we
  		;; do want to reset the mode for VC, so we do it explicitly.
--- 417,423 ----
  		       (memq buf auto-revert-buffer-list))
  		  (setq auto-revert-buffer-list
  			(delq buf auto-revert-buffer-list)))
! 	      (when (auto-revert-active-p)
  		(auto-revert-handler)
  		;; `preserve-modes' avoids changing the (minor) modes.  But we
  		;; do want to reset the mode for VC, so we do it explicitly.

Diff finished.  Sat Mar 20 19:30:15 2004
============================================================

Re: autorevert.el

[Luc Teirlinck]

Here is my proposed patch for dired.el supporting the auto-revert patch.

Note that, for people not using auto-revert the one visible change is
that `g' in dired would no longer print the "Reading directory %s..."
and "Reading directory %s...done" messages.  I asked before whether
this would be OK and the only reactions were from Stefan and Kai who
both agreed.  If one wants to keep those messages, then I would need
an auto-revert-flag variable to tell `dired-internal-noselect' that it
is called by auto-revert (to silence those messages), which Stefan
finds very unaesthetic.

Until somebody takes care of the portability problem mentioned by Eli,
I guess that setting global-auto-revert-non-file-buffers to t will have
essentially no effect on dired buffers for MS Windows.  (I do not know
whether there only is a portability problem for MS Windows or whether
similar problems exist for mac, VMS and/or other operating systems.)

`dired-directory-changed-p' is the exact code priorly used by
`dired-internal-noselect'.  I could actually do things differently
from the way they are done below, and incorporate
`dired-directory-changed-p' into `dired-buffer-stale-p', instead of
making it a separate function, and have `dired-internal-noselect' call
`dired-buffer-stale-p' instead of `dired-directory-changed-p'.  Then
one could give the variable buffer-stale-function an operating system
specific value.

I could document the portability problem if it would become clear that
it is not going to be solved any time soon.

===File ~/dired-diff========================================
*** dired.el.~1.272.~	Tue Feb  3 15:42:36 2004
--- dired.el	Fri Mar 19 14:05:59 2004
***************
*** 513,524 ****
        (setq dir-or-list dirname))
      (dired-internal-noselect dir-or-list switches)))
  
  ;; Separate function from dired-noselect for the sake of dired-vms.el.
  (defun dired-internal-noselect (dir-or-list &optional switches mode)
    ;; If there is an existing dired buffer for DIRNAME, just leave
    ;; buffer as it is (don't even call dired-revert).
    ;; This saves time especially for deep trees or with ange-ftp.
!   ;; The user can type `g'easily, and it is more consistent with find-file.
    ;; But if SWITCHES are given they are probably different from the
    ;; buffer's old value, so call dired-sort-other, which does
    ;; revert the buffer.
--- 513,546 ----
        (setq dir-or-list dirname))
      (dired-internal-noselect dir-or-list switches)))
  
+ ;; The following is an internal dired function.  It returns non-nil if
+ ;; the directory visited by the current dired buffer has changed on
+ ;; disk.  DIRNAME should be the directory name of that directory.
+ (defun dired-directory-changed-p (dirname)
+   (not (let ((attributes (file-attributes dirname))
+ 		    (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)))))))
+ 
+ (defun dired-buffer-stale-p (&optional noconfirm)
+   "Return non-nil if current dired buffer needs updating.
+ If NOCONFIRM is non-nil, then this function always returns nil
+ for a remote directory.  This feature is used by Auto Revert Mode."
+   (let ((dirname
+ 	 (if (consp dired-directory) (car dired-directory) dired-directory)))
+     (and (stringp dirname)
+ 	 (not (when noconfirm (file-remote-p dirname)))
+ 	 (file-readable-p dirname)
+ 	 (dired-directory-changed-p dirname))))
+ 
  ;; Separate function from dired-noselect for the sake of dired-vms.el.
  (defun dired-internal-noselect (dir-or-list &optional switches mode)
    ;; If there is an existing dired buffer for DIRNAME, just leave
    ;; buffer as it is (don't even call dired-revert).
    ;; This saves time especially for deep trees or with ange-ftp.
!   ;; The user can type `g' easily, and it is more consistent with find-file.
    ;; But if SWITCHES are given they are probably different from the
    ;; buffer's old value, so call dired-sort-other, which does
    ;; revert the buffer.
***************
*** 544,563 ****
  	  ;; kill-all-local-variables any longer.
  	  (setq buffer (create-file-buffer (directory-file-name dirname)))))
      (set-buffer buffer)
!     (if (not new-buffer-p)     ; existing buffer ...
! 	(cond (switches        ; ... but new switches
  	       ;; file list may have changed
  	       (setq dired-directory dir-or-list)
  	       ;; this calls dired-revert
  	       (dired-sort-other switches))
  	      ;; If directory has changed on disk, offer to revert.
! 	      ((if (let ((attributes (file-attributes dirname))
! 			 (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)))))
! 		   nil
  		 (message "%s"
  			  (substitute-command-keys
  			   "Directory has changed on disk; type \\[revert-buffer] to update Dired")))))
--- 566,579 ----
  	  ;; kill-all-local-variables any longer.
  	  (setq buffer (create-file-buffer (directory-file-name dirname)))))
      (set-buffer buffer)
!     (if (not new-buffer-p)		; existing buffer ...
! 	(cond (switches			; ... but new switches
  	       ;; file list may have changed
  	       (setq dired-directory dir-or-list)
  	       ;; this calls dired-revert
  	       (dired-sort-other switches))
  	      ;; If directory has changed on disk, offer to revert.
! 	      ((when (dired-directory-changed-p dirname)
  		 (message "%s"
  			  (substitute-command-keys
  			   "Directory has changed on disk; type \\[revert-buffer] to update Dired")))))
***************
*** 634,640 ****
        ;; based on dired-directory, e.g. with ange-ftp to a SysV host
        ;; where ls won't understand -Al switches.
        (run-hooks 'dired-before-readin-hook)
-       (message "Reading directory %s..." dirname)
        (if (consp buffer-undo-list)
  	  (setq buffer-undo-list nil))
        (let (buffer-read-only
--- 650,655 ----
***************
*** 643,649 ****
  	(widen)
  	(erase-buffer)
  	(dired-readin-insert))
-       (message "Reading directory %s...done" dirname)
        (goto-char (point-min))
        ;; Must first make alist buffer local and set it to nil because
        ;; dired-build-subdir-alist will call dired-clear-alist first
--- 658,663 ----
***************
*** 1201,1206 ****
--- 1215,1222 ----
  ;; Dired mode is suitable only for specially formatted data.
  (put 'dired-mode 'mode-class 'special)
  
+ (defvar buffer-stale-function)
+ 
  (defun dired-mode (&optional dirname switches)
    "\
  Mode for \"editing\" directory listings.
***************
*** 1279,1284 ****
--- 1295,1302 ----
  	(propertized-buffer-identification "%17b"))
    (set (make-local-variable 'revert-buffer-function)
         (function dired-revert))
+   (set (make-local-variable 'buffer-stale-function)
+        (function dired-buffer-stale-p))
    (set (make-local-variable 'page-delimiter)
         "\n\n")
    (set (make-local-variable 'dired-directory)
============================================================

Re: autorevert.el

[Luc Teirlinck]

Below is my patch to make the Buffer Menu autorevert.

Without an auto-revert-flag type variable to tell functions that they
are called by auto-revert, making the Buffer Menu auto-revertable,
without intolerable side effects, requires two changes that affect
non auto-revert users.  But I believe that these changes are positive
anyway and one can even be considered to be a bug fix.

The most important change is the following.  If you currently update a
Buffer Menu that lists only file visiting buffers, the updated version
lists the usual Buffer Menu including non-file buffers.  I believe
this is a bug regardless of any auto-revert stuff.  Updating and
toggling file-buffers-only status are unrelated decisions.  The patch
below would make the `g' command keep the file-buffers-only status
unchanged and would add a new binding `T' that explicitly toggles the
file-buffers-only status.

The second change is that `g' would keep point at the same position
(integer value: markers are useless, because `g' erases the original
contents).  This is necessary for auto-revert, because otherwise
point keeps jumping to the beginning of the buffer, which is
intolerable if the buffer-menu is selected and one is working in it.
But I believe that even without auto-revert being enabled, one often
just types `g' to make sure the Buffer Menu is up to date.  If it is
up to date, one does not want point to move.

Note that with my patch, auto-reverting will still make point move if
the Buffer Menu is shown in a non-selected window.  But in that case,
the Buffer Menu will _really_ change if one switches to it, so that
the old value of point would be meaningless anyway.

My code unconditionally auto-reverts the Buffer Menu (if there is one)
every auto-revert-interval seconds.  However, I did not notice any
measurable CPU usage with auto-revert-interval set to its default
value of 5 seconds.

===File ~/buff-menu-diff====================================
*** buff-menu.el.~1.63.~	Tue Sep  2 07:30:05 2003
--- buff-menu.el	Sat Mar 20 19:37:33 2004
***************
*** 99,104 ****
--- 99,114 ----
  (defvar Buffer-menu-mode-map nil
    "Local keymap for `Buffer-menu-mode' buffers.")
  
+ (defvar Buffer-menu-files-only nil
+   "Non-nil if the current buffer-menu lists only file buffers.
+ This variable determines whether reverting the buffer lists only
+ file buffers.  It affects both manual reverting and reverting by
+ Auto Revert Mode.")
+ 
+ (make-variable-buffer-local 'Buffer-menu-files-only)
+ 
+ (defvar buffer-stale-function)
+ 
  (if Buffer-menu-mode-map
      ()
    (setq Buffer-menu-mode-map (make-keymap))
***************
*** 131,136 ****
--- 141,147 ----
    (define-key Buffer-menu-mode-map "b" 'Buffer-menu-bury)
    (define-key Buffer-menu-mode-map "g" 'Buffer-menu-revert)
    (define-key Buffer-menu-mode-map "V" 'Buffer-menu-view)
+   (define-key Buffer-menu-mode-map "T" 'Buffer-menu-toggle-files-only)
    (define-key Buffer-menu-mode-map [mouse-2] 'Buffer-menu-mouse-select)
  )
  
***************
*** 167,179 ****
  \\[Buffer-menu-backup-unmark] -- back up a line and remove marks.
  \\[Buffer-menu-toggle-read-only] -- toggle read-only status of buffer on this line.
  \\[Buffer-menu-revert] -- update the list of buffers.
  \\[Buffer-menu-bury] -- bury the buffer listed on this line."
    (kill-all-local-variables)
    (use-local-map Buffer-menu-mode-map)
    (setq major-mode 'Buffer-menu-mode)
    (setq mode-name "Buffer Menu")
!   (make-local-variable 'revert-buffer-function)
!   (setq revert-buffer-function 'Buffer-menu-revert-function)
    (setq truncate-lines t)
    (setq buffer-read-only t)
    (run-hooks 'buffer-menu-mode-hook))
--- 178,193 ----
  \\[Buffer-menu-backup-unmark] -- back up a line and remove marks.
  \\[Buffer-menu-toggle-read-only] -- toggle read-only status of buffer on this line.
  \\[Buffer-menu-revert] -- update the list of buffers.
+ \\[Buffer-menu-toggle-files-only] -- toggle whether the menu diplays only file buffers.
  \\[Buffer-menu-bury] -- bury the buffer listed on this line."
    (kill-all-local-variables)
    (use-local-map Buffer-menu-mode-map)
    (setq major-mode 'Buffer-menu-mode)
    (setq mode-name "Buffer Menu")
!   (set (make-local-variable 'revert-buffer-function)
!        'Buffer-menu-revert-function)
!   (set (make-local-variable 'buffer-stale-function)
!        #'(lambda (&optional noconfirm) t))
    (setq truncate-lines t)
    (setq buffer-read-only t)
    (run-hooks 'buffer-menu-mode-hook))
***************
*** 184,190 ****
    (revert-buffer))
  
  (defun Buffer-menu-revert-function (ignore1 ignore2)
!   (list-buffers))
  \f
  (defun Buffer-menu-buffer (error-if-non-existent-p)
    "Return buffer described by this line of buffer menu."
--- 198,218 ----
    (revert-buffer))
  
  (defun Buffer-menu-revert-function (ignore1 ignore2)
!   ;; We can not use save-excursion here.  The buffer gets erased.
!   (let ((old-point (point)))
!     (list-buffers-noselect Buffer-menu-files-only)
!     (goto-char old-point)))
! 
! (defun Buffer-menu-toggle-files-only (arg)
!   "Toggle whether the current buffer-menu diplays only file buffers.
! With a positive ARF diplay only file buffers.  With zero or
! negative ARG, display other buffers as well."
!   (interactive "P")
!   (setq Buffer-menu-files-only
! 	(cond ((not arg) (not Buffer-menu-files-only))
! 	      ((> (prefix-numeric-value arg) 0) t)))
!   (revert-buffer))
! 
  \f
  (defun Buffer-menu-buffer (error-if-non-existent-p)
    "Return buffer described by this line of buffer menu."
***************
*** 662,667 ****
--- 690,697 ----
        ;; current buffer is not displayed for some reason.
        (and desired-point
  	   (goto-char desired-point))
+       (setq Buffer-menu-files-only files-only)
+       (set-buffer-modified-p nil)
        (current-buffer))))
  
  ;;; arch-tag: e7dfcfc9-6cb2-46e4-bf55-8ef1936d83c6
============================================================

Re: autorevert.el

[Eli Zaretskii]

> Date: Sat, 20 Mar 2004 21:26:51 -0600 (CST)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
> Until somebody takes care of the portability problem mentioned by Eli,
> I guess that setting global-auto-revert-non-file-buffers to t will have
> essentially no effect on dired buffers for MS Windows.

How about checking the directory size as well?  That could work for MS
filesystems as well, if `stat' does a good job of reporting the
directory size (in the MS-DOS port, it does).

Re: autorevert.el

[Richard Stallman]

These Dired changes are ok.  Please install them.

The reason g in Dired displays messages is that can take a long time
for large directories.  But this seems like a sufficient reason
to get rid of the messages.

Re: autorevert.el

[Luc Teirlinck]

Eli Zaretskii wrote:

   How about checking the directory size as well?  That could work for MS
   filesystems as well, if `stat' does a good job of reporting the
   directory size (in the MS-DOS port, it does).

You mean (nth 7 (file-attributes dirname))?  (Corresponds to
st_size.)  Somehow does not seem to work (on GNU/Linux).  It returns
4096 for all empty to medium size directories.  In other words, it is
useless for our purposes.

Sincerely,

Luc.

Re: autorevert.el

[Eli Zaretskii]

> Date: Sun, 21 Mar 2004 20:44:46 -0600 (CST)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
>    How about checking the directory size as well?  That could work for MS
>    filesystems as well, if `stat' does a good job of reporting the
>    directory size (in the MS-DOS port, it does).
> 
> You mean (nth 7 (file-attributes dirname))?

Yes.

> Somehow does not seem to work (on GNU/Linux).  It returns 4096 for
> all empty to medium size directories.

That's expected: a directory's file grows very slowly, since each
entry takes up a small number of bytes, so another 4K block will be
allocated only after lots of files are added.  But that doesn't mean
it's not an indication of a change.

> In other words, it is useless for our purposes.

I'm at a loss how you arrived at that conclusion, especially since I
suggested looking at the size for the benefit of systems that cannot
use the time stamp.  Isn't this better than not knowing the directory
changed at all?

Re: autorevert.el

[Stefan Monnier]

>    How about checking the directory size as well?  That could work for MS
>    filesystems as well, if `stat' does a good job of reporting the
>    directory size (in the MS-DOS port, it does).

> You mean (nth 7 (file-attributes dirname))?  (Corresponds to
> st_size.)  Somehow does not seem to work (on GNU/Linux).  It returns
> 4096 for all empty to medium size directories.  In other words, it is
> useless for our purposes.

He ment to check it additionally to the mod-time.
I.e. if either the mod-time or the size has changed, then the dir
has changed.


        Stefan

Re: autorevert.el

[Luc Teirlinck]

Eli Zaretskii wrote:

   I'm at a loss how you arrived at that conclusion, especially since I
   suggested looking at the size for the benefit of systems that cannot
   use the time stamp.  Isn't this better than not knowing the directory
   changed at all?

But the probability of finding it out this way is close to zero.

Stefan Monnier wrote:

   He ment to check it additionally to the mod-time.
   I.e. if either the mod-time or the size has changed, then the dir
   has changed.

Yes, but the size only seems to change after adding or deleting
approximately 200 files.  Many (probably most) of my directories do
not even contain that many files and never will.  They have size 4096
and always will.

What would seem to happen on MS Windows if we did that would be that,
normally, directories do not autorevert, so the user believes that
they never autorevert.  Once in a very blue moon, one huge directory
suddenly starts autoreverting all the time.  The user wonders what is
going on.  All of a sudden it stops autoreverting and never
autoreverts again.  Note that this system is very much biased toward
autoreverting large directories.  Small directories, that would be
very cheap to autorevert, never will be autoreverted.  It would seem
to be relatively better never to autorevert.  Or automatically revert
directories whose size is _below_ a certain threshold every
auto-revert-interval seconds, on systems for which we have no better
method.

Note that to check whether the size has changed, dired needs to store
the old size.  I did not check whether it currently does that.

What if I just go ahead and install my patches to autorevert and
dired?  (Additional changes can always be made later.)  Then maybe
somebody volunteers to solve the portability problem in the only
really correct way.  The problem affects dired proper and ido-mode as
much as it affects auto-revert.  Do the maintainers of the various
ports routinely read emacs-devel?  None appear to be Cc-ed.

Otherwise people using non-POSIX systems can try to set
buffer-stale-function to various values in dired-mode using
dired-mode-hook and see what works best.

Sincerely,

Luc.

Re: autorevert.el

[Luc Teirlinck]

>From my previous message:

    Do the maintainers of the various ports routinely read emacs-devel?
    None appear to be Cc-ed.

Except for the MS-DOS port of course.  I was thinking of Jason.  I do
not know who is maintaining the MacOS currently.

Sincerely,

Luc.

Re: autorevert.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: 22 Mar 2004 11:23:25 -0500
> 
> > You mean (nth 7 (file-attributes dirname))?  (Corresponds to
> > st_size.)  Somehow does not seem to work (on GNU/Linux).  It returns
> > 4096 for all empty to medium size directories.  In other words, it is
> > useless for our purposes.
> 
> He ment to check it additionally to the mod-time.

Yes; sorry if that wasn't clear.

Re: autorevert.el

[Stefan Monnier]

> Here is the proposed patch.  I actually took a look at the code
> duplication in the current code and there appeared to be quite some.
> My patch gets rid of one more function.

Thanks, it looks pretty good.
One thing tho: if you want to call the var buffer-stale-function, you have
to defvar it in files.el or somesuch.  If you defvar it in autorevert.el,
you should call it auto-revert-stale-function or something.


        Stefan

Re: autorevert.el

[Eli Zaretskii]

> Date: Mon, 22 Mar 2004 13:39:08 -0600 (CST)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
>    I'm at a loss how you arrived at that conclusion, especially since I
>    suggested looking at the size for the benefit of systems that cannot
>    use the time stamp.  Isn't this better than not knowing the directory
>    changed at all?
> 
> But the probability of finding it out this way is close to zero.

Close to zero is still infinitely better than zero.

>    He ment to check it additionally to the mod-time.
>    I.e. if either the mod-time or the size has changed, then the dir
>    has changed.
> 
> Yes, but the size only seems to change after adding or deleting
> approximately 200 files.

That's on GNU/Linux and Unix systems, but not necessarily on others.

> What would seem to happen on MS Windows if we did that would be that,
> normally, directories do not autorevert, so the user believes that
> they never autorevert.  Once in a very blue moon, one huge directory
> suddenly starts autoreverting all the time.  The user wonders what is
> going on.  All of a sudden it stops autoreverting and never
> autoreverts again.

You again assume that DOS and Windows behave like Posix systems; they
don't.  `stat' is not a system call on DOS/Windows, so whoever writes
its emulation for the library could, for example, add the lengths of
all the names of the directory files and use that as the directory's
size.  With such a version of `stat', autorevert will work like it
does on Posix platforms.

> What if I just go ahead and install my patches to autorevert and
> dired?

It's your code, so you may do as you wish.  Thanks for working on it.

Re: autorevert.el

[Stefan Monnier]

>> What if I just go ahead and install my patches to autorevert and
>> dired?

I general, I think that any code which you know will be needed in the end
and is already an improvement should be installed right away even if you
intend to improve it further real-soon-now.

Reviewing/improving/fixing/editing patches via email is not nearly as
convenient as doing it via a shared repository.


        Stefan

Re: autorevert.el

[Luc Teirlinck]

Eli Zaretskii wrote:

   You again assume that DOS and Windows behave like Posix systems; they
   don't.  `stat' is not a system call on DOS/Windows, so whoever writes
   its emulation for the library could, for example, add the lengths of
   all the names of the directory files and use that as the directory's
   size.  With such a version of `stat', autorevert will work like it
   does on Posix platforms.

Not completely.  When renaming a file to a name with the same length
(common when one made a typo, when updating a version number, changing
a suffix and so on), dired would not get notified.  So even with your
suggested version of stat, looking at the directory size would seem to
be clearly inferior to the prior suggestion of listening to the
messages delivered when a directory changes.

Anyway, the question is whether one can find anybody willing and able
to implement either of these suggestions.  I would say, preferably the
"listening to the messages" one.

Sincerely,

Luc.

Re: autorevert.el

[Luc Teirlinck]

Stefan Monnier do:

   One thing tho: if you want to call the var buffer-stale-function, you have
   to defvar it in files.el or somesuch.

I believe I move the defvar to files.el.  That makes sense, because
that is where the related variable revert-buffer-function is defvar-ed.
Since files.el is pre-loaded (and very early) that would also mean
that all those compiler-defvars in other files would no longer be
needed.  So it does look a lot better.

Sincerely,

Luc.

Re: autorevert.el

[Luc Teirlinck]

>From my prior message:

   I believe I move the defvar to files.el.

I obviously meant:  "I believe I will move the defvar to files.el".

Sincerely,

Luc.

Re: autorevert.el

[Eli Zaretskii]

> Date: Tue, 23 Mar 2004 22:10:08 -0600 (CST)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
> When renaming a file to a name with the same length (common when one
> made a typo, when updating a version number, changing a suffix and
> so on), dired would not get notified.

So who is talking about almost-zero probability cases now? ;-)

> So even with your
> suggested version of stat, looking at the directory size would seem to
> be clearly inferior to the prior suggestion of listening to the
> messages delivered when a directory changes.

FYI, the MS-DOS port actually _has_ such a version of `stat', and it
cannot listen to the Windows messages because a DOS executable is
ostracized by M$ from having this luxury.

(In general, I'm opposed to planting into Emacs user-level features
implicit assumptions about Posix-specific behavior, in this case, that
a directory is just another file.  Yes, I know: Emacs is being
developed primarily for Posix-compliant platforms, bla, bla...)

> Anyway, the question is whether one can find anybody willing and able
> to implement either of these suggestions.

I don't see how adding a call to file-attributes is a big deal that
needs a call for volunteers, but as I said: you wrote the code, so you
get to decide what's in it.

Re: autorevert.el

[Stefan Monnier]

>> So even with your suggested version of stat, looking at the directory
>> size would seem to be clearly inferior to the prior suggestion of
>> listening to the messages delivered when a directory changes.

Hmmm... we should compare it to the current code, not to some hypothetical
other implementation that nobody has shown the willingness or ability
to write.

Of course, when/if the other code is written, we can use it in place of the
"compare timestamp and size" since it will presumably be an overall
better solution.

> (In general, I'm opposed to planting into Emacs user-level features
> implicit assumptions about Posix-specific behavior, in this case, that
> a directory is just another file.  Yes, I know: Emacs is being
> developed primarily for Posix-compliant platforms, bla, bla...)

I must say I don't understand what you're talking about here.
The "listen to messages" thingy is very much non-posix.


        Stefan

Re: autorevert.el

[Luc Teirlinck]

Eli Zaretskii wrote:

   > Anyway, the question is whether one can find anybody willing and able
   > to implement either of these suggestions.

   I don't see how adding a call to file-attributes is a big deal that
   needs a call for volunteers,

In the message I was responding to, you seemed to talk about the
described version of stat in the hypothetical and the future.  So the
two suggestions I was talking about were listening to the messages and
writing that version of stat.  I did not know that there already was
such a version.  (At least on MS_DOS.)

Sincerely,

Luc.

Re: autorevert.el

[Luc Teirlinck]

Eli Zaretskii wrote:

   > When renaming a file to a name with the same length (common when one
   > made a typo, when updating a version number, changing a suffix and
   > so on), dired would not get notified.

   So who is talking about almost-zero probability cases now? ;-)

It is definitely not almost-zero probability in my usage.
But more importantly (from `(libc.info.gz)Attribute Meanings'):

    `off_t st_size'
          This specifies the size of a regular file in bytes.  For
          files that are really devices this field isn't usually
          meaningful.  For symbolic links this specifies the length of
          the file name the link refers to.

Note that apparently, stat is only supposed to return a well defined
value for st_size for regular files and for symbolic links, not for
directories.  You told me it returns a meaningful value on ms-dos.
Why would that lead us to assume that it returns a meaningful value on
all operating systems?  It definitely does not return something
terribly useful on GNU/Linux.  Maybe on other operating systems it
returns garbage that changes on every call regardless of whether there
was any change in the directory.

   (In general, I'm opposed to planting into Emacs user-level features
   implicit assumptions about Posix-specific behavior, in this case, that
   a directory is just another file.  Yes, I know: Emacs is being
   developed primarily for Posix-compliant platforms, bla, bla...)

And I'm opposed to planting into Emacs user-level features implicit
assumptions about MS-DOS-port-specific behavior, in this case, that
st_size is useful to reliably determine the directory size.  If you
want to do this for ms-dos, then that is your prerogative as a
maintainer (the patch below would do that), but I do not believe that
one can just blindly do this for any operating system without knowing
what stat is going to do on that system.

   I don't see how adding a call to file-attributes is a big deal that
   needs a call for volunteers,

It is not.  See the patch below.  I already explained in a previous
message what I really meant.

I do not have access to a machine with an MS operating system and
Emacs CVS installed.  Hence I can not test the patch below.  It
compiles without warnings.  To test the patch below, set
global-auto-revert-non-file-buffers to t.  I just noticed that there
are bad bugs in the vc-part of the original autorevert patch (not my
patch, the prior one).  These are not yet taken care of.  If you do
not want to be confronted with these bugs while testing the patch,
just do not visit any CVS buffer with uncommitted changes while testing.

===File ~/dired-diff========================================
diff -c /home/teirllm/dired.old.el /home/teirllm/emacscvsdir/emacs/lisp/dired.el
*** /home/teirllm/dired.old.el	Wed Mar 24 22:07:09 2004
--- /home/teirllm/emacscvsdir/emacs/lisp/dired.el	Wed Mar 24 23:46:37 2004
***************
*** 525,530 ****
--- 525,538 ----
        (setq dir-or-list dirname))
      (dired-internal-noselect dir-or-list switches)))
  
+ (defvar dired-directory-size nil
+   "Internal variable used by dired on certain operating systems.
+ On ms-dos, the value is the sum of all file lenghts in the directory.
+ On other operating systems, the value may have a different meaning
+ or be meaningless.")
+ 
+ (put 'dired-directory-size 'permanent-local t)
+ 
  ;; The following is an internal dired function.  It returns non-nil if
  ;; the directory visited by the current dired buffer has changed on
  ;; disk.  DIRNAME should be the directory name of that directory.
***************
*** 534,540 ****
  	 (or (eq modtime 0)
  	     (not (eq (car attributes) t))
  	     (and (= (car (nth 5 attributes)) (car modtime))
! 		  (= (nth 1 (nth 5 attributes)) (cdr modtime)))))))
  
  (defun dired-buffer-stale-p (&optional noconfirm)
    "Return non-nil if current dired buffer needs updating.
--- 542,552 ----
  	 (or (eq modtime 0)
  	     (not (eq (car attributes) t))
  	     (and (= (car (nth 5 attributes)) (car modtime))
! 		  (= (nth 1 (nth 5 attributes)) (cdr modtime))
! 		  (if (eq system-type 'ms-dos)
! 		      ;; This does not work perfectly.
! 		      (= dired-directory-size (nth 7 attributes))
! 		    t))))))
  
  (defun dired-buffer-stale-p (&optional noconfirm)
    "Return non-nil if current dired buffer needs updating.
***************
*** 677,682 ****
--- 689,696 ----
        ;; dired-build-subdir-alist will call dired-clear-alist first
        (set (make-local-variable 'dired-subdir-alist) nil)
        (dired-build-subdir-alist)
+       (set (make-local-variable 'dired-directory-size)
+ 	   (nth 7 (file-attributes dirname)))
        (let ((attributes (file-attributes dirname)))
  	(if (eq (car attributes) t)
  	    (set-visited-file-modtime (nth 5 attributes))))

Diff finished.  Wed Mar 24 23:49:01 2004
============================================================

Re: autorevert.el

[Luc Teirlinck]

The following appears to be better than my original patch (stylistic
improvement, not bug fix):

===File ~/dired-diff========================================
diff -c /home/teirllm/dired.old.el /home/teirllm/emacscvsdir/emacs/lisp/dired.el
*** /home/teirllm/dired.old.el	Wed Mar 24 22:07:09 2004
--- /home/teirllm/emacscvsdir/emacs/lisp/dired.el	Thu Mar 25 00:39:40 2004
***************
*** 525,530 ****
--- 525,538 ----
        (setq dir-or-list dirname))
      (dired-internal-noselect dir-or-list switches)))
  
+ (defvar dired-directory-size nil
+   "Internal variable used by dired on certain operating systems.
+ On ms-dos, the value is the sum of all file lenghts in the directory.
+ On other operating systems, the value may have a different meaning
+ or be meaningless.")
+ 
+ (put 'dired-directory-size 'permanent-local t)
+ 
  ;; The following is an internal dired function.  It returns non-nil if
  ;; the directory visited by the current dired buffer has changed on
  ;; disk.  DIRNAME should be the directory name of that directory.
***************
*** 534,540 ****
  	 (or (eq modtime 0)
  	     (not (eq (car attributes) t))
  	     (and (= (car (nth 5 attributes)) (car modtime))
! 		  (= (nth 1 (nth 5 attributes)) (cdr modtime)))))))
  
  (defun dired-buffer-stale-p (&optional noconfirm)
    "Return non-nil if current dired buffer needs updating.
--- 542,552 ----
  	 (or (eq modtime 0)
  	     (not (eq (car attributes) t))
  	     (and (= (car (nth 5 attributes)) (car modtime))
! 		  (= (nth 1 (nth 5 attributes)) (cdr modtime))
! 		  (if (eq system-type 'ms-dos)
! 		      ;; This does not work perfectly.
! 		      (= dired-directory-size (nth 7 attributes))
! 		    t))))))
  
  (defun dired-buffer-stale-p (&optional noconfirm)
    "Return non-nil if current dired buffer needs updating.
***************
*** 678,685 ****
        (set (make-local-variable 'dired-subdir-alist) nil)
        (dired-build-subdir-alist)
        (let ((attributes (file-attributes dirname)))
! 	(if (eq (car attributes) t)
! 	    (set-visited-file-modtime (nth 5 attributes))))
        (set-buffer-modified-p nil)
        ;; No need to narrow since the whole buffer contains just
        ;; dired-readin's output, nothing else.  The hook can
--- 690,699 ----
        (set (make-local-variable 'dired-subdir-alist) nil)
        (dired-build-subdir-alist)
        (let ((attributes (file-attributes dirname)))
! 	(when (eq (car attributes) t)
! 	  (set-visited-file-modtime (nth 5 attributes))
! 	  (set (make-local-variable 'dired-directory-size)
! 	       (nth 7 attributes))))
        (set-buffer-modified-p nil)
        ;; No need to narrow since the whole buffer contains just
        ;; dired-readin's output, nothing else.  The hook can

Diff finished.  Thu Mar 25 00:40:04 2004
============================================================

Re: autorevert.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: 24 Mar 2004 13:03:16 -0500
> 
> > (In general, I'm opposed to planting into Emacs user-level features
> > implicit assumptions about Posix-specific behavior, in this case, that
> > a directory is just another file.  Yes, I know: Emacs is being
> > developed primarily for Posix-compliant platforms, bla, bla...)
> 
> I must say I don't understand what you're talking about here.
> The "listen to messages" thingy is very much non-posix.

I was talking about the assumption that the directory's time stamp
changes whenever files are created or removed in that directory.

A more portable way of building a feature that needs to know when
directory contents change would be to define a system-specific
function/macro that would return an indication of that, and leave its
implementation to platform-specific files.

Re: autorevert.el

[Eli Zaretskii]

> Date: Thu, 25 Mar 2004 00:20:20 -0600 (CST)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
> Note that apparently, stat is only supposed to return a well defined
> value for st_size for regular files and for symbolic links, not for
> directories.

The glibc manual keeps silence about directories, but in fact, at
least to the best of my knowledge, st_size does return the size of
the directory file on GNU/Linux and other Posix-compliant systems.

> You told me it returns a meaningful value on ms-dos.

The standard C library used to build Emacs on MS-DOS simply tries to
do its best to return some meaningful value for a directory's size.

>    (In general, I'm opposed to planting into Emacs user-level features
>    implicit assumptions about Posix-specific behavior, in this case, that
>    a directory is just another file.  Yes, I know: Emacs is being
>    developed primarily for Posix-compliant platforms, bla, bla...)
> 
> And I'm opposed to planting into Emacs user-level features implicit
> assumptions about MS-DOS-port-specific behavior

So am I; see my other message in this thread from a few minutes ago.

>    I don't see how adding a call to file-attributes is a big deal that
>    needs a call for volunteers,
> 
> It is not.  See the patch below.

Thank you.

Re: autorevert.el

[Stefan Monnier]

> I was talking about the assumption that the directory's time stamp
> changes whenever files are created or removed in that directory.

Oh, I see.  I should have figured that out.

> A more portable way of building a feature that needs to know when
> directory contents change would be to define a system-specific
> function/macro that would return an indication of that, and leave its
> implementation to platform-specific files.

In principle that may be true.  But for now I think it's perfectly fine to
just hardcode the few different ways we know of in dired-stale-buffer-p.


        Stefan