bug#36159: [PATCH] auto-revert mode doesn't work when changing buffer file name

["Mattias Engdegård" <mattiase@acm.org>]

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

11 juni 2019 kl. 15.15 skrev Michael Albinus <michael.albinus@gmx.de>:
> 
>> +(defun auto-revert--set-visited-file-name ()
> 
> Since we add it to a hook from another package, it shouldn't be marked
> internal. Call it `auto-revert-set-visited-file-name'.

Done. I actually wanted to add a buffer-local hook, but it turns out that `after-set-visited-file-name-hook', as written, wouldn't work for local hooks -- the very action of `set-visited-file-name' wipes out local hooks, no doubt for good reasons, before they can trigger.

> In the other tests, we separate the unwindforms from the bodyform (an
> empty line, plus a comment). Maybe you could do it here as well.

Done.
I also removed the binding of `auto-revert-avoid-polling' in the new test, since it isn't strictly necessary to exhibit the bug.

>   FAILED  auto-revert-test05-global-notify-remote
>   FAILED  auto-revert-test06-write-file-remote
> 
> Do you want to check, or shall I do it?

Oh, would you do that? I feel bad about causing test failures like this!
The first error may be from a recently committed (f2e4c34de6) fix of an earlier copy-paste mistake; sorry about that. I'm unsure how to deal with these errors myself; I have no inotifywait equivalent on this machine (macOS).

Thanks a lot for your help!


[-- Attachment #2: 0001-Keep-auto-revert-mode-working-when-changing-buffer-f.patch --]
[-- Type: application/octet-stream, Size: 5067 bytes --]

From 4baa9d2f7e704cc722e962a4ab5245157f0ff547 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Mon, 10 Jun 2019 17:52:50 +0200
Subject: [PATCH] Keep auto-revert-mode working when changing buffer file name

* lisp/autorevert.el (after-set-visited-file-name-hook):
Add unconditionally.
(global-auto-revert-mode): Don't use
`after-set-visited-file-name-hook' here.
(auto-revert-set-visited-file-name): Rename from
`auto-revert--global-set-visited-file-name' and generalise.
* test/lisp/autorevert-tests.el (auto-revert-test06-write-file): New.
---
 lisp/autorevert.el            | 19 ++++++++++---------
 test/lisp/autorevert-tests.el | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index 2de855b303..5c79a7e795 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -347,6 +347,8 @@ auto-revert-find-file-function
 
 (add-hook 'find-file-hook
 	  #'auto-revert-find-file-function)
+(add-hook 'after-set-visited-file-name-hook
+          #'auto-revert-set-visited-file-name)
 
 (defvar auto-revert--buffers-by-watch-descriptor
   (make-hash-table :test 'equal)
@@ -508,8 +510,6 @@ global-auto-revert-mode
             (auto-revert--global-add-current-buffer)))
         ;; Make sure future buffers are added as well.
         (add-hook 'find-file-hook #'auto-revert--global-adopt-current-buffer)
-        (add-hook 'after-set-visited-file-name-hook
-                  #'auto-revert--global-set-visited-file-name)
         ;; To track non-file buffers, we need to listen in to buffer
         ;; creation in general.  Listening to major-mode changes is
         ;; suitable, since we then know whether it's a mode that is tracked.
@@ -520,8 +520,6 @@ global-auto-revert-mode
     ;; Turn global-auto-revert-mode OFF.
     (remove-hook 'after-change-major-mode-hook
                  #'auto-revert--global-adopt-current-buffer)
-    (remove-hook 'after-set-visited-file-name-hook
-                 #'auto-revert--global-set-visited-file-name)
     (remove-hook 'find-file-hook #'auto-revert--global-adopt-current-buffer)
     (dolist (buf (buffer-list))
       (with-current-buffer buf
@@ -551,14 +549,17 @@ auto-revert--global-adopt-current-buffer
   (auto-revert--global-add-current-buffer)
   (auto-revert-set-timer))
 
-(defun auto-revert--global-set-visited-file-name ()
-  "Update Global Auto-Revert management of the current buffer.
+(defun auto-revert-set-visited-file-name ()
+  "Update Auto-Revert management of the current buffer.
 Called after `set-visited-file-name'."
-  ;; Remove any existing notifier first so that we don't track the
-  ;; wrong file in case the file name was changed.
   (when auto-revert-notify-watch-descriptor
+    ;; Remove any existing notifier so that we don't track the wrong
+    ;; file in case the file name was changed.
     (auto-revert-notify-rm-watch))
-  (auto-revert--global-adopt-current-buffer))
+    (cond (global-auto-revert-mode
+           (auto-revert--global-adopt-current-buffer))
+          ((or auto-revert-mode auto-revert-tail-mode)
+           (auto-revert-set-timer))))
 
 (defun auto-revert--polled-buffers ()
   "List of buffers that need to be polled."
diff --git a/test/lisp/autorevert-tests.el b/test/lisp/autorevert-tests.el
index 3c9e04bf19..f21fb864f2 100644
--- a/test/lisp/autorevert-tests.el
+++ b/test/lisp/autorevert-tests.el
@@ -542,6 +542,40 @@ auto-revert-test--wait-for-buffer-text
 (auto-revert--deftest-remote auto-revert-test05-global-notify
   "Test `global-auto-revert-mode' without polling for remote buffers.")
 
+(ert-deftest auto-revert-test06-write-file ()
+  "Verify that notification follows `write-file' correctly."
+  :tags '(:expensive-test)
+  (skip-unless (or file-notify--library
+                   (file-remote-p temporary-file-directory)))
+  (let* ((auto-revert-use-notify t)
+         (file-1 (make-temp-file "auto-revert-test"))
+         (file-2 (concat file-1 "-2"))
+         (buf nil))
+    (unwind-protect
+        (progn
+          (setq buf (find-file-noselect file-1))
+          (with-current-buffer buf
+            (insert "A")
+            (save-buffer)
+
+            (auto-revert-mode 1)
+
+            (insert "B")
+            (write-file file-2)
+
+            (auto-revert-test--write-file "C" file-2)
+            (auto-revert-test--wait-for-buffer-text
+             buf "C" (+ auto-revert-interval 1))
+            (should (equal (buffer-string) "C"))))
+
+      ;; Clean up.
+      (ignore-errors (kill-buffer buf))
+      (ignore-errors (delete-file file-1))
+      (ignore-errors (delete-file file-2)))))
+
+(auto-revert--deftest-remote auto-revert-test06-write-file
+  "Test `write-file' in `auto-revert-mode' for remote buffers.")
+
 (defun auto-revert-test-all (&optional interactive)
   "Run all tests for \\[auto-revert]."
   (interactive "p")
-- 
2.20.1 (Apple Git-117)