From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Michael Albinus Newsgroups: gmane.emacs.bugs Subject: bug#35418: [PATCH] Don't poll auto-revert files that use notification Date: Mon, 29 Apr 2019 14:18:20 +0200 Message-ID: <87imuxxawj.fsf@gmx.de> References: <83sgu71b91.fsf@gnu.org> <74CB5185-5DA1-4786-BD9C-9EEB3D43B3C1@acm.org> <83o94uz9h2.fsf@gnu.org> <875zqzssql.fsf@gmx.de> <83d0l7v193.fsf@gnu.org> <86EFE367-90FF-4786-BC91-FC28FAF38A4E@acm.org> <877ebdqmbj.fsf@gmx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="266617"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: 35418@debbugs.gnu.org To: Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Apr 29 14:19:15 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hL5FW-00179t-Lm for geb-bug-gnu-emacs@m.gmane.org; Mon, 29 Apr 2019 14:19:10 +0200 Original-Received: from localhost ([127.0.0.1]:56869 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hL5FV-0006JN-Mm for geb-bug-gnu-emacs@m.gmane.org; Mon, 29 Apr 2019 08:19:09 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:42790) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hL5FP-0006JD-1S for bug-gnu-emacs@gnu.org; Mon, 29 Apr 2019 08:19:04 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hL5FN-0007Cy-Vq for bug-gnu-emacs@gnu.org; Mon, 29 Apr 2019 08:19:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:53259) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hL5FN-0007Ct-T2 for bug-gnu-emacs@gnu.org; Mon, 29 Apr 2019 08:19:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hL5FN-0000pw-OP for bug-gnu-emacs@gnu.org; Mon, 29 Apr 2019 08:19:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Michael Albinus Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 29 Apr 2019 12:19:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 35418 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 35418-submit@debbugs.gnu.org id=B35418.15565403143183 (code B ref 35418); Mon, 29 Apr 2019 12:19:01 +0000 Original-Received: (at 35418) by debbugs.gnu.org; 29 Apr 2019 12:18:34 +0000 Original-Received: from localhost ([127.0.0.1]:38570 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hL5Ew-0000pH-1M for submit@debbugs.gnu.org; Mon, 29 Apr 2019 08:18:34 -0400 Original-Received: from mout.gmx.net ([212.227.15.18]:51251) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hL5Eu-0000p4-Tj for 35418@debbugs.gnu.org; Mon, 29 Apr 2019 08:18:33 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1556540302; bh=4YrDRyHUJNODAdaA0WQdyefL0jsk95UR8t45uAs/TQY=; h=X-UI-Sender-Class:From:To:Cc:Subject:References:Date:In-Reply-To; b=lJctczP7AtK+7rhLxnrradZCnZeYT9wYwByVmhO7m4hUnRHZtqOINch0x2dLvK1kv VJHSFcTGuGUkkwGYQAgOh5i3OPXDDwk3AmNwYjmOcCUf31n9Q9F+ZT1+SSig3i9uGD Ew0WxnyyRYQsXUchb29ERv3Tak+PD1BSMtbV7dDE= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Original-Received: from detlef.gmx.de ([213.220.159.69]) by mail.gmx.com (mrgmx004 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MqaxO-1gyviI28RM-00maWP; Mon, 29 Apr 2019 14:18:22 +0200 In-Reply-To: ("Mattias \=\?utf-8\?Q\?Engdeg\=C3\=A5rd\=22's\?\= message of "Mon, 29 Apr 2019 13:06:50 +0200") X-Provags-ID: V03:K1:W805P8V9yNodyHMFZAqVT2wc63IoE+9vcA0brfXh1hTXJ5oyRGG ffoiu8yWJ+o4sz+pEapV24+wGB2Q9KTMBu6DKcOajjtuq6DeuyM0CnXPENEEuOpjNMjkx8W QZhpLB4bNoEmxphRfMUboqbEkDCk1V6vn4VwN85O2090ncnuTqp8U+kUvAtbWKef/c8zLqc g/Vd0aZ6y9po4riuwjz4Q== X-UI-Out-Filterresults: notjunk:1;V03:K0:1gxL7+nJn48=:IiCqHeFX6Waa12Nfukz4xU MzUQi5VEP1KL1F5zCrwGyT115T7g5HKFagOp9YYoo3IKizDMEOOHTWDaGFqcNoT4dqtCmRwre jv+isk3lpAYMbAhl7FxLSAj4F7wmRcIkzkyUDXmdkA+LxXDVvK3mxJDTyqnSyCcxEdSUUwtkM 7Gn8EJXQonc1V1oqh63xsnXrPqDBD6pCVYjA08CcFCDnYpjRQuCfx64lGpHBBVtxnt/B9howq jYkvp1yBQbbcat01Bg460qcpg9q0UJ7pYlzJTIdksdWPKsSnSXFpdcloLxroMJrcFj7/aV9kW TsNMPH7OB4DmIXCALIylxoJftWe4Yq0IVr97t55RRJTe0QwqrgSQlVl6FsjnCaqS6wIiLX87n M7NZFuz35w1XqpmcAvd1C+lER8+HLPMNpSyf2wX69jrQTscm8zpc3yWU+FDHZf6DwSDdF5TAE RqF8uwbSAePnRHBZ01HQSM/A70WVGwgkqqDF6yHHH6+LABkx9pEdnWJ9qRXmD6j0C5lxeOVvS N1NHwO5On2rCAVRxJkZbxTXLPWSfgfZK6egPdylNQ4nb+GCxN/A8GIZZNc33LCSy6nwkXfJEp 9ErON7njWsSvP+VyVAFcmMeaJBRAeaIHP1a8Lv9ArVoeuh6szIPDIALF6Eqdofmbf6ri6pi1k +sq6IIJEnnJrg+tciNIptWDDs/MrU9QZYymcG13KfDo83mdNtVtipTkJIg1LNIme8LtgIWykl cjiTnlpmAASEGKO51OqQ8D2/VxYdsghEYmm/jP1LqTmDVJ9DAJAlbDoXotoxK3SHnSzo0ULz X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:158423 Archived-At: Mattias Engdeg=C3=A5rd writes: > 29 apr. 2019 kl. 09.53 skrev Michael Albinus : >>=20 >>> 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? >>=20 >> 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? I hardly disagree, this is always true :-) > 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 would let it for you. >> 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. I have also hard times when choosing a proper name. Do what you believe is best suited, unless Eli comes with something better. (It is my experience over years, that he beats me always with better names.) >> 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. Yes, but the function as proposed would fit as well. > 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.) Yes, but is N large enough to experience the difference? > I can replace it with a function if you want, but the code didn't seem > to gain much from doing so. There are several places you need to modify the variable. This gave me the impression that one function would fit better, because if you need to touch (set) avariable at several places, there are good chances to miss it somewhere. I'm not saying you do in this case, it is just my style to keep things together (in one function, for example). >> `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'. My proposal was to use it NOT as a predicate, but as a function returning the buffer list. > Thank you very much for your review! Updated patch attached. Best regards, Michael.