Suggestion for autorevert.el

Suggestion for autorevert.el

[Eric Hanchrow]

I often use `autorevert' to do what `tail' does -- namely, watch the
end of a constantly-growing file.  However, the current code doesn't
make that easy, because point never moves.  So here's an idea for
keeping point at the end of the buffer.  You'll note I have a
question: the new variable auto-revert-tail seems like it ought to be
buffer-local, but it's not clear to me how to make it so if I use
`defcustom'.  I was tempted to use `defvar' instead, since a few
other variables in autorevert.el are themselves defined with `defvar'
... but I'm not sure why.

    diff -bu /usr/local/src/emacs-cvs/lisp/autorevert.el.\~1.22.\~ /usr/local/src/emacs-cvs/lisp/autorevert.el
    --- /usr/local/src/emacs-cvs/lisp/autorevert.el.~1.22.~	2004-03-20 17:00:11.000000000 -0800
    +++ /usr/local/src/emacs-cvs/lisp/autorevert.el	2004-03-23 18:24:19.000000000 -0800
    @@ -176,6 +176,12 @@
       :group 'auto-revert
       :type 'boolean)

    +;; should this be buffer-local?
    +(defcustom auto-revert-tail nil
    +  "When non-nil, move point to the end of the buffer after reverting."
    +  :group 'auto-revert
    +  :type 'boolean)
    +
     (defcustom global-auto-revert-ignore-modes '()
       "List of major modes Global Auto-Revert Mode should not check."
       :group 'auto-revert
    @@ -401,7 +407,9 @@
           (if (eq revert 'vc)
              (vc-mode-line buffer-file-name))
           (if auto-revert-verbose
    -	  (message "Reverting buffer `%s'." (buffer-name))))))
    +	  (message "Reverting buffer `%s'." (buffer-name)))
    +      (if auto-revert-tail
    +          (goto-char (point-max))))))

     (defun auto-revert-buffers ()
       "Revert buffers as specified by Auto-Revert and Global Auto-Revert Mode.

    Diff finished.  Tue Mar 23 18:34:41 2004

-- 
Hamburgers!  The cornerstone of any nutritious breakfast.
        -- Jules {From "Pulp Fiction"}

Re: Suggestion for autorevert.el

[Luc Teirlinck]

Eric Hanchrow wrote:

   You'll note I have a question: the new variable auto-revert-tail
   seems like it ought to be buffer-local, but it's not clear to me
   how to make it so if I use `defcustom'.  I was tempted to use
   `defvar' instead, since a few other variables in autorevert.el are
   themselves defined with `defvar' ... but I'm not sure why.

Your variable auto-revert-tail, in one form or another, seems to make
sense.  (`t' would definitely be the wrong default for certain
non-file buffers, such as dired or the Buffer Menu.)  If you use
defcustom then setting the value with Custom will set the global
default value.  Since the behavior can be quite disruptive if it is
non-intentional, also for file buffers, I do not believe that using
this as a global default would be useful to many users.

I personally would use defvar and make-variable-buffer-local.  As
such, I believe that it can be useful.  But maybe there are better
solutions than that.  Variations are possible.  

Maybe if point _already_ is at the end of the buffer, then it stays
there.  That would seem to be more suitable for a global default and
hence a defcustom.  That one would not seem to get too disruptive too
often.

Sincerely,

Luc.

Re: Suggestion for autorevert.el

[Richard Stallman]

    I often use `autorevert' to do what `tail' does -- namely, watch the
    end of a constantly-growing file.  However, the current code doesn't
    make that easy, because point never moves.  So here's an idea for
    keeping point at the end of the buffer.

It would be consistent with the rest of Emacs
to keep point at the end if it was previously at the end.
Please do it that way, rather than introducing an option.

Re: Suggestion for autorevert.el

[Luc Teirlinck]

Richard Stallman wrote (concerning the issue of making auto-revert
suitable for tailing):

   It would be consistent with the rest of Emacs to keep point at the
   end if it was previously at the end.  Please do it that way, rather
   than introducing an option.

Did you mean that `revert-buffer' should be changed to keep point at
the end if it was there or just auto-revert?  If consistency is the
issue, it probably would be the former.  Eric raised two issues.  The
first, inconsistency with the documentation is easy to fix, just
change the documentation.  That would leave the question of what to do
with an empty buffer, where point is both at beginning and end.

Sincerely,

Luc.

Re: Suggestion for autorevert.el

[Juanma Barranquero]

On Wed, 31 Mar 2004 21:30:42 -0600 (CST)
Luc Teirlinck <teirllm@dms.auburn.edu> wrote:

> That would leave the question of what to do
> with an empty buffer, where point is both at beginning and end.

Well, if you write in an empty buffer, the point moves forward and stays
at the end, so it "feels" like it is initially at the end (and the fact
that it is also the beginning is purely coincidental).

My 0.02€,

                                                                Juanma

Re: Suggestion for autorevert.el

[Richard Stallman]

    Did you mean that `revert-buffer' should be changed to keep point at
    the end if it was there or just auto-revert?

Just auto-revert, was what I had in mind.
I had not thought of doing this in revert-buffer,
but it is an interesting suggestion.

      That would leave the question of what to do
    with an empty buffer, where point is both at beginning and end.

For auto-revert, in the case where you want to read the new output in
the file, this case should be treated as "at the end".  However,
in ordinary revert-buffer, that would be undesirable.

So I think this should be done in auto-revert, NOT in revert-buffer.
And it should test simply (eobp).

Re: Suggestion for autorevert.el

[Luc Teirlinck]

Eric Hanchrow wrote:

   `File: emacs, Node: Reverting' says ``revert-buffer' keeps point at
     the same distance (measured in characters) from the beginning of
     the file.'

Which is wrong (or obsolete).  To see that it is wrong, save a file
"myfile" with only content "123456789" and position point in the
buffer on the `6'.  Then do `echo abcdefghi > myfile' from a shell.
Then revert.  If the above description were accurate, point would be
on the letter `f'.  Instead, it is at the beginning of the buffer.
Maybe accurately explaining the actual positioning algorithm could be
somewhat too technical for the Emacs manual.  The present (highly)
simplified description seems very misleadingly precise.  What about
staying more "clearly vague"?  I can install the patch below if
desired.  Note that we are talking about man/files.texi, _not_
lispref/files.texi.

===File ~/man-files-diff====================================
*** files.texi	20 Jan 2004 16:17:38 -0600	1.87
--- files.texi	01 Apr 2004 17:29:17 -0600	
***************
*** 855,865 ****
  the current buffer.  Since reverting a buffer unintentionally could lose
  a lot of work, you must confirm this command with @kbd{yes}.
  
!   @code{revert-buffer} keeps point at the same distance (measured in
! characters) from the beginning of the file.  If the file was edited only
! slightly, you will be at approximately the same piece of text after
! reverting as before.  If you have made drastic changes, the same value of
! point in the old file may address a totally different piece of text.
  
    Reverting marks the buffer as ``not modified'' until another change is
  made.
--- 855,864 ----
  the current buffer.  Since reverting a buffer unintentionally could lose
  a lot of work, you must confirm this command with @kbd{yes}.
  
!   @code{revert-buffer} tries to position point in such a way that, if
! the file was edited only slightly, you will be at approximately the
! same piece of text after reverting as before.  If you have made drastic
! changes, point may wind up in a totally different piece of text.
  
    Reverting marks the buffer as ``not modified'' until another change is
  made.
============================================================

Re: Suggestion for autorevert.el

[Luc Teirlinck]

Adapting auto-revert for tailing turned out to be somewhat more
complex than may have seemed at first.  Indeed, when tailing a file in
a window, I definitely do not want to keep that window permanently
selected for tailing to work correctly.  But then just worrying about
the buffer value of point is not good enough, we have to worry about
the _window_ values of point.

The added code, which calls the non-primitive and non-optimized
`get-buffer-window-list' and does some consing, only gets called when
one needs to revert a _file buffer_.  In my usage, this happens very
infrequently.  If a huge amount of files are continuously reverting,
then maybe there could be a problem.

One could leave the patch as is, or make it dependent on a user option
`auto-revert-tail', as Eric originally suggested.  If the latter, one
could make the default value `t', since I believe that a huge amount
of continuously reverting files would be an extremely rare occurrence.
But the option would allow people to (maybe temporarily) disable the
feature if excessive CPU usage or consing became a problem.

As a side remark: is there ever any reason to use (buffer-file-name)
instead of just buffer-file-name?  I used the latter, which produces
stylistic inconsistency with prior code in autorevert.el.  If there
are no objections, I would change the prior code.  (The patch below
does not yet do that.)

===File ~/autorevert-tail-diff==============================
*** autorevert.el	31 Mar 2004 10:03:44 -0600	1.28
--- autorevert.el	01 Apr 2004 21:27:48 -0600	
***************
*** 44,49 ****
--- 44,60 ----
  ;; seconds.  The check is aborted whenever the user actually uses
  ;; Emacs.  You should never even notice that this package is active
  ;; (except that your buffers will be reverted, of course).
+ ;;
+ ;; After reverting a file buffer, Auto Revert Mode normally puts point
+ ;; at the same position that a regular manual revert would.  However,
+ ;; there is one exception to this rule.  If point is at the end of the
+ ;; buffer before reverting, it stays at the end.  Similarly if point
+ ;; is displayed at the end of a file buffer in any window, it will stay
+ ;; at the end of the buffer in that window, even if the window is not
+ ;; selected.  This way, you can use Auto Revert Mode to `tail' a file.
+ ;; Just put point at the end of the buffer and it will stay there.
+ ;; These rules apply to file buffers. For non-file buffers, the
+ ;; behavior may be mode dependent.
  
  ;; Usage:
  ;;
***************
*** 298,306 ****
    "Revert current buffer, if appropriate.
  This is an internal function used by Auto-Revert Mode."
    (unless (buffer-modified-p)
!     (let (revert)
!       (or (and (buffer-file-name)
! 	       (file-readable-p (buffer-file-name))
  	       (not (verify-visited-file-modtime (current-buffer)))
  	       (setq revert t))
  	  (and (or auto-revert-mode global-auto-revert-non-file-buffers)
--- 309,317 ----
    "Revert current buffer, if appropriate.
  This is an internal function used by Auto-Revert Mode."
    (unless (buffer-modified-p)
!     (let (revert eob winlist eoblist)
!       (or (and buffer-file-name
! 	       (file-readable-p buffer-file-name)
  	       (not (verify-visited-file-modtime (current-buffer)))
  	       (setq revert t))
  	  (and (or auto-revert-mode global-auto-revert-non-file-buffers)
***************
*** 312,318 ****
  	(when (and auto-revert-verbose
  		   (not (eq revert 'fast)))
  	  (message "Reverting buffer `%s'." (buffer-name)))
! 	(revert-buffer 'ignore-auto 'dont-ask 'preserve-modes))
        ;; `preserve-modes' avoids changing the (minor) modes.  But we
        ;; do want to reset the mode for VC, so we do it manually.
        (when (or revert auto-revert-check-vc-info)
--- 323,340 ----
  	(when (and auto-revert-verbose
  		   (not (eq revert 'fast)))
  	  (message "Reverting buffer `%s'." (buffer-name)))
! 	(when buffer-file-name
! 	  (setq eob (eobp))
! 	  (setq winlist (get-buffer-window-list (current-buffer) 'no-mini t))
! 	  (setq eoblist ())
! 	  (dolist (window winlist)
! 	    (push (cons (= (window-point window) (point-max)) window)
! 		  eoblist)))
! 	(revert-buffer 'ignore-auto 'dont-ask 'preserve-modes)
! 	(when buffer-file-name
! 	  (when eob (goto-char (point-max)))
! 	  (dolist (pair eoblist)
! 	    (when (car pair) (set-window-point (cdr pair) (point-max))))))
        ;; `preserve-modes' avoids changing the (minor) modes.  But we
        ;; do want to reset the mode for VC, so we do it manually.
        (when (or revert auto-revert-check-vc-info)
============================================================

Re: Suggestion for autorevert.el

[Luc Teirlinck]

>From my previous message:

   As a side remark: is there ever any reason to use (buffer-file-name)
   instead of just buffer-file-name?  I used the latter, which produces
   stylistic inconsistency with prior code in autorevert.el.  If there
   are no objections, I would change the prior code.  (The patch below
   does not yet do that.)

Actually, I inadvertently sent a version of my patch which _does_
change the two prior occurrences of (buffer-file-name) to buffer-file-name.

Sincerely,

Luc.

Re: Suggestion for autorevert.el

[Stefan Monnier]

> The added code, which calls the non-primitive and non-optimized
> `get-buffer-window-list' and does some consing, only gets called when
> one needs to revert a _file buffer_.  In my usage, this happens very
> infrequently.  If a huge amount of files are continuously reverting,
> then maybe there could be a problem.

I think all the existing code that has to deal with similar issues
just uses get-buffer-window and ignores the problem of the buffer being
displayed in several windows.
So I'm pleasantly impressed by your patch, and at the same time, I'm
wondering whether it's worth the trouble.


        Stefan

Re: Suggestion for autorevert.el

[Luc Teirlinck]

Stefan Monnier wrote:

   I think all the existing code that has to deal with similar issues
   just uses get-buffer-window and ignores the problem of the buffer being
   displayed in several windows.
   So I'm pleasantly impressed by your patch, and at the same time, I'm
   wondering whether it's worth the trouble.

If one is tailing a file in a specially sized small window, one may
want to actually read other portions of the file in another window
without interrupting or disturbing the tailing.  So this is definitely
a situation where visiting a buffer in several windows makes sense.  I
believe that the potential for trouble by ignoring this is much bigger
than the potential for trouble caused by an excessive amount of
constantly reverting buffers.  In the (unlikely) latter case there
probably is already trouble anyway, even without my patch, and, unless
the user really does not care about CPU usage, he probably would have
to clean up his buffer list or disable Global Auto Revert mode.  So I
now believe that there is not even a need for an option disabling the
feature, because whenever the need to do that would arise, one
probably has a problem one needs to take care of in some other way,
regardless.

Sincerely,

Luc.

Re: Suggestion for autorevert.el

[Luc Teirlinck]

>From my previous message:

   The added code, which calls the non-primitive and non-optimized
   `get-buffer-window-list'

I do not know what I was worrying about, because `get-buffer-window-list'
does by no means appear to be as slow as I assumed (I mistakenly
deduced that from comments in the source code saying that it really
should be a primitive).  Apparently one would need several thousands
of constantly reverting buffers for these calls to add up to something
and in such a situation, there would presumably be other things to
worry about.

Anyway, the following patch would appear to be better than the one I
submitted yesterday (there is no difference in behavior, it just
appears to be somewhat more efficient):

===File ~/autorevert-newdiff================================
*** autorevert.el	31 Mar 2004 10:03:44 -0600	1.28
--- autorevert.el	02 Apr 2004 17:15:25 -0600	
***************
*** 44,49 ****
--- 44,60 ----
  ;; seconds.  The check is aborted whenever the user actually uses
  ;; Emacs.  You should never even notice that this package is active
  ;; (except that your buffers will be reverted, of course).
+ ;;
+ ;; After reverting a file buffer, Auto Revert Mode normally puts point
+ ;; at the same position that a regular manual revert would.  However,
+ ;; there is one exception to this rule.  If point is at the end of the
+ ;; buffer before reverting, it stays at the end.  Similarly if point
+ ;; is displayed at the end of a file buffer in any window, it will stay
+ ;; at the end of the buffer in that window, even if the window is not
+ ;; selected.  This way, you can use Auto Revert Mode to `tail' a file.
+ ;; Just put point at the end of the buffer and it will stay there.
+ ;; These rules apply to file buffers. For non-file buffers, the
+ ;; behavior may be mode dependent.
  
  ;; Usage:
  ;;
***************
*** 298,307 ****
    "Revert current buffer, if appropriate.
  This is an internal function used by Auto-Revert Mode."
    (unless (buffer-modified-p)
!     (let (revert)
!       (or (and (buffer-file-name)
! 	       (file-readable-p (buffer-file-name))
! 	       (not (verify-visited-file-modtime (current-buffer)))
  	       (setq revert t))
  	  (and (or auto-revert-mode global-auto-revert-non-file-buffers)
  	       revert-buffer-function
--- 309,318 ----
    "Revert current buffer, if appropriate.
  This is an internal function used by Auto-Revert Mode."
    (unless (buffer-modified-p)
!     (let ((buffer (current-buffer)) revert eob eoblist)
!       (or (and buffer-file-name
! 	       (file-readable-p buffer-file-name)
! 	       (not (verify-visited-file-modtime buffer))
  	       (setq revert t))
  	  (and (or auto-revert-mode global-auto-revert-non-file-buffers)
  	       revert-buffer-function
***************
*** 312,318 ****
  	(when (and auto-revert-verbose
  		   (not (eq revert 'fast)))
  	  (message "Reverting buffer `%s'." (buffer-name)))
! 	(revert-buffer 'ignore-auto 'dont-ask 'preserve-modes))
        ;; `preserve-modes' avoids changing the (minor) modes.  But we
        ;; do want to reset the mode for VC, so we do it manually.
        (when (or revert auto-revert-check-vc-info)
--- 323,341 ----
  	(when (and auto-revert-verbose
  		   (not (eq revert 'fast)))
  	  (message "Reverting buffer `%s'." (buffer-name)))
! 	(when buffer-file-name
! 	  (setq eob (eobp))
! 	  (walk-windows
! 	   #'(lambda (window)
! 	       (and (eq (window-buffer window) buffer)
! 		    (= (window-point window) (point-max))
! 		    (push window eoblist)))
! 	   'no-mini t))
! 	(revert-buffer 'ignore-auto 'dont-ask 'preserve-modes)
! 	(when buffer-file-name
! 	  (when eob (goto-char (point-max)))
! 	  (dolist (window eoblist)
! 	    (set-window-point window (point-max)))))
        ;; `preserve-modes' avoids changing the (minor) modes.  But we
        ;; do want to reset the mode for VC, so we do it manually.
        (when (or revert auto-revert-check-vc-info)
============================================================

Re: Suggestion for autorevert.el

[Richard Stallman]

In Emacs 19 or so, I changed M-x revert so that it would try to keep
point with the text, most of the time.  I don't know any way to do
it perfectly, of course.  But it can be done right if point is in
unchanged text before all the changes, or after all the changes.

    Which is wrong (or obsolete).  To see that it is wrong, save a file
    "myfile" with only content "123456789" and position point in the
    buffer on the `6'.  Then do `echo abcdefghi > myfile' from a shell.
    Then revert.  If the above description were accurate, point would be
    on the letter `f'.  Instead, it is at the beginning of the buffer.

Perhaps that is correct behavior in that case, because there is no
unaltered text at the beginning or at the end.

However, I think I have seen incorrect handling of this over the past
couple of years, in cases where character set decoding was going on.
I never tried to debug it.


This change is good:

*** files.texi	20 Jan 2004 16:17:38 -0600	1.87
--- files.texi	01 Apr 2004 17:29:17 -0600	
***************
*** 855,865 ****
  the current buffer.  Since reverting a buffer unintentionally could lose
  a lot of work, you must confirm this command with @kbd{yes}.
  
!   @code{revert-buffer} keeps point at the same distance (measured in
! characters) from the beginning of the file.  If the file was edited only
! slightly, you will be at approximately the same piece of text after
! reverting as before.  If you have made drastic changes, the same value of
! point in the old file may address a totally different piece of text.
  
    Reverting marks the buffer as ``not modified'' until another change is
  made.
--- 855,864 ----
  the current buffer.  Since reverting a buffer unintentionally could lose
  a lot of work, you must confirm this command with @kbd{yes}.
  
!   @code{revert-buffer} tries to position point in such a way that, if
! the file was edited only slightly, you will be at approximately the
! same piece of text after reverting as before.  If you have made drastic
! changes, point may wind up in a totally different piece of text.
  
    Reverting marks the buffer as ``not modified'' until another change is
  made.
============================================================