Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element

From: "Basil L. Contovounesios" <[email-masked]>
Date: Thu, 14 Mar 2019 16:17:28 +0000

Michael Heerdegen <[email-masked]> writes:

> "Basil L. Contovounesios" <[email-masked]> writes:
>
>> One solution is to leave seq-contains as it is, and switch to using
>> seq-position (or some new predicate) as a predicate instead.  Another is
>> to make seq-contains return a boolean instead of the needle found, which
>> would be a backward-incompatible change similar to that for
>> map-contains-key.  I attach a patch for each of these respective
>> solutions; WDYT?
>
> We also have `seq-some' which can be used as a contain predicate,
> e.g. to fix this bug:
>
> From c53a80c29e696ab64d4279ca6f495c8e0e1b16b4 Mon Sep 17 00:00:00 2001
> From: Michael Heerdegen <[email-masked]>
> Date: Thu, 14 Mar 2019 13:55:41 +0100
> Subject: [PATCH] Fix seq-intersection with nil
>
> ---
>  lisp/emacs-lisp/seq.el | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
> index 4a811d7895..5718343a8f 100644
> --- a/lisp/emacs-lisp/seq.el
> +++ b/lisp/emacs-lisp/seq.el
> @@ -409,12 +409,13 @@ seq-sort-by
>  (cl-defgeneric seq-intersection (sequence1 sequence2 &optional testfn)
>    "Return a list of the elements that appear in both SEQUENCE1 and SEQUENCE2.
>  Equality is defined by TESTFN if non-nil or by `equal' if nil."
> -  (seq-reduce (lambda (acc elt)
> -                (if (seq-contains sequence2 elt testfn)
> -                    (cons elt acc)
> -                  acc))
> -              (seq-reverse sequence1)
> -              '()))
> +  (let ((testfn (or testfn #'equal)))
> +    (seq-reduce (lambda (acc elt)
> +                  (if (seq-some (apply-partially testfn elt) sequence2)
> +                      (cons elt acc)
> +                    acc))
> +                (seq-reverse sequence1)
> +                '())))
>
>  (cl-defgeneric seq-difference (sequence1 sequence2 &optional testfn)
>    "Return a list of the elements that appear in SEQUENCE1 but not in SEQUENCE2.
> --
> 2.20.1
>
>
> (probably needed in more places as you did in one of your patches)
>
> So I think we don't necessarily need something new or a backward
> incompatible change.

My first patch makes an analogous backward-compatible change using the
more efficient seq-position in place of seq-some.

Thanks,

-- 
Basil