bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Boruch Baum]

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

The attached proposed patch adds a defcustom and makes a change to two
existing functions in order to allow a user the option to only have
autorevert mode operate on visible buffers. The change to function
'after-find-files' may also fix a bug (or change behavior) in that
sometimes when being prompted to manually revert a buffer that had been
in auto-revert-mode, the auto-revert-mode status would be lost, ie. set
to nil.

Use of this feature:

1) Reduces CPU workload when using autorevert

2) Reduces messages when using `auto-revert-verbose'

Possible disadvantage:

1) When making a buffer visible, it might take as long as
   'auto-revert-interval' seconds for any accumulated changes to
   auto-revert. The default for that value is five seconds.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[-- Attachment #2: autorevert.patch --]
[-- Type: text/x-diff, Size: 1273 bytes --]

diff --git a/autorevert.el b/autorevert.el
index 7b83026..ace010e 100644
--- a/autorevert.el
+++ b/autorevert.el
@@ -302,6 +302,12 @@ You should set this variable through Custom."
   :type 'regexp
   :version "24.4")

+(defcustom auto-revert-only-if-visible nil
+  "If non-nil, suppress Auto-Revert Mode when a buffer isn't visible."
+  :group 'auto-revert
+  :type 'boolean
+  :version "28")
+
 ;; Internal variables:

 (defvar auto-revert-buffer-list ()
@@ -686,8 +692,14 @@ This is an internal function used by Auto-Revert Mode."
                               #'buffer-stale--default-function)
                           t))))
          eob eoblist)
-    (setq auto-revert-notify-modified-p nil)
-    (when revert
+    (when (and revert
+               (or (not auto-revert-only-if-visible)
+                   (member buffer
+                     (mapcar 'window-buffer
+                       (get-buffer-window-list
+                         nil nil
+                         (if (display-graphic-p) 'visible (window-normalize-frame nil)))))))
+      (setq auto-revert-notify-modified-p nil)
       (when (and auto-revert-verbose
                  (not (eq revert 'fast)))
         (message "Reverting buffer `%s'." (buffer-name)))

[-- Attachment #3: files.patch --]
[-- Type: text/x-diff, Size: 522 bytes --]

diff --git a/files.el b/files.el
index e6629d2..2cdf451 100644
--- a/files.el
+++ b/files.el
@@ -2570,7 +2570,10 @@ unless NOMODES is non-nil."
   (unless nomodes
     (when (and view-read-only view-mode)
       (view-mode -1))
-    (normal-mode t)
+    (let ((auto-revert auto-revert-mode))
+      (normal-mode t)
+      (when auto-revert
+        (auto-revert-mode 1)))
     ;; If requested, add a newline at the end of the file.
     (and (memq require-final-newline '(visit visit-save))
 	 (> (point-max) (point-min))

[-- Attachment #4: NEWS.patch --]
[-- Type: text/x-diff, Size: 543 bytes --]

diff --git a/NEWS b/NEWS
index 4076630..579f1f9 100644
--- a/NEWS
+++ b/NEWS
@@ -1157,6 +1157,12 @@ type symbols.
 messages, contain the error name of that message now.  They can be
 made visible by setting user variable 'dbus-show-dbus-errors' to
 non-nil, even if protected by 'dbus-ignore-errors' otherwise.
+** Autorevert
+
++++
+*** autorevert can be restricted to act only when a buffer is visible
+
+Enable the feature using defcustom variable 'auto-revert-only-if-visible'.

 \f
 * New Modes and Packages in Emacs 28.1

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Eli Zaretskii]

> Date: Tue, 15 Sep 2020 00:07:28 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> 
> +(defcustom auto-revert-only-if-visible nil
> +  "If non-nil, suppress Auto-Revert Mode when a buffer isn't visible."

"Visible" is ambiguous in this context.  I suggest "not displayed in
any window" instead.  Or, better, just "auto-revert only if buffer is
displayed" (which also solved a problem with double negation).

> +  :version "28")
              ^^^^
"28.1"

> +                         (if (display-graphic-p) 'visible (window-normalize-frame nil)))))))

Is the different treatment of GUI and TTY frames necessary here?

Btw, what will be the effect of this option?  Suppose some buffer was
not displayed and missed its auto-revert opportunity.  Then I switch
to it in some window -- will it appear with stale contents, or will it
auto-revert before being displayed in the window?

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Boruch Baum]

On 2020-09-15 17:30, Eli Zaretskii wrote:
> > Date: Tue, 15 Sep 2020 00:07:28 -0400
> > From: Boruch Baum <boruch_baum@gmx.com>
> >
> > +(defcustom auto-revert-only-if-visible nil
> > +  "If non-nil, suppress Auto-Revert Mode when a buffer isn't visible."
>
> "Visible" is ambiguous in this context.  I suggest "not displayed in
> any window" instead.  Or, better, just "auto-revert only if buffer is
> displayed" (which also solved a problem with double negation).

In the case of TTY emacs, only one frame seems possible to ever be
visible, but each and every frame seems to be considered, let's say
'displayed'. In that situation, this patch only auto-reverts buffers
displayed on windows of the actually 'displayed' TTY frame.

> > +  :version "28")
>               ^^^^
> "28.1"

Thanks. I had just taken a guess.

> > +                         (if (display-graphic-p) 'visible (window-normalize-frame nil)))))))
>
> Is the different treatment of GUI and TTY frames necessary here?

For users of TTY emacs (ahem, me), most assuredly. In TTY emacs, AFAICT
only one frame can ever be 'displayed' (actually visible), but function
`get-buffer-window-list' with argument ALL-FRAMES set to 'visible
returns all windows on all frames. From that function's docstring:

  --8<--cut here-(start)------------------------------------------- >8
  - ‘visible’ means consider all windows on all visible frames on
    the current terminal.
  --8<--cut here-(end)--------------------------------------------- >8

That behavior of function `get-buffer-window-list' may in turn be
because of the behavior of function `frame-visible-p', whose docstring
begins with:

  --8<--cut here-(start)------------------------------------------- >8
  Return t if FRAME is "visible" (actually in use for display).
  --8<--cut here-(end)--------------------------------------------- >8

But ends with the punch-line:

  --8<--cut here-(start)------------------------------------------- >8
  If FRAME is a text terminal frame, this always returns t.
  Such frames are always considered visible, whether or not they are
  currently being displayed on the terminal.
  --8<--cut here-(end)--------------------------------------------- >8

So since for the purposes of this patch, what matters is the actual user
experience, the distinction *was* needed.

  Off-topic: would anything constructive result if I submit a separate
  'complaint' about that behavior / judgment / decision that 'Such
  frames are always considered visible'?

> Btw, what will be the effect of this option?  Suppose some buffer was
> not displayed and missed its auto-revert opportunity.  Then I switch
> to it in some window -- will it appear with stale contents, or will it
> auto-revert before being displayed in the window?

I kind of mentioned this in my note: From my testing, at the instant an
un-reverted buffer is displayed, it is in its 'stale' state but is also
instantly considered for auto-revert. The *theoretical* maximum delay to
the auto-revert then being performed is based upon defcustom
`auto-revert-interval', whose default is five seconds. In practice, the
possible downsides are that: 1) it might be visually surprising to the
user to see the auto-revert occur; 2) if the user was very hurried to
type at wherever POINT was in the buffer, then the auto-revert would be
further delayed if defcustom `auto-revert-stop-on-user-input' was non-nil
(the default), and would only happen once the hurried typist pauses. In
my anecdotal use / testing / debugging, this has only every happened
when I consciously made the effort to race the auto-revert, but the
experience of a caffeinated or adrenaline-d user might be different.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Eli Zaretskii]

> Date: Tue, 15 Sep 2020 11:39:58 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: 43412@debbugs.gnu.org
> 
>   Off-topic: would anything constructive result if I submit a separate
>   'complaint' about that behavior / judgment / decision that 'Such
>   frames are always considered visible'?

That's why I said "visible" is ambiguous.  "Frame visibility" is a
concept in Emacs, so changing it now, and for TTY displays on top of
that, is next to unimaginable, IMO.

> > Btw, what will be the effect of this option?  Suppose some buffer was
> > not displayed and missed its auto-revert opportunity.  Then I switch
> > to it in some window -- will it appear with stale contents, or will it
> > auto-revert before being displayed in the window?
> 
> I kind of mentioned this in my note: From my testing, at the instant an
> un-reverted buffer is displayed, it is in its 'stale' state but is also
> instantly considered for auto-revert.

Maybe we should arrange it to actually auto-revert before being
displayed.  I envision bug reports if we don't.

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Boruch Baum]

On 2020-09-15 18:49, Eli Zaretskii wrote:
> > Date: Tue, 15 Sep 2020 11:39:58 -0400
> > From: Boruch Baum <boruch_baum@gmx.com>
> > Cc: 43412@debbugs.gnu.org
> >
> > > Btw, what will be the effect of this option?  Suppose some buffer was
> > > not displayed and missed its auto-revert opportunity.  Then I switch
> > > to it in some window -- will it appear with stale contents, or will it
> > > auto-revert before being displayed in the window?
> >
> > I kind of mentioned this in my note: From my testing, at the instant an
> > un-reverted buffer is displayed, it is in its 'stale' state but is also
> > instantly considered for auto-revert.
>
> Maybe we should arrange it to actually auto-revert before being
> displayed.  I envision bug reports if we don't.

Do you have a strategy or implementation in mind? Would adding a
function to `window-configuration-change-hook' do the trick (ie. catch
all relevant events)?

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Eli Zaretskii]

> Date: Tue, 15 Sep 2020 12:12:39 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: 43412@debbugs.gnu.org
> 
> > Maybe we should arrange it to actually auto-revert before being
> > displayed.  I envision bug reports if we don't.
> 
> Do you have a strategy or implementation in mind? Would adding a
> function to `window-configuration-change-hook' do the trick (ie. catch
> all relevant events)?

Yes, I think so.

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Boruch Baum]

On 2020-09-15 20:24, Eli Zaretskii wrote:
> > Date: Tue, 15 Sep 2020 12:12:39 -0400
> > From: Boruch Baum <boruch_baum@gmx.com>
> > Cc: 43412@debbugs.gnu.org
> >
> > > Maybe we should arrange it to actually auto-revert before being
> > > displayed.  I envision bug reports if we don't.
> >
> > Do you have a strategy or implementation in mind? Would adding a
> > function to `window-configuration-change-hook' do the trick (ie. catch
> > all relevant events)?
>
> Yes, I think so.

It looks to me like that's not the case: my testing seems to show that
it does catch cases of changing a buffer displayed in a window but does
not catch changes of frames due to functions like `other-frame' or
`select-frame'.

So, the good news is that I've written the code that makes the
improvement for the caught cases, and I can submit that.

As for the cases of changing frames, a less-desirable option would be to
preempt bug-reports by documenting the limitation. Auto-revert already
has other curious limitations (eg. for dired buffers it doesn't operate
_at__all_ on many types of file changes), and this limitation only
introduces a delay, so by comparison its a pretty mild limitation.

A better option would be able to catch frame-change events. I haven't
found a straightforward way to trap that. Does such a method exist?

An inelegant solution that would cover most of the remaining events
would be to advise :after ~4 frame functions, and to add an element to
variable `move-frame-functions'.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Eli Zaretskii]

> Date: Wed, 16 Sep 2020 16:11:04 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: 43412@debbugs.gnu.org
> 
> It looks to me like that's not the case: my testing seems to show that
> it does catch cases of changing a buffer displayed in a window but does
> not catch changes of frames due to functions like `other-frame' or
> `select-frame'.
> 
> So, the good news is that I've written the code that makes the
> improvement for the caught cases, and I can submit that.
> 
> As for the cases of changing frames, a less-desirable option would be to
> preempt bug-reports by documenting the limitation. Auto-revert already
> has other curious limitations (eg. for dired buffers it doesn't operate
> _at__all_ on many types of file changes), and this limitation only
> introduces a delay, so by comparison its a pretty mild limitation.

If we have no better way, we could document this as a limitation,
yes.  But let's make one more attempt to solve this.

> A better option would be able to catch frame-change events. I haven't
> found a straightforward way to trap that. Does such a method exist?

Can you describe the problematic case in more detail?  With that in
hand, perhaps Martin (CC'ed) could suggest a method.

> An inelegant solution that would cover most of the remaining events
> would be to advise :after ~4 frame functions, and to add an element to
> variable `move-frame-functions'.

Yes, I'd prefer to avoid such solutions.

Thanks.

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Boruch Baum]

On 2020-09-17 16:13, Eli Zaretskii wrote:
> > Date: Wed, 16 Sep 2020 16:11:04 -0400
> > From: Boruch Baum <boruch_baum@gmx.com>
> > Cc: 43412@debbugs.gnu.org
> >
> > It looks to me like that's not the case: my testing seems to show that
> > it does catch cases of changing a buffer displayed in a window but does
> > not catch changes of frames due to functions like `other-frame' or
> > `select-frame'.
> >
> > So, the good news is that I've written the code that makes the
> > improvement for the caught cases, and I can submit that.
> >
> > As for the cases of changing frames, a less-desirable option would be to
> > preempt bug-reports by documenting the limitation. Auto-revert already
> > has other curious limitations (eg. for dired buffers it doesn't operate
> > _at__all_ on many types of file changes), and this limitation only
> > introduces a delay, so by comparison its a pretty mild limitation.
>
> If we have no better way, we could document this as a limitation,
> yes.  But let's make one more attempt to solve this.
>
> > A better option would be able to catch frame-change events. I haven't
> > found a straightforward way to trap that. Does such a method exist?
>
> Can you describe the problematic case in more detail?  With that in
> hand, perhaps Martin (CC'ed) could suggest a method.

I'm needing now to focus on Rosh HaShanna preparations, and can let
emacs distract me for the next few days, but from memory, on tty emacs:

1) touch ~/foo

2) find it in emacs

3) bury the frame so it's not human-visible

   3.1) OT: I find it useful to globally bind C-x 5 b to function
        select-frame-by-name, because I do this routinely as part of my
        workflow. Another option might be C-x 5 2.

4) M-! echo "bar" >> ~/foo

5) switch to the buried frame

6) Depending upon your timing, you'll need to wait up to
   auto-revert-interval seconds for the buffer to be updated.

> > An inelegant solution that would cover most of the remaining events
> > would be to advise :after ~4 frame functions, and to add an element to
> > variable `move-frame-functions'.
>
> Yes, I'd prefer to avoid such solutions.

It maybe could all be solved if there were a hook for frame change of
state (eg. getting/losing focus, being raised/buried). It's surprising
to me that such events were never thought significant enough in emacs to
merit a hook.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Eli Zaretskii]

> Date: Thu, 17 Sep 2020 16:10:49 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: martin rudalics <rudalics@gmx.at>, 43412@debbugs.gnu.org
> 
> > Can you describe the problematic case in more detail?  With that in
> > hand, perhaps Martin (CC'ed) could suggest a method.
> 
> I'm needing now to focus on Rosh HaShanna preparations, and can let
> emacs distract me for the next few days, but from memory, on tty emacs:

So the problems you had were only with TTY frames?

> 1) touch ~/foo
> 
> 2) find it in emacs
> 
> 3) bury the frame so it's not human-visible
> 
>    3.1) OT: I find it useful to globally bind C-x 5 b to function
>         select-frame-by-name, because I do this routinely as part of my
>         workflow. Another option might be C-x 5 2.
> 
> 4) M-! echo "bar" >> ~/foo
> 
> 5) switch to the buried frame
> 
> 6) Depending upon your timing, you'll need to wait up to
>    auto-revert-interval seconds for the buffer to be updated.

Thanks.  Hopefully, Martin will be able to help us out here.

> It maybe could all be solved if there were a hook for frame change of
> state (eg. getting/losing focus, being raised/buried). It's surprising
> to me that such events were never thought significant enough in emacs to
> merit a hook.

We do have focus-in-hook and focus-out-hook, for starters.

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[martin rudalics]

 >> It looks to me like that's not the case: my testing seems to show that
 >> it does catch cases of changing a buffer displayed in a window but does
 >> not catch changes of frames due to functions like `other-frame' or
 >> `select-frame'.
 >>
 >> So, the good news is that I've written the code that makes the
 >> improvement for the caught cases, and I can submit that.
 >>
 >> As for the cases of changing frames, a less-desirable option would be to
 >> preempt bug-reports by documenting the limitation. Auto-revert already
 >> has other curious limitations (eg. for dired buffers it doesn't operate
 >> _at__all_ on many types of file changes), and this limitation only
 >> introduces a delay, so by comparison its a pretty mild limitation.
 >
 > If we have no better way, we could document this as a limitation,
 > yes.  But let's make one more attempt to solve this.
 >
 >> A better option would be able to catch frame-change events. I haven't
 >> found a straightforward way to trap that. Does such a method exist?
 >
 > Can you describe the problematic case in more detail?  With that in
 > hand, perhaps Martin (CC'ed) could suggest a method.

 From what I've read so far, Baruch wants to react to two events only: A
window displays another buffer and a window gets selected, where the
latter subsumes anything like frame selection or switching, or a frame
getting focus.  The former should be handled by adding a function to
'window-buffer-change-functions'.  The latter should be handled by
adding a function to 'window-selection-change-functions'.

When these added functions (probably it's one and the same function) are
run, one can use 'window-old-buffer', 'frame-old-selected-window',
'old-selected-window' and 'old-selected-frame' to individually check
what has changed since the last time.  For example, to find out whether
a window on the frame reported by 'window-buffer-change-functions' has
changed its buffer, 'walk-window-tree' for that frame with a function
that checks whether 'window-buffer' for any such window differs from
'window-old-buffer'.  If it does differ, you probably want to check
whether that buffer should be reverted.

For 'window-selection-change-functions' you probably want to just check
whether the buffer of the selected window of the reported frame should
be reverted.  I would avoid using 'window-configuration-change-hook'
because that hook also triggers when a window changed its size.  All
hooks are described in detail in section "28.28 Hooks for Window
Scrolling and Changes" of the Elisp manual.  If you have any further
questions, please ask.

martin

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Boruch Baum]

On 2020-09-18 09:49, martin rudalics wrote:
> If you have any further questions, please ask.

Thanks for the informative response. None of the symbols you mentioned
are available in the emacs version I normally use, so they were all new
to me. I do have two follow-up questions that could save me some time:

1) For function `window-buffer-change-functions' when set globally: the
docstring says that it takes an arg FRAME. During execution of those
functions, will that arg necessarily be `(selected-frame)'?

2) For function `window-buffer-change-functions' when set buffer-local:
Is there a straightforward way to ensure that the value will be set for
all buffers, current and future? This would be subtly different from the
description of what occurs when setting the value globally, because no
window-walk would be required, and it wouldn't be triggered by window
deletion events. All I see is `buffer-list-update-hook' which is run by
`get-buffer-create' (OK), but it's also run by ‘make-indirect-buffer’,
‘rename-buffer’, ‘kill-buffer’, ‘bury-buffer-internal’ and
‘select-window’ (all unnecessary for setting a buffer-local value in
`window-buffer-change-functions').

> When these added functions (probably it's one and the same function) are
> run, one can use 'window-old-buffer', 'frame-old-selected-window',
> 'old-selected-window' and 'old-selected-frame' to individually check
> what has changed since the last time.  For example, to find out whether
> a window on the frame reported by 'window-buffer-change-functions' has
> changed its buffer, 'walk-window-tree' for that frame with a function
> that checks whether 'window-buffer' for any such window differs from
> 'window-old-buffer'.  If it does differ, you probably want to check
> whether that buffer should be reverted.

> For 'window-selection-change-functions' you probably want to just check
> whether the buffer of the selected window of the reported frame should
> be reverted.  I would avoid using 'window-configuration-change-hook'
> because that hook also triggers when a window changed its size.  All
> hooks are described in detail in section "28.28 Hooks for Window
> Scrolling and Changes" of the Elisp manual.

Slightly off-topic: The 'describe-*' output for these symbols all have
an ambiguous statement "Probably introduced at or before Emacs version
27.1". What's that 'probably' all about? Shouldn't the statement be
unequivocal, at least for recently added symbols?

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[martin rudalics]

 > 1) For function `window-buffer-change-functions' when set globally: the
 > docstring says that it takes an arg FRAME. During execution of those
 > functions, will that arg necessarily be `(selected-frame)'?

No.  FRAME denotes the frame where a buffer change occurred.

 > 2) For function `window-buffer-change-functions' when set buffer-local:
 > Is there a straightforward way to ensure that the value will be set for
 > all buffers, current and future?

No.  The term 'buffer-local' contradicts the formulation "for all
buffers".  If you want to catch changes for several buffers, you either
have to specify the buffer-local value for each of them or you have to
use the default value.

 > This would be subtly different from the
 > description of what occurs when setting the value globally, because no
 > window-walk would be required, and it wouldn't be triggered by window
 > deletion events.

Window deletion events as well as events that remove a buffer from a
window are caught by the default value only.  Note in this context that
'window-old-buffer' might not even return a meaningful value if the old
buffer has been deleted in the meantime.

 > All I see is `buffer-list-update-hook' which is run by
 > `get-buffer-create' (OK), but it's also run by ‘make-indirect-buffer’,
 > ‘rename-buffer’, ‘kill-buffer’, ‘bury-buffer-internal’ and
 > ‘select-window’ (all unnecessary for setting a buffer-local value in
 > `window-buffer-change-functions').

Note that 'buffer-list-update-hook' is run in many locations that are
not related to window changes.

 > Slightly off-topic: The 'describe-*' output for these symbols all have
 > an ambiguous statement "Probably introduced at or before Emacs version
 > 27.1". What's that 'probably' all about? Shouldn't the statement be
 > unequivocal, at least for recently added symbols?

Emacs might be just cautious here.  It it finds a definition in version
27.1 and not in version 26.1, it doesn't want to deny that the variable
might have already existed in version 22.1 and was temporarily deleted
in version 25.1.

martin

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Eli Zaretskii]

> Date: Tue, 29 Sep 2020 09:05:20 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 43412@debbugs.gnu.org
> 
> Slightly off-topic: The 'describe-*' output for these symbols all have
> an ambiguous statement "Probably introduced at or before Emacs version
> 27.1". What's that 'probably' all about? Shouldn't the statement be
> unequivocal, at least for recently added symbols?

The code which determines that uses some heuristics that can fail.

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Michael Albinus]

Boruch Baum <boruch_baum@gmx.com> writes:

Hi Boruch,

> The attached proposed patch adds a defcustom and makes a change to two
> existing functions in order to allow a user the option to only have
> autorevert mode operate on visible buffers. The change to function
> 'after-find-files' may also fix a bug (or change behavior) in that
> sometimes when being prompted to manually revert a buffer that had been
> in auto-revert-mode, the auto-revert-mode status would be lost, ie. set
> to nil.

I haven't tested your patch, but I'm asking myself whether it cooperates
with autorevert triggered by file notifications. That is, if a file
changes on disk, a notification event is sent, and autovert happens. If
autorevert does not happen because the buffer is not visible, this event
will be lost, and no autorevert will happen later on when the buffer
becomes visible.

Have you tested this scenario?

Best regards, Michael.

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Boruch Baum]

On 2020-09-15 20:47, Michael Albinus wrote:
> I haven't tested your patch, but I'm asking myself whether it cooperates
> with autorevert triggered by file notifications.
> ...
> Have you tested this scenario?

YES! In fact, that was a major motivation for the submission, and
the primary method of testing it.

Please do test the patch, and report back.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Michael Albinus]

Boruch Baum <boruch_baum@gmx.com> writes:

Hi Boruch,

>> I haven't tested your patch, but I'm asking myself whether it cooperates
>> with autorevert triggered by file notifications.
>> ...
>> Have you tested this scenario?
>
> YES! In fact, that was a major motivation for the submission, and
> the primary method of testing it.
>
> Please do test the patch, and report back.

I see (still haven't tested, just reviewing). I don't understand yet,
why it is necessary to move "(setq auto-revert-notify-modified-p nil)"
into the "(when (and revert ..." form.

Furthermore, I see a problem to call auto-revert functionality in
files.el. autorevert.el is not dumped into the emacs binary. Wouldn't it
be better to add a function to find-file-hook? I understand the problem
with adding a newline at file end, but maybe this could be solved differently?

Best regards, Michael.

bug#43412: [FEATURE] autorevert-only-if-visible [PATCH]

[Boruch Baum]

On 2020-09-17 13:07, Michael Albinus wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> Hi Boruch,
>
> >> I haven't tested your patch, but I'm asking myself whether it cooperates
> >> with autorevert triggered by file notifications.
> >> ...
> >> Have you tested this scenario?
> >
> > YES! In fact, that was a major motivation for the submission, and
> > the primary method of testing it.
> >
> > Please do test the patch, and report back.
>
> I see (still haven't tested, just reviewing).

Actually testing the code would be helpful. I'm beginning to feel the
'crunch' in pre-holiday preparations for Jewish New Years, so you're on
your own until at least sometime Monday.

> I don't understand yet, why it is necessary to move "(setq
> auto-revert-notify-modified-p nil)" into the "(when (and revert ..."
> form.

The reset was being applied too broadly and preventing auto-reverts from
occurring when they needed to be. I don't have the specific key-sequence
handy, but it should be clear if you try the code with/without it in
common auto-revert-if-not-visible scenarios.

> Furthermore, I see a problem to call auto-revert functionality in
> files.el. autorevert.el is not dumped into the emacs binary.

I don't understand what you mean. What's the problem?

> Wouldn't it be better to add a function to find-file-hook?

Tried it, and it was insufficient.

> I understand the problem with adding a newline at file end

I don't. AFAIK, it has nothing to do with my patch.

> but maybe this could be solved differently?

Don't know what you mean. What are you suggesting?

Like I mentioned earlier, I need a few days for the Holiday. If you can
test the patch, that would be great.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0