Subject: bug#43714: 28.1: auto-revert code improvements [PATCH]

From: Boruch Baum <[email-masked]>
Date: Wed, 30 Sep 2020 14:33:44 -0400

On 2020-09-30 13:41, Glenn Morris wrote:
> The mailing list archives around the time of a commit are often informative.
> https://lists.gnu.org/r/emacs-devel/2005-05/msg01414.html

Absolutely great, Glenn!

An obvious follow-up observation: The patch that was applied was for
Luc's option #3, but it seems to me that the honest solution to the
problem with timers is his option #2, to put the fix in timers. Was that
ever done?

I'm guessing a decision at some point was made not to pursue Luc's
option #2 because the emacs documentation for timers[1] says, "If a
timer function calls functions that can change the match data, it should
save and restore the match data. See Saving Match Data". Reconsidering
that decision could be of benefit emacs-wide.

Attached is a new version of the patch, with the 'save-match-data'
restored. Thanks again, Glenn.

[1] https://www.gnu.org/software/emacs/manual/html_node/elisp/Timers.html

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0
diff --git a/autorevert.el b/autorevert.el
index 011febf..4ca8010 100644
--- a/autorevert.el
+++ b/autorevert.el
@@ -869,6 +869,62 @@ This is an internal function used by Auto-Revert Mode."
       (restore-buffer-modified-p modified)))
   (set-visited-file-modtime))

+(defun auto-revert--buffer-candidates ()
+  "Return a prioritized list of buffers to maybe auto-revert.
+The differences between this return value and the reference
+variable `auto-revert-buffer-list'. include: 1) this has more
+entries when in global-auto-revert-mode; 2) this prioritizes
+buffers not reverted last time due to user interruption. "
+  (let (remaining new
+        (bufs (delq nil
+               ;; Buffers with remote contents shall be reverted only
+               ;; if the connection is established already.
+                    (mapcar
+                     (lambda (buf)
+                       (and (buffer-live-p buf)
+                            (with-current-buffer buf
+                              (and
+                               (or (not (file-remote-p default-directory))
+                                   (file-remote-p default-directory nil t))
+                                   buf))))
+                     (auto-revert--polled-buffers)))))
+    ;; Partition `bufs' into two halves depending on whether or not
+    ;; the buffers are in `auto-revert-remaining-buffers'.  The two
+    ;; halves are then re-joined with the "remaining" buffers at the
+    ;; head of the list.
+    (dolist (buf auto-revert-remaining-buffers)
+      (if (memq buf bufs)
+          (push buf remaining)))
+    (dolist (buf bufs)
+      (if (not (memq buf remaining))
+          (push buf new)))
+    (nreverse (nconc new remaining))))
+
+(defun auto-revert-buffer (buf)
+  "Revert a single buffer.
+
+This is performed as specified by Auto-Revert and Global
+Auto-Revert Modes."
+  (if (not (buffer-live-p buf))
+    (auto-revert-remove-current-buffer buf)
+   (with-current-buffer buf
+     ;; Test if someone has turned off Auto-Revert Mode
+     ;; in a non-standard way, for example by changing
+     ;; major mode.
+     (and (not auto-revert-mode)
+          (not auto-revert-tail-mode)
+          (auto-revert-remove-current-buffer))
+     (when (auto-revert-active-p)
+       ;; Enable file notification.
+       ;; Don't bother creating a notifier for non-file buffers
+       ;; unless it explicitly indicates that this works.
+       (when (and auto-revert-use-notify
+                  (not auto-revert-notify-watch-descriptor)
+                  (or buffer-file-name
+                      buffer-auto-revert-by-notification))
+         (auto-revert-notify-add-watch))
+       (auto-revert-handler)))))
+
 (defun auto-revert-buffers ()
   "Revert buffers as specified by Auto-Revert and Global Auto-Revert Mode.

@@ -892,67 +948,19 @@ are checked first the next time this function is called.
 This function is also responsible for removing buffers no longer in
 Auto-Revert Mode from `auto-revert-buffer-list', and for canceling
 the timer when no buffers need to be checked."
-
   (save-match-data
-    (let ((bufs (auto-revert--polled-buffers))
-	  remaining new)
-      ;; Buffers with remote contents shall be reverted only if the
-      ;; connection is established already.
-      (setq bufs (delq nil
-                       (mapcar
-                        (lambda (buf)
-                          (and (buffer-live-p buf)
-                               (with-current-buffer buf
-                                 (and
-                                  (or (not (file-remote-p default-directory))
-                                      (file-remote-p default-directory nil t))
-                                      buf))))
-                        bufs)))
-      ;; Partition `bufs' into two halves depending on whether or not
-      ;; the buffers are in `auto-revert-remaining-buffers'.  The two
-      ;; halves are then re-joined with the "remaining" buffers at the
-      ;; head of the list.
-      (dolist (buf auto-revert-remaining-buffers)
-	(if (memq buf bufs)
-	    (push buf remaining)))
-      (dolist (buf bufs)
-	(if (not (memq buf remaining))
-	    (push buf new)))
-      (setq bufs (nreverse (nconc new remaining)))
+    (let ((bufs (auto-revert--buffer-candidates)))
       (while (and bufs
 		  (not (and auto-revert-stop-on-user-input
 			    (input-pending-p))))
-	(let ((buf (car bufs)))
-          (if (not (buffer-live-p buf))
-              ;; Remove dead buffer from `auto-revert-buffer-list'.
-              (auto-revert-remove-current-buffer buf)
-            (with-current-buffer buf
-              ;; Test if someone has turned off Auto-Revert Mode
-              ;; in a non-standard way, for example by changing
-              ;; major mode.
-              (if (and (not auto-revert-mode)
-                       (not auto-revert-tail-mode)
-                       (memq buf auto-revert-buffer-list))
-                  (auto-revert-remove-current-buffer))
-              (when (auto-revert-active-p)
-                ;; Enable file notification.
-                ;; Don't bother creating a notifier for non-file buffers
-                ;; unless it explicitly indicates that this works.
-                (when (and auto-revert-use-notify
-                           (not auto-revert-notify-watch-descriptor)
-                           (or buffer-file-name
-                               buffer-auto-revert-by-notification))
-                  (auto-revert-notify-add-watch))
-                (auto-revert-handler)))))
-	(setq bufs (cdr bufs)))
+        (auto-revert-buffer (pop bufs)))
       (setq auto-revert-remaining-buffers bufs)
       ;; Check if we should cancel the timer.
       (unless (auto-revert--need-polling-p)
-        (if (timerp auto-revert-timer)
-            (cancel-timer auto-revert-timer))
+        (when (timerp auto-revert-timer)
+          (cancel-timer auto-revert-timer))
 	(setq auto-revert-timer nil)))))

-
 ;; The end:
 (provide 'autorevert)