Subject: Re: bug#35418: [PATCH] Don't poll auto-revert files that use notification

From: Michael Albinus <[email-masked]>
Date: Fri, 10 May 2019 11:49:30 +0200

Mattias Engdegård <[email-masked]> writes:

Hi Mattias,

> I understand that some people are queasy about using advice in code
> like this, and am open to suggestions about alternatives.

It's not just being qeasy. It is a design rule, that advising functions
do not belong to core Emacs.

> What the code needs is a reasonable (not necessarily bullet-proof) way
> to detect new file buffers and changes to buffer-file-name of those
> buffers. Monitoring `find-file-noselect' and `set-visited-file-name'
> turned out to be good enough.
> For the former, it might be possible to get away with
> `after-change-major-mode-hook' instead (already used for non-file
> buffers).

There is `find-file-hook'. If we need to hook into
`set-visited-file-name', we shall create a new hook
`after-set-visited-file-name', and run it there.

> +(defvar-local global-auto-revert--tracked-buffer nil
> +  "Non-nil if buffer is handled by Global Auto-Revert mode.")
> +

Somehow, I'm not so comfortable with that name. Could we take
`auto-revert-global-mode'? It is similar to `auto-revert-mode' and
`auto-revert-tail-mode', with the disadvantage that there does not exist
such a mode.

Alternatively, we could create a local variable `global-autorevert-mode'
in buffers which are tracked, and check always for that local value
where it matters.

> +(defun auto-revert--find-file-noselect-advice (buffer)
> +  "Adopt BUFFER for Global Auto-Revert if appropriate.
> +Called with the return value of `find-file-noselect'."
> +  (auto-revert--global-add-buffer buffer)
> +  (auto-revert-set-timer)
> +  buffer)
> +
> +(defun auto-revert--after-change-major-mode ()
> +  "Adopt the current buffer for Global Auto-Revert if appropriate.
> +Called after the current buffer got a new major mode."
> +  (auto-revert--global-add-buffer (current-buffer))
> +  (auto-revert-set-timer))

These are almost identical. Make argument buffer optional, and it is
just one function.

Best regards, Michael.