[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#35418: [PATCH] Don't poll auto-revert files that use notification
From: |
Mattias Engdegård |
Subject: |
bug#35418: [PATCH] Don't poll auto-revert files that use notification |
Date: |
Mon, 29 Apr 2019 13:06:50 +0200 |
29 apr. 2019 kl. 09.53 skrev Michael Albinus <michael.albinus@gmx.de>:
>
>> Here is an updated patch. There is a new variable,
>> `auto-revert-always-poll', which is t by default.
>> There is also a note in etc/NEWS. Does it merit a mention in the manual as
>> well?
>
> Yes, please.
There is now a paragraph added to the manual.
By the way, the organisation of this part of the manual could be improved --
don't you agree?
There is a section called Reverting, which starts about `revert-buffer' but
then goes on to talk about the auto-revert, global-auto-revert and
auto-revert-tail modes and details about the mechanisms behind them: polling,
intervals, notification.
Then there is a (sibling) section called Autorevert, which despite its name
only talks about auto-reverting non-file buffers.
This can be reorganised in various ways. We could move all autorevert text to a
sibling node to Reverting, or to one or more child nodes. In any case, such
text shuffling should not be part of this patch.
> I believe it shall be said, that this user option does not compete with
> `auto-revert-use-notify'. Rather, polling is used additionally to file
> notification. When `auto-revert-use-notify' is nil, the value of
> `auto-revert-always-poll' doesn't matter; there will always be polling.
Good point; the doc string has been clarified.
> Saying this, the user option might need another name. What about
> `auto-revert-also-poll'?
Naming is always hard. I started with `auto-revert-avoid-polling' but wanted to
avoid a negative name.
I tried `auto-revert-also-poll' but it somehow didn't feel right; not all
buffers use notification.
It is nothing I feel strongly about, so if you do prefer that name I'll change,
but I've kept the original name in the patch for now.
>> +(defvar auto-revert--polled-buffers ()
>> + "List of buffers in Auto-Revert Mode that must be polled.
>> +It contains the buffers in `auto-revert-buffer-list' whose
>> +`auto-revert-notify-watch-descriptor' is nil.")
>
> Is this variable needed? It is used only once in
> `auto-revert--need-polling', and it could be computed easily by
It is also used in `auto-revert-buffers', but you are quite right that it could
be a function. I decided to maintain it as a derived state because it felt
silly to replace O(1) code with O(N), and the invariant is clear enough (stated
in its doc string). (Some of the places where the variable is updated are O(N)
but less frequently executed.)
I can replace it with a function if you want, but the code didn't seem to gain
much from doing so.
> `auto-revert--need-polling' shall always return the buffer list, also for
> `global-auto-revert-mode'.
Sorry, it was meant as a predicate and is only used as such.
Clarified by renaming it to `auto-revert--need-polling-p'.
Thank you very much for your review! Updated patch attached.
0001-Don-t-poll-auto-revert-files-that-use-notification.patch
Description: Binary data
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, (continued)
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Mattias Engdegård, 2019/04/25
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Eli Zaretskii, 2019/04/25
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Mattias Engdegård, 2019/04/25
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Michael Albinus, 2019/04/27
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Eli Zaretskii, 2019/04/27
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Michael Albinus, 2019/04/27
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Mattias Engdegård, 2019/04/27
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Eli Zaretskii, 2019/04/27
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Mattias Engdegård, 2019/04/28
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Michael Albinus, 2019/04/29
- bug#35418: [PATCH] Don't poll auto-revert files that use notification,
Mattias Engdegård <=
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Michael Albinus, 2019/04/29
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Eli Zaretskii, 2019/04/29
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Mattias Engdegård, 2019/04/29
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Michael Albinus, 2019/04/29
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Eli Zaretskii, 2019/04/29
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Mattias Engdegård, 2019/04/30
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Michael Albinus, 2019/04/30
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Mattias Engdegård, 2019/04/30
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Michael Albinus, 2019/04/30
- bug#35418: [PATCH] Don't poll auto-revert files that use notification, Eli Zaretskii, 2019/04/29