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

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

10 maj 2019 kl. 11.49 skrev Michael Albinus <michael.albinus@gmx.de>:

> 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.

Thank you, it looks like `find-file-hook' will do. It's `set-visited-file-name' that lacks a hook. We could add one, `after-visited-file-name-change-hook' say, and run it at the end of that function. It would come in handy for fixing the write-file bug, too.

That would suffice for this particular need, but we may contemplate some variations for general utility, such as passing the old value of buffer-file-name to the hook. It also wouldn't catch direct modifications of buffer-file-name, but that mostly happens in  special buffers that we don't want to autorevert anyway (?).

Perhaps we should exclude all buffers whose name start with a space from any kind of auto-revert, just in case.

>> +(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.

Agreed, and I never liked that variable name much myself. What about `auto-revert--global-mode'? (More names in autorevert.el should have double dashes, but I suppose it was written before that convention came along.)

> 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.

Possibly, but that sounds slightly more error-prone.

>> +(defun auto-revert--find-file-noselect-advice (buffer)
[..]
>> +(defun auto-revert--after-change-major-mode ()
> 
> These are almost identical. Make argument buffer optional, and it is
> just one function.

Good point, but the advice functions will probably be replaced anyway per your request; let's see what it looks like when that is done.