>From afddf78a703b73729ec7b5562747ebeb5077966d Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" Date: Thu, 14 Mar 2019 12:10:28 +0000 Subject: [PATCH] Do not use seq-contains as a predicate (bug#34852) * doc/lispref/sequences.texi (Sequence Functions): * lisp/emacs-lisp/seq.el (seq-contains): Document unsuitability of seq-contains as a predicate. (seq-position): Document return value when needle not found. (seq-set-equal-p, seq-uniq, seq-intersection, seq-difference): Use seq-position instead of seq-contains as a predicate. * test/lisp/emacs-lisp/seq-tests.el (test-seq-contains-should-return-the-elt): (test-seq-position): Test case when needle is nil. --- doc/lispref/sequences.texi | 7 +++++++ lisp/emacs-lisp/seq.el | 21 ++++++++++++--------- test/lisp/emacs-lisp/seq-tests.el | 14 +++++++++----- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/doc/lispref/sequences.texi b/doc/lispref/sequences.texi index 0c3c4e3b28..e33944a2dd 100644 --- a/doc/lispref/sequences.texi +++ b/doc/lispref/sequences.texi @@ -787,6 +787,9 @@ Sequence Functions @var{elt}. If the optional argument @var{function} is non-@code{nil}, it is a function of two arguments to use instead of the default @code{equal}. +Note that this function is not suitable as a predicate since its value +depends on that of @var{elt}, which may be @code{nil}. + @example @group (seq-contains '(symbol1 symbol2) 'symbol1) @@ -796,6 +799,10 @@ Sequence Functions (seq-contains '(symbol1 symbol2) 'symbol3) @result{} nil @end group +@group +(seq-contains '(symbol1 nil) nil) +@result{} nil +@end group @end example @end defun diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el index 4a811d7895..de8456eb79 100644 --- a/lisp/emacs-lisp/seq.el +++ b/lisp/emacs-lisp/seq.el @@ -357,7 +357,9 @@ seq-sort-by (cl-defgeneric seq-contains (sequence elt &optional testfn) "Return the first element in SEQUENCE that is equal to ELT. -Equality is defined by TESTFN if non-nil or by `equal' if nil." +Equality is defined by TESTFN if non-nil or by `equal' if nil. +Note that this function is not suitable as a predicate since its +value depends on that of ELT, which may be nil." (seq-some (lambda (e) (when (funcall (or testfn #'equal) elt e) e)) @@ -366,12 +368,13 @@ seq-sort-by (cl-defgeneric seq-set-equal-p (sequence1 sequence2 &optional testfn) "Return non-nil if SEQUENCE1 and SEQUENCE2 contain the same elements, regardless of order. Equality is defined by TESTFN if non-nil or by `equal' if nil." - (and (seq-every-p (lambda (item1) (seq-contains sequence2 item1 testfn)) sequence1) - (seq-every-p (lambda (item2) (seq-contains sequence1 item2 testfn)) sequence2))) + (and (seq-every-p (lambda (item1) (seq-position sequence2 item1 testfn)) sequence1) + (seq-every-p (lambda (item2) (seq-position sequence1 item2 testfn)) sequence2))) (cl-defgeneric seq-position (sequence elt &optional testfn) "Return the index of the first element in SEQUENCE that is equal to ELT. -Equality is defined by TESTFN if non-nil or by `equal' if nil." +Return nil if no such element is found. Equality is defined by +TESTFN if non-nil or by `equal' if nil." (let ((index 0)) (catch 'seq--break (seq-doseq (e sequence) @@ -385,7 +388,7 @@ seq-sort-by TESTFN is used to compare elements, or `equal' if TESTFN is nil." (let ((result '())) (seq-doseq (elt sequence) - (unless (seq-contains result elt testfn) + (unless (seq-position result elt testfn) (setq result (cons elt result)))) (nreverse result))) @@ -410,7 +413,7 @@ seq-sort-by "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) + (if (seq-position sequence2 elt testfn) (cons elt acc) acc)) (seq-reverse sequence1) @@ -420,9 +423,9 @@ 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)) - (cons elt acc) - acc)) + (if (seq-position sequence2 elt testfn) + acc + (cons elt acc))) (seq-reverse sequence1) '())) diff --git a/test/lisp/emacs-lisp/seq-tests.el b/test/lisp/emacs-lisp/seq-tests.el index d8f00cfea4..417606d9d7 100644 --- a/test/lisp/emacs-lisp/seq-tests.el +++ b/test/lisp/emacs-lisp/seq-tests.el @@ -183,7 +183,9 @@ test-sequences-oddp (ert-deftest test-seq-contains-should-return-the-elt () (with-test-sequences (seq '(3 4 5 6)) - (should (= 5 (seq-contains seq 5))))) + (should (= 5 (seq-contains seq 5)))) + (should-not (seq-contains '(1 2 nil) nil)) + (should-not (seq-contains [1 2 nil] nil))) (ert-deftest test-seq-every-p () (with-test-sequences (seq '(43 54 22 1)) @@ -384,12 +386,14 @@ test-sequences-oddp (ert-deftest test-seq-position () (with-test-sequences (seq '(2 4 6)) - (should (null (seq-position seq 1))) + (should-not (seq-position seq 1)) + (should-not (seq-position seq nil)) (should (= (seq-position seq 4) 1))) - (let ((seq '(a b c))) - (should (null (seq-position seq 'd #'eq))) + (let ((seq '(a b c nil))) + (should-not (seq-position seq 'd #'eq)) (should (= (seq-position seq 'a #'eq) 0)) - (should (null (seq-position seq (make-symbol "a") #'eq))))) + (should (= (seq-position seq nil #'eq) 3)) + (should-not (seq-position seq (make-symbol "a") #'eq)))) (ert-deftest test-seq-sort-by () (let ((seq ["x" "xx" "xxx"])) -- 2.20.1