From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: "Basil L. Contovounesios" Newsgroups: gmane.emacs.bugs Subject: bug#34852: 26.1; seq-intersection ignores nil as element Date: Thu, 14 Mar 2019 12:22:40 +0000 Message-ID: <87zhpxabn3.fsf@tcd.ie> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="135531"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: Nicolas Petton , 34852@debbugs.gnu.org, Stefan Monnier To: "Miguel V. S. Frasson" Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Mar 14 13:23:25 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1h4POL-000Z1h-L3 for geb-bug-gnu-emacs@m.gmane.org; Thu, 14 Mar 2019 13:23:21 +0100 Original-Received: from localhost ([127.0.0.1]:35928 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h4POK-0007zj-Ik for geb-bug-gnu-emacs@m.gmane.org; Thu, 14 Mar 2019 08:23:20 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:36453) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h4PO5-0007y2-23 for bug-gnu-emacs@gnu.org; Thu, 14 Mar 2019 08:23:06 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h4PO3-00006w-ES for bug-gnu-emacs@gnu.org; Thu, 14 Mar 2019 08:23:05 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:57756) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1h4PO2-00006b-Fk for bug-gnu-emacs@gnu.org; Thu, 14 Mar 2019 08:23:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1h4PO2-0005HL-7s for bug-gnu-emacs@gnu.org; Thu, 14 Mar 2019 08:23:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "Basil L. Contovounesios" Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 14 Mar 2019 12:23:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 34852 X-GNU-PR-Package: emacs Original-Received: via spool by 34852-submit@debbugs.gnu.org id=B34852.155256617120260 (code B ref 34852); Thu, 14 Mar 2019 12:23:02 +0000 Original-Received: (at 34852) by debbugs.gnu.org; 14 Mar 2019 12:22:51 +0000 Original-Received: from localhost ([127.0.0.1]:43067 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1h4PNq-0005Gh-Ux for submit@debbugs.gnu.org; Thu, 14 Mar 2019 08:22:51 -0400 Original-Received: from mail-ed1-f53.google.com ([209.85.208.53]:40246) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1h4PNo-0005GS-7j for 34852@debbugs.gnu.org; Thu, 14 Mar 2019 08:22:49 -0400 Original-Received: by mail-ed1-f53.google.com with SMTP id r23so4419014edm.7 for <34852@debbugs.gnu.org>; Thu, 14 Mar 2019 05:22:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=flEyVJn1Zef5nRHP7cCJFz5sT92t8Lk4H77Y+wmsrjU=; b=mF6aRndhlkt3j4y/4jsUbaVxbbXQWrPRnSrdT3vrJ9liFXbBBRordrZBR3+KVpJhPF 0jYHI+v27Kl81rTlNqHIQtDOmzfjZ4soqhWmlKUrnZPsnGCpo0HAMMF/KriXTJa4s3V7 fwQqhyQaVvdT3Q+a+qtHri5t1e9a7kURCokTd5j4CG8pj2RWOxH4BOgZfFnNFRpc0krB JbV9jFZtCvlve0GB3z73pW3chwocwYK0eoSIlEFl8KeJ1L95SLTT79DX/CpHzRA01Sg6 nDfRyzndVMCNRgT73iX2VmOLLn5WxIf4YzfO993M5JvNjGcl5pJnRZWJFgD7Q+qAMh/P MxoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=flEyVJn1Zef5nRHP7cCJFz5sT92t8Lk4H77Y+wmsrjU=; b=dIZFX1q20vCRecTJvgNXou1VXHxfzfKiXwrLeflFAKB3Tp5enJyJ69RzAls2GoMUvy nOIgSDvIjy+zL8REXmXgTmxcEBxM144t8OtFgpId7EAkeU065HAV2WIx5Wt89rNe10Dw v8nzjhpLmcRsQrPCVtIJiaBfT7SYr8oF8dKfWGcqOnAlHNLuSkLq8NkMMgCd4kHWAHmx c+lAofioOWXy7ZXCd0gpnL6ssiMnYG3GHCb+CNxK2tJNWQyDQtD3wB0xTzM1JCeKbLj2 7HQmG6aGVJ3qRllvKS1Xwr9Uwie4HiG+ouh3Kp6Fe1QnAzsiJR5qlAFNu6iYTys5GpNZ mwRA== X-Gm-Message-State: APjAAAW7+Zhe+nDat4EWgCv7GaxTUV/5GnuDJk8E6CMyZEgxQDqu1BFO YI3wv0zYf0kityS+gUtem8oyDQ== X-Google-Smtp-Source: APXvYqymXaSV/EahZQ5NGsoDtDJ9nsacYF9WLsXVUv6TRJ9U9CuQTsLMlCv8aTtPZwAHDPsBPrIkdQ== X-Received: by 2002:a05:6402:1807:: with SMTP id g7mr11321776edy.184.1552566162391; Thu, 14 Mar 2019 05:22:42 -0700 (PDT) Original-Received: from localhost ([2a02:8084:20e2:c380:20c2:134e:4f3a:683a]) by smtp.gmail.com with ESMTPSA id g20sm1066163ejk.72.2019.03.14.05.22.41 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Thu, 14 Mar 2019 05:22:41 -0700 (PDT) In-Reply-To: (Miguel V. S. Frasson's message of "Wed, 13 Mar 2019 23:16:30 -0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:156323 Archived-At: --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Do-not-use-seq-contains-as-a-predicate-bug-34852.patch >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 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Return-boolean-instead-of-element-in-seq-contains.patch >From 7b8d5672db306e3bb52196b04c74eb7098ec32a3 Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" Date: Thu, 14 Mar 2019 11:19:55 +0000 Subject: [PATCH] Return boolean instead of element in seq-contains * lisp/emacs-lisp/seq.el: Bump major version. (seq-contains): Return non-nil instead of needle if it is found in the sequence. (bug#34852) * test/lisp/emacs-lisp/seq-tests.el (test-seq-contains): Add check for nil as an element. (test-seq-contains-should-return-the-elt): Remove, replacing with... (test-seq-contains-return-non-nil): ...this new test. * doc/lispref/sequences.texi (Sequence Functions): * etc/NEWS: Document change to seq-contains. --- doc/lispref/sequences.texi | 9 +++++---- etc/NEWS | 3 +++ lisp/emacs-lisp/seq.el | 7 +++---- test/lisp/emacs-lisp/seq-tests.el | 9 +++++---- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/doc/lispref/sequences.texi b/doc/lispref/sequences.texi index 0c3c4e3b28..b0c65fe526 100644 --- a/doc/lispref/sequences.texi +++ b/doc/lispref/sequences.texi @@ -783,14 +783,15 @@ Sequence Functions @defun seq-contains sequence elt &optional function - This function returns the first element in @var{sequence} that is equal to -@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}. + This function returns non-@code{nil} if an element in @var{sequence} +is equal to @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}. @example @group (seq-contains '(symbol1 symbol2) 'symbol1) -@result{} symbol1 +@result{} t @end group @group (seq-contains '(symbol1 symbol2) 'symbol3) diff --git a/etc/NEWS b/etc/NEWS index 410c1821ae..b41ff89613 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -383,6 +383,9 @@ buffers. New convenience functions 'seq-first' and 'seq-rest' give easy access to respectively the first and all but the first elements of sequences. ++++ +*** 'seq-contains' now returns a boolean rather than the element found. + --- ** Follow mode In the current follow group of windows, "ghost" cursors are no longer diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el index 4a811d7895..73cb985b85 100644 --- a/lisp/emacs-lisp/seq.el +++ b/lisp/emacs-lisp/seq.el @@ -4,7 +4,7 @@ ;; Author: Nicolas Petton ;; Keywords: sequences -;; Version: 2.21 +;; Version: 3.0 ;; Package: seq ;; Maintainer: emacs-devel@gnu.org @@ -356,11 +356,10 @@ seq-sort-by count)) (cl-defgeneric seq-contains (sequence elt &optional testfn) - "Return the first element in SEQUENCE that is equal to ELT. + "Return non-nil if an element in SEQUENCE is equal to ELT. Equality is defined by TESTFN if non-nil or by `equal' if nil." (seq-some (lambda (e) - (when (funcall (or testfn #'equal) elt e) - e)) + (funcall (or testfn #'equal) elt e)) sequence)) (cl-defgeneric seq-set-equal-p (sequence1 sequence2 &optional testfn) diff --git a/test/lisp/emacs-lisp/seq-tests.el b/test/lisp/emacs-lisp/seq-tests.el index d8f00cfea4..fee31e7f8f 100644 --- a/test/lisp/emacs-lisp/seq-tests.el +++ b/test/lisp/emacs-lisp/seq-tests.el @@ -176,14 +176,15 @@ test-sequences-oddp (ert-deftest test-seq-contains () (with-test-sequences (seq '(3 4 5 6)) (should (seq-contains seq 3)) - (should-not (seq-contains seq 7))) + (should-not (seq-contains seq 7)) + (should-not (seq-contains seq nil))) (with-test-sequences (seq '()) (should-not (seq-contains seq 3)) (should-not (seq-contains seq nil)))) -(ert-deftest test-seq-contains-should-return-the-elt () - (with-test-sequences (seq '(3 4 5 6)) - (should (= 5 (seq-contains seq 5))))) +(ert-deftest test-seq-contains-return-non-nil () + (should (seq-contains '(1 2 nil) nil)) + (should (seq-contains [1 2 nil] nil))) (ert-deftest test-seq-every-p () (with-test-sequences (seq '(43 54 22 1)) -- 2.20.1 --=-=-= Content-Type: text/plain "Miguel V. S. Frasson" writes: > seq-intersection skips nil as common element, so returns wrong result. > > Reproducing from emacs -Q: > > In *scratch* buffer, eval the expressions > > (progn > (require 'seq) > (seq-intersection '(1 2 nil) '(1 nil) 'eq)) > > -> (1) > > (seq-intersection '(1 2 nil) '(1 nil) 'equal) > > -> (1) > > Expected result in both cases: (1 nil) This is actually due to seq-contains returning the element found, rather than a boolean indicating whether the element was found: (seq-contains '(nil) nil) ; => nil The nature of seq-contains as a predicate-ish has been discussed in the past[1], and Stefan recently fixed a similar problem with map-contains-key[2]. [1]: https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg01256.html [2]: * lisp/emacs-lisp/map.el: Make the functions generic 2018-12-11 17:54:13 -0500 https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=1691a51094d35ac4b2c311fa407c6b77eea7a105 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? -- Basil --=-=-=--