16 apr. 2019 kl. 09.04 skrev Michael Albinus <michael.albinus@gmx.de>:

> I haven't tested thoroughly yet, but wouldn't it suffice if in
> auto-revert-notify-rm-watch there is just the test
> 
> (when (file-notify-valid-p auto-revert-notify-watch-descriptor)
> 
> instead of
> 
> (when auto-revert-notify-watch-descriptor

Thanks for reading my change. It is a fair question!

First of all, the descriptor wouldn't then be removed from `auto-revert-notify-watch-descriptor-hash-list' since that part is also guarded by the condition, but that's just a matter of rearranging code.

(Not only is `auto-revert-notify-watch-descriptor-hash-list' a mouthful, it is a bit misleading. It maps descriptors to lists of buffers. How about `auto-revert--buffers-by-watch-descriptor'?)

I'm not necessarily enamoured with `file-notify-valid-p'. It just tells whether there is, right now, a descriptor that looks like the argument, in use by someone, somewhere. These descriptors are reused, making the predicate dangerous to rely on, or requires brittle code that just knows that no reuse has occurred.

Slightly more robust would be to stop reusing descriptors: either made mutable, so that they can be invalidated, or made unique by using a counter. However, the basic design is still somewhat dubious: it tells us whether the descriptor is valid, but that just raises the question: why do we even have to ask? Correct code should understand its own invariants.

Now that you `mentioned auto-revert-notify-rm-watch', does it strike you as odd the way it does

    (maphash
     (lambda (key value)
       (when (equal key some-key)
         do-something))
     some-hashtable)

instead of using the hash table directly? Suggested patch to fix this attached.

(For that matter, the documentation doesn't say what mutation is permitted inside `maphash'. I can guess from the source.)

By the way, why don't we give each buffer in auto-revert-mode a unique descriptor, so that the table just maps descriptors to buffers, instead of to lists of buffers? It would simplify the code in many places, and it cannot be that common to have multiple buffers for the same file that it warrants the descriptor-sharing optimisation.