19 apr. 2019 kl. 16.56 skrev Michael Albinus : > >> (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'?) > > No objection. Thanks, I'll do this separately. >> 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. > > In theory you are right. But I fear there could be situations where such > assumptions do ne keep. A double-check is OK. What about using the checks in assertions to validate the assumptions, instead of making the code bug-tolerant? Assertions both inform the reader and check correctness. After all, bug-tolerant code just leads to more bugs, to code where the just-in-case conditions cannot be distinguished from the essential ones, and where nobody understands the 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. > > I'm still not convinced that we need REMOVE-DESCRIPTOR. We shall always > remove the descriptor, and assure, that no superfluous events are raised. Can I take it that you are happy with the patch attached to that message, which just does away with the maphash? Regarding getting rid of remove-descriptor, that's fine with me -- I'm attaching a new patch which does the work in file-notify instead, which is how I interpret your wish. With that patch, e9e807e931 (addition of the remove-descriptor parameter) can be reverted. >> 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. > > Well, the descriptor is the one we get from filenotify. I don't believe > we shall do double housekeeping. Sounds to me error-prone. Agreed, and filenotify seems to ensure that each watch gets a distinct descriptor even for the same file. I'll prepare a patch for simplifying the table in autorevert.el, unless you can remember the reason for introducing the buffer lists in ef3544f6a6. I have read bug#13540, which is mentioned in the commit message, and am none the wiser.