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