From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH v3] Allow applying filters to summary consecutively Date: Sun, 06 Nov 2022 09:34:30 +0200 Message-ID: <8335aw35a1.fsf@gnu.org> References: <871qqshyeo.fsf@autistici.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="1280"; mail-complaints-to="usenet@ciao.gmane.io" Cc: rms@gnu.org, rpluim@gmail.com, emacs-devel@gnu.org To: Andrea Monaco Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Nov 06 08:35:57 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oraCD-00007U-3z for ged-emacs-devel@m.gmane-mx.org; Sun, 06 Nov 2022 08:35:57 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oraBB-0000Bb-Lp; Sun, 06 Nov 2022 02:34:53 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oraB9-0000BQ-R7 for emacs-devel@gnu.org; Sun, 06 Nov 2022 02:34:51 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oraB9-0001Fo-49; Sun, 06 Nov 2022 02:34:51 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=cqe+t1APRfBrndWLqFKDrzZJ9m3leibqboiwi3jJJh8=; b=hHaYviHUHibQ kMlTHRRzT731k9Yv/vCpikHBknXM1PVxG+wb/hI/4+nbQGSnCkbzddS+M/Bbpvz/voHt5Qwn5XbVm GJv2NKNDQeLOaSN2+FelY17oCiA72FlGMe5nguUy6NMeWyK7CdVZm9ysdY72kwDlX9sMuZSapDFFu idp0qLvdPTcsmU5ERoVin7iwSPna8dZH5p+ewlBzj7q6WrFAx0ajZxhf8PvaSn1/ep8/WlJnbQcRu RYitQLiplxM5SlYYavKeWH2Ige+55w4X6Pqz6Fa3Gt+0TdsiWfgb2a0k3cdMD1EFzLwtb394fwmoJ Hx+mTt2778ESSFzn4EkrCA==; Original-Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oraB7-0001mn-PV; Sun, 06 Nov 2022 02:34:50 -0500 In-Reply-To: <871qqshyeo.fsf@autistici.org> (message from Andrea Monaco on Fri, 28 Oct 2022 15:26:39 +0200) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: "Emacs-devel" Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:299241 Archived-At: > From: Andrea Monaco > Cc: rms@gnu.org, rpluim@gmail.com, emacs-devel@gnu.org > Date: Fri, 28 Oct 2022 15:26:39 +0200 > > This version should be final. Famous last words ;-) Seriously, though: having to apply 3 patches one on top of the other is something I'd like to avoid, so in the future if you don't feel the patch is final, please say so when you post it, to prevent me from doing unnecessary jobs. I have several comments to the patch, mainly on the doc strings: > -(defcustom rmail-summary-apply-filters-consecutively nil > - "If non-nil, Rmail summary commands apply filtering on top existing filtering. > -When this variable is non-nil, `rmail-summary-by-*' commands work on the > -current summary, and so their filtering can be stacked one on top of another. > -This allows gradual narrowing of the selection of the messages." > +(defcustom rmail-summary-intersect-consecutive-filters nil > + "Non-nil means that commands rmail-summary-by-* works on the > +current summary and so can be intersected one after the other." The first line of a doc string must be a complete sentence. In the original code, I made sure that was so, but your changes destroy that. In addition, I don't think I understand what you mean by "intersected", and neither will users who will read this doc string. Please think of some better, clearer description, if you think the original one was inaccurate or incorrect for some reason. > (defvar rmail-summary-currently-displayed-msgs nil > - "String made of `y' and `n'. > -The character at position i tells wether message i is shown in the > -summary or not. First character is ignored. > -Used when applying `rmail-summary-by-*' commands consecutively. Filled > -by `rmail-summary-fill-displayed-messages'.") > + "Boolean vector that for index i tells whether message i is > +shown on the summary or not. First element is ignored. Used > +when applying rmail-summary-by-* commands consecutively. Filled > +by rmail-summary-populate-displayed-messages.") Same here: the first line must be a complete sentence. Also, references to variables and functions inside doc string should be quoted `like this', to allow Help commands to create hyperlinks from them. > -(defun rmail-summary-fill-displayed-messages () > - "Fill the rmail-summary-currently-displayed-msgs string." > +(defun rmail-summary-populate-displayed-messages () > + "Populate the rmail-summary-currently-displayed-msgs vector." Quoting of variable names again. > (defun rmail-summary-negate () > - "Toggle display of messages that match the summary and those which do not." > + "Negate the current summary. That is, show the messages that > +are not displayed, and vice versa." The first line of a doc string must be a single complete sentence. And I don't really understand why you changed the original doc string. Was it incorrect? I think using "negate" is not a good idea, since it's too technical and doesn't explain clearly enough what this command does. The original doc string attempted to clarify that in simple terms. > +(defun rmail-summary--exists-1 () > + "Like rmail-summary-exists, but works in both main and summary buffers." Please quote `like this' the function name mentioned in the doc string. Thanks.