bug#44639: [PATCH 2/2] autorevert: map each watch descriptor to a single buffer

[Michael Albinus <michael.albinus@gmx.de>]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi,

> Patch 1/2 was applied to Emacs 28 some time ago (and works fine), so
> this was just a followup on 2/2 -- whether this clean-up patch (which
> apparently doesn't work correctly, according to Michael) is going to get
> any further work done, or whether this issue should just be closed...

I've reworked the patch, using an association list instead of a hash
table. By this, I've fixed also the error which was always evident, but
uncovered only by Spencer's patch.

Pushed to master, I'm closing the bug. The patch itself is appended
below.

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 8816 bytes --]

*** /tmp/ediffGTBgTB	2021-01-28 15:18:21.881661122 +0100
--- /home/albinus/src/emacs/lisp/autorevert.el	2021-01-28 15:06:38.034456811 +0100
***************
*** 355,364 ****
  (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)
!   "A hash table mapping notification descriptors to lists of buffers.
! The buffers use that descriptor for auto-revert notifications.
  The key is equal to `auto-revert-notify-watch-descriptor' in each
  buffer.")

--- 355,363 ----
  (add-hook 'after-set-visited-file-name-hook
            #'auto-revert-set-visited-file-name)

! (defvar auto-revert--buffer-by-watch-descriptor nil
!   "An association list mapping notification descriptors to buffers.
! The buffer uses that descriptor for auto-revert notifications.
  The key is equal to `auto-revert-notify-watch-descriptor' in each
  buffer.")

***************
*** 630,645 ****

  (defun auto-revert-notify-rm-watch ()
    "Disable file notification for current buffer's associated file."
!   (let ((desc auto-revert-notify-watch-descriptor)
!         (table auto-revert--buffers-by-watch-descriptor))
!     (when desc
!       (let ((buffers (delq (current-buffer) (gethash desc table))))
!         (if buffers
!             (puthash desc buffers table)
!           (remhash desc table)))
!       (ignore-errors
! 	(file-notify-rm-watch desc))
!       (remove-hook 'kill-buffer-hook #'auto-revert-notify-rm-watch t)))
    (setq auto-revert-notify-watch-descriptor nil
  	auto-revert-notify-modified-p nil))

--- 629,640 ----

  (defun auto-revert-notify-rm-watch ()
    "Disable file notification for current buffer's associated file."
!   (when-let ((desc auto-revert-notify-watch-descriptor))
!     (setq auto-revert--buffer-by-watch-descriptor
!           (assoc-delete-all desc auto-revert--buffer-by-watch-descriptor))
!     (ignore-errors
!       (file-notify-rm-watch desc))
!     (remove-hook 'kill-buffer-hook #'auto-revert-notify-rm-watch t))
    (setq auto-revert-notify-watch-descriptor nil
  	auto-revert-notify-modified-p nil))

***************
*** 660,672 ****
                 (if buffer-file-name '(change attribute-change) '(change))
                 'auto-revert-notify-handler))))
        (when auto-revert-notify-watch-descriptor
!         (setq auto-revert-notify-modified-p t)
!         (puthash
!          auto-revert-notify-watch-descriptor
!          (cons (current-buffer)
! 	       (gethash auto-revert-notify-watch-descriptor
! 		        auto-revert--buffers-by-watch-descriptor))
!          auto-revert--buffers-by-watch-descriptor)
          (add-hook 'kill-buffer-hook #'auto-revert-notify-rm-watch nil t))))

  ;; If we have file notifications, we want to update the auto-revert buffers
--- 655,664 ----
                 (if buffer-file-name '(change attribute-change) '(change))
                 'auto-revert-notify-handler))))
        (when auto-revert-notify-watch-descriptor
!         (setq auto-revert-notify-modified-p t
!               auto-revert--buffer-by-watch-descriptor
!               (cons (cons auto-revert-notify-watch-descriptor (current-buffer))
!                     auto-revert--buffer-by-watch-descriptor))
          (add-hook 'kill-buffer-hook #'auto-revert-notify-rm-watch nil t))))

  ;; If we have file notifications, we want to update the auto-revert buffers
***************
*** 696,703 ****
  	   (action (nth 1 event))
  	   (file (nth 2 event))
  	   (file1 (nth 3 event)) ;; Target of `renamed'.
! 	   (buffers (gethash descriptor
! 			     auto-revert--buffers-by-watch-descriptor)))
        ;; Check, that event is meant for us.
        (cl-assert descriptor)
        ;; Since we watch a directory, a file name must be returned.
--- 688,695 ----
  	   (action (nth 1 event))
  	   (file (nth 2 event))
  	   (file1 (nth 3 event)) ;; Target of `renamed'.
! 	   (buffer (alist-get descriptor auto-revert--buffer-by-watch-descriptor
!                               nil nil #'equal)))
        ;; Check, that event is meant for us.
        (cl-assert descriptor)
        ;; Since we watch a directory, a file name must be returned.
***************
*** 706,714 ****
        (when auto-revert-debug
          (message "auto-revert-notify-handler %S" event))

!       (if (eq action 'stopped)
!           ;; File notification has stopped.  Continue with polling.
!           (cl-dolist (buffer buffers)
              (with-current-buffer buffer
                (when (or
                       ;; A buffer associated with a file.
--- 698,706 ----
        (when auto-revert-debug
          (message "auto-revert-notify-handler %S" event))

!       (when (buffer-live-p buffer)
!         (if (eq action 'stopped)
!             ;; File notification has stopped.  Continue with polling.
              (with-current-buffer buffer
                (when (or
                       ;; A buffer associated with a file.
***************
*** 721,758 ****
                  (auto-revert-notify-rm-watch)
                  ;; Restart the timer if it wasn't running.
                  (unless auto-revert-timer
!                   (auto-revert-set-timer)))))

!         ;; Loop over all buffers, in order to find the intended one.
!         (cl-dolist (buffer buffers)
!           (when (buffer-live-p buffer)
!             (with-current-buffer buffer
!               (when (or
!                      ;; A buffer associated with a file.
!                      (and (stringp buffer-file-name)
!                           (or
!                            (and (memq
!                                  action '(attribute-changed changed created))
!                                 (string-equal
!                                  (file-name-nondirectory file)
!                                  (file-name-nondirectory buffer-file-name)))
!                            (and (eq action 'renamed)
!                                 (string-equal
!                                  (file-name-nondirectory file1)
!                                  (file-name-nondirectory buffer-file-name)))))
!                      ;; A buffer w/o a file, like dired.
!                      (and (null buffer-file-name)
!                           (memq action '(created renamed deleted))))
!                 ;; Mark buffer modified.
!                 (setq auto-revert-notify-modified-p t)
!
!                 ;; Revert the buffer now if we're not locked out.
!                 (unless auto-revert--lockout-timer
!                   (auto-revert-handler)
!                   (setq auto-revert--lockout-timer
!                         (run-with-timer
!                          auto-revert--lockout-interval nil
!                          #'auto-revert--end-lockout buffer)))))))))))

  (defun auto-revert--end-lockout (buffer)
    "End the lockout period after a notification.
--- 713,747 ----
                  (auto-revert-notify-rm-watch)
                  ;; Restart the timer if it wasn't running.
                  (unless auto-revert-timer
!                   (auto-revert-set-timer))))

!           (with-current-buffer buffer
!             (when (or
!                    ;; A buffer associated with a file.
!                    (and (stringp buffer-file-name)
!                         (or
!                          (and (memq
!                                action '(attribute-changed changed created))
!                               (string-equal
!                                (file-name-nondirectory file)
!                                (file-name-nondirectory buffer-file-name)))
!                          (and (eq action 'renamed)
!                               (string-equal
!                                (file-name-nondirectory file1)
!                                (file-name-nondirectory buffer-file-name)))))
!                    ;; A buffer w/o a file, like dired.
!                    (and (null buffer-file-name)
!                         (memq action '(created renamed deleted))))
!               ;; Mark buffer modified.
!               (setq auto-revert-notify-modified-p t)
!
!               ;; Revert the buffer now if we're not locked out.
!               (unless auto-revert--lockout-timer
!                 (auto-revert-handler)
!                 (setq auto-revert--lockout-timer
!                       (run-with-timer
!                        auto-revert--lockout-interval nil
!                        #'auto-revert--end-lockout buffer))))))))))

  (defun auto-revert--end-lockout (buffer)
    "End the lockout period after a notification.