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

[Nicolas Petton <nicolas@petton.fr>]

[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Nicolas Petton <nicolas@petton.fr> writes:
>
>>  (cl-defgeneric seq-contains (sequence elt &optional testfn)
>> +  (declare (obsolete "Use `seq-contains-p' instead." "27.1"))
>
> According to make-obsolete, "use instead" strings should not start with
> a capital letter, but even better is (obsolete seq-contains-p "27.1"),
> which adds a link to seq-contains-p in the *Help* buffer.

Indeed, that's better, thank you.

>> @@ -420,7 +430,7 @@ seq-sort-by
>>    "Return a list of the elements that appear in SEQUENCE1 but not in SEQUENCE2.
>>  Equality is defined by TESTFN if non-nil or by `equal' if nil."
>>    (seq-reduce (lambda (acc elt)
>> -                (if (not (seq-contains sequence2 elt testfn))
>> +                (if (not (seq-contains-p sequence2 elt testfn))
>>                      (cons elt acc)
>>                    acc))
>
> Is there any harm in inverting this conditional structure, so that it
> reads positively?

It's not the purpose of this patch, but I can commit it separately.

>> +(ert-deftest test-seq-contains-p ()
>> +  (with-test-sequences (seq '(3 4 5 6))
>> +    (should (eq (seq-contains-p seq 3) t))
>> +    (should-not (seq-contains-p seq 7)))
>> +  (with-test-sequences (seq '())
>> +    (should-not (seq-contains-p seq 3))
>> +    (should-not (seq-contains-p seq nil))))
>> +
>
> I think there should also be an explicit nil membership check:
>
> (should (seq-contains-p  [nil] nil))
> (should (seq-contains-p '(nil) nil))

Good idea!

Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]