Subject: bug#32731: 26.1.50; Ibuffer filter by mode: Handle >1 mode names

From: Eli Zaretskii <[email-masked]>
Date: Wed, 19 Sep 2018 12:42:28 +0300

> From: Tino Calancha <[email-masked]>
> Date: Wed, 19 Sep 2018 18:23:52 +0900
> Cc: [email-masked]
> 
> Following patch adds a key :composable, which also serves as
> documentation.

Thanks, a few nits to the documentation parts:

>  ---
> +*** All mode filters accept a symbol or a list of symbols, i.e., you
> +can filter several major modes with easy.

Please try to keep the first line in a NEWS item a single complete
sentence, as much as possible: that allows for more convenient
browsing in Outline mode.  In this case, I would break this into two
sentences: a short announcement and followup details:

   *** All mode filters can now accept a list of symbols.
   This means you can now easily filter several major modes, as well
   as a single mode.

>  ;;;###autoload (autoload 'ibuffer-filter-by-mode "ibuf-ext")
>  (define-ibuffer-filter mode
> -  "Limit current view to buffers with major mode QUALIFIER."
> +  "Limit current view to buffers with major mode in QUALIFIER.
> +QUALIFIER is the mode name as a symbol or a list of symbols.

In the first sentence, I'd suggest to rephrase:

  Limit current view to buffers with major mode(s) specified by QUALIFIER.

because "in QUALIFIER" only covers the case of a list.

Similarly in other doc strings you modified the same way.

> +Called interactively, accept a comma separated list of mode names.
> +When called interactively, this function allows selection of modes
>  currently used by buffers."

These two sentences seem to be in contradiction, so they probably need
to be merged or rephrased.

>  (cl-defmacro define-ibuffer-filter (name documentation
>  				       (&key
>  					reader
> -					description)
> +					description
> +                                        composable)
>  				       &rest body)
>    "Define a filter named NAME.
>  DOCUMENTATION is the documentation of the function.
>  READER is a form which should read a qualifier from the user.
>  DESCRIPTION is a short string describing the filter.
> +COMPOSABLE is a boolean; if non-nil, the filter accepts both,
                                                               ^
That comma should be deleted, it gets in the way of understanding what
you mean.  And I would actually rephrase

  if non-nil, the filter accepts either a single condition or a list
  of them

because "both" might be interpreted as meaning "and", not "or".

I also wonder whether "COMPOSABLE" is a good name for this argument;
how about ACCEPT-LIST instead?

Thanks for working on this.