Re: [PATCH] Allow applying filters to summary consecutively (was: Summary by thread in rmail)

[Eli Zaretskii <eliz@gnu.org>]

> From: Andrea Monaco <andrea.monaco@autistici.org>
> Cc: rms@gnu.org
> Date: Sun, 09 Oct 2022 22:33:01 +0200
> 
> This patch adds optional argument keep-filtering to rmail-summary-by-*
> commands.  When t, the filter works on the currently displayed summary.

Thanks.  But the rmail-summary-* are commands, not just functions.
How can an Rmail user invoke these commands to work only on the
messages in the currently-displayed summary?  Without some user
ability to take advantage of these new options, the feature is less
useful, IMO.

> +(defun rmail-summary-negate ()
> +  "Invert the current set of displayed messages in the summary."

The doc string should explain what is meant by "negation" here.

> +  (interactive)
> +  (rmail-summary-fill-displayed-messages)
> +  (rmail-new-summary "Negate"

I think "Negate" is not the best description of the summary being
produced.  How about "NOT" instead?  It should produce mode-line
display such as

   RMAIL Summary: NOT regexp ....

> +		     (lambda (msg)
> +		       (if
> +			   (char-equal (aref rmail-summary-currently-displayed-msgs msg)
> +				       ?n)

Shouldn't a simple '=' be enough here, instead of char-equal (here and
elsewhere in the patch)?  These y/n markers are completely internal,
AFAIU, so all the additional features of char-equal, such as
case-non-sensitivity, should not be relevant.

> -(defun rmail-summary-by-labels (labels)
> +(defun rmail-summary-by-labels (labels &optional keep-filtering)
>    "Display a summary of all messages with one or more LABELS.
>  LABELS should be a string containing the desired labels, separated by commas."

Please add the description of the new optional argument to the doc
strings of all the functions you are modifying.