GNU bug report logs - #34847
27.0.50; auto-revert-buffers occasionally selects a killed buffer

Previous Next

Full log

View this message in rfc822 format

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 34847 <at> debbugs.gnu.org, Dale Sedivec <dale <at> codefu.org>
Subject: bug#34847: 27.0.50; auto-revert-buffers occasionally selects a killed buffer
Date: Sat, 06 Apr 2019 03:02:45 +0100

[Message part 1 (text/plain, inline)]

Michael Albinus <michael.albinus <at> gmx.de> writes:

> Something like this. But the final `auto-revert-remove-current-buffer'
> needs the buffer to be removed as the current one. So it is a bit more
> complex.
>
> I came up with the following patch:
>
> *** /tmp/ediffWnT0dx	2019-03-24 15:30:02.022068542 +0100
> --- /home/albinus/src/emacs/lisp/autorevert.el	2019-03-24 15:26:41.756960378 +0100
> ***************
> *** 343,352 ****
>
>   ;; Functions:
>
> ! (defun auto-revert-remove-current-buffer ()
>     "Remove dead buffer from `auto-revert-buffer-list'."
>     (setq auto-revert-buffer-list
> !         (delq (current-buffer) auto-revert-buffer-list)))
>
>   ;;;###autoload
>   (define-minor-mode auto-revert-mode
> --- 343,352 ----
>
>   ;; Functions:
>
> ! (defun auto-revert-remove-current-buffer (&optional buffer)
>     "Remove dead buffer from `auto-revert-buffer-list'."
>     (setq auto-revert-buffer-list
> !         (delq (or buffer (current-buffer)) auto-revert-buffer-list)))
>
>   ;;;###autoload
>   (define-minor-mode auto-revert-mode

Can you please update the docstring?  E.g.

  "Remove BUFFER from `auto-revert-buffer-list'.
BUFFER defaults to `current-buffer'."

> ***************
> *** 772,781 ****
>         (setq bufs (delq nil
>                          (mapcar
>                           (lambda (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
> --- 772,783 ----
>         (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

Indentation seems a bit off here.  Note that you can use 'when' instead
of 'and' to fit more easily within 80 columns, if you prefer.

Otherwise LGTM.  Can you please also make the following change while
you're at it?

[autorevert.diff (text/x-diff, inline)]

diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index bc7c616ecb..d736f0d31c 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -509,7 +509,7 @@ auto-revert-notify-rm-watch
 	   (ignore-errors
 	     (file-notify-rm-watch auto-revert-notify-watch-descriptor)))))
      auto-revert-notify-watch-descriptor-hash-list)
-    (remove-hook 'kill-buffer-hook #'auto-revert-notify-rm-watch))
+    (remove-hook 'kill-buffer-hook #'auto-revert-notify-rm-watch t))
   (setq auto-revert-notify-watch-descriptor nil
 	auto-revert-notify-modified-p nil))
 

[Message part 3 (text/plain, inline)]

It seems to have been left over from a past change:

* lisp/autorevert.el: Use lexical-binding.  Fix hook usage.
  2015-05-22 00:03:12 -0400
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9e41e0bc6a6fdb7a01841db62d8de42d67be534a

> Could you pls check whether the patch works for you with magit? (I
> don't use magit myself)

Works for me; I hope to see it land soon.  See also my other message for
a related but more subtle problem: https://debbugs.gnu.org/34847#16

Thanks,

-- 
Basil

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.