A few thoughts on seq.el

[Tino Calancha]

Dear Nico,

I realize a delightful minimalist design choice in this lib:
to keep just the minimum set of funtions providing same funcionality.
For instance, seq.el has neither `seq-notevery' nor `seq-notany'
which can be easily obtained as negation of `seq-every-p' and `seq-some'.

Another example, instead of defining two functions per each operation
,as CL does (*) for several functions,

I) func (seq, elt), one receiving args (SEQ ELT).
II) func-if (pred, seq), other receiving args (PRED SEQ).

seq.el seems to prefer introducing just the more general funcs in II) 
(**).

There are two functions that escape from this logic.
A) seq-position
  I suggest to define this function so that it receives args as II),
  like other functions in the file.
  With the current definition, it is not possible to recover
  the useful behaviour in II) (***).

B) seq-contains
  Once you add this in the lib, the pair (seq-contains, seq-find)
  is analog to CL (find, find-if).  This breaks the minimalist design
  choice mentioned above.

  I believe `seq-contains' was introduced for convenience:
  it is used elsewhere in seq.el and map.el.
  For consistency with the rest of the file, i would expect
  not having defined `seq-contains', though.

C) Does something like this have any sense? ; possible with different name
(map-delete-if PRED MAP)
"Delete all keys from MAP satisfying (PRED key val) and return MAP.
If MAP is an array, store nil at the index of each deleted key.

MAP can be a list, hash-table or array."

;; As `map-remove' but returning the same type as (type-of MAP).

D) If the only dependence on 'cl-lib in seq.el is `cl-subseq',
   Would be possible to include a func `seq-replace'
   (stolen from `cl-replace') and then define `seq-subseq',
   a stolen version of `cl-subseq' as well?

   That would remove the 'cl-lib dependence, one of the
   arguments to not load 'seq at Emacs start up, AFAIK.
   Does it have any sense?

----------
(*)
In fact, CL define func-if-not, as well.

(**)
We can get I) behaviour from II) with ease.

(***)
- we can get CL `position' from `position-if'.
- we cannot get CL `position-if' from `position'.


Regards,
Tino


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> From 1fc3a02cbc275e98876efbc9a85e65891c4937c3 Mon Sep 17 00:00:00 2001
From: Tino Calancha <address@hidden>
Date: Sun, 31 Jul 2016 13:39:01 +0900
Subject: [PATCH 1/2] seq-position: accepts 2 args, PRED and SEQUENCE

* lisp/emacs-lisp/seq.el (seq-position): Function accepts 2 arg
PRED and SEQUENCE; return the index of the first element in
SEQUENCE for which (PRED element) is non-nil.
All callers updated.
---
 lisp/emacs-lisp/seq.el            |  9 ++++-----
 lisp/emacs-lisp/subr-x.el         |  2 +-
 lisp/ibuffer.el                   |  2 +-
 test/lisp/emacs-lisp/seq-tests.el | 10 +++++-----
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index e5004f8..9a97f55 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -353,13 +353,12 @@ seq-sort-by
                 e))
             sequence))

-(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."
+(cl-defgeneric seq-position (pred sequence)
+"Return the index of the first element in SEQUENCE for which (PRED 
element) is non-nil."
   (let ((index 0))
     (catch 'seq--break
-      (seq-doseq (e sequence)
-        (when (funcall (or testfn #'equal) e elt)
+      (seq-doseq (elt sequence)
+        (when (funcall pred elt)
           (throw 'seq--break index))
         (setq index (1+ index)))
       nil)))
diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el
index 173cd11..7d067d2 100644
--- a/lisp/emacs-lisp/subr-x.el
+++ b/lisp/emacs-lisp/subr-x.el
@@ -233,7 +233,7 @@ read-multiple-choice
            (mapconcat
             (lambda (elem)
               (let* ((name (cadr elem))
-                     (pos (seq-position name (car elem)))
+                     (pos (seq-position (lambda (e) (equal e (car elem))) 
name))
                      (altered-name
                       (cond
                        ;; Not in the name string.
diff --git a/lisp/ibuffer.el b/lisp/ibuffer.el
index 8e24629..4ff753c 100644
--- a/lisp/ibuffer.el
+++ b/lisp/ibuffer.el
@@ -1813,7 +1813,7 @@ name
   (let ((string (propertize (buffer-name)
                             'font-lock-face
                             (ibuffer-buffer-name-face buffer mark))))
-    (if (not (seq-position string ?\n))
+    (if (not (seq-position (lambda (e) (char-equal e ?\n)) string))
         string
       (replace-regexp-in-string
        "\n" (propertize "^J" 'font-lock-face 'escape-glyph) string))))
diff --git a/test/lisp/emacs-lisp/seq-tests.el 
b/test/lisp/emacs-lisp/seq-tests.el
index c2065c6..8302cf4 100644
--- a/test/lisp/emacs-lisp/seq-tests.el
+++ b/test/lisp/emacs-lisp/seq-tests.el
@@ -359,12 +359,12 @@ test-sequences-oddp

 (ert-deftest test-seq-position ()
   (with-test-sequences (seq '(2 4 6))
-    (should (null (seq-position seq 1)))
-    (should (= (seq-position seq 4) 1)))
+    (should-not (seq-position (lambda (e) (= e 1)) seq))
+    (should (= (seq-position (lambda (e) (= e 4)) seq) 1)))
   (let ((seq '(a b c)))
-    (should (null (seq-position seq 'd #'eq)))
-    (should (= (seq-position seq 'a #'eq) 0))
-    (should (null (seq-position seq (make-symbol "a") #'eq)))))
+    (should-not (seq-position (lambda (e) (eq e 'd)) seq))
+    (should (= (seq-position (lambda (e) (eq e 'a)) seq) 0))
+    (should-not (seq-position (lambda (e) (eq e (make-symbol "a"))) 
seq))))

 (ert-deftest test-seq-sort-by ()
   (let ((seq ["x" "xx" "xxx"]))
--
2.8.1

> From bd7284cdd09908caa5ef7c89b669b8a0be0a20d8 Mon Sep 17 00:00:00 2001
From: Tino Calancha <address@hidden>
Date: Sun, 31 Jul 2016 14:22:20 +0900
Subject: [PATCH 2/2] Delete seq-contains

* lisp/emacs-lisp/seq.el (seq-contains): Delete this function.
Updated all callers: callers use 'seq-find' instead.
---
 lisp/emacs-lisp/map.el            |  4 +++-
 lisp/emacs-lisp/seq.el            | 12 +++++++++---
 test/lisp/emacs-lisp/seq-tests.el | 16 ++++++----------
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el
index 98a8871..cce6bd6 100644
--- a/lisp/emacs-lisp/map.el
+++ b/lisp/emacs-lisp/map.el
@@ -264,7 +264,9 @@ map-contains-key
 Equality is defined by TESTFN if non-nil or by `equal' if nil.

 MAP can be a list, hash-table or array."
-  (seq-contains (map-keys map) key testfn))
+  (seq-find (lambda (e)
+              (funcall (or testfn #'equal) e key))
+            (map-keys map)))

 (defun map-some (pred map)
   "Return a non-nil if (PRED key val) is non-nil for any key/value pair 
in MAP.
diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 9a97f55..5082449 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -368,7 +368,9 @@ 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-find
+               (lambda (e) (funcall (or testfn #'equal) e elt))
+               result)
         (setq result (cons elt result))))
     (nreverse result)))

@@ -393,7 +395,9 @@ 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-find (lambda (e)
+                                (funcall (or testfn #'equal) e elt))
+                              sequence2)
                     (cons elt acc)
                   acc))
               (seq-reverse sequence1)
@@ -403,7 +407,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))
+                (if (not (seq-find (lambda (e)
+                                     (funcall (or testfn #'equal) e elt))
+                                   sequence2))
                     (cons elt acc)
                   acc))
               (seq-reverse sequence1)
diff --git a/test/lisp/emacs-lisp/seq-tests.el 
b/test/lisp/emacs-lisp/seq-tests.el
index 8302cf4..8cde9ac 100644
--- a/test/lisp/emacs-lisp/seq-tests.el
+++ b/test/lisp/emacs-lisp/seq-tests.el
@@ -171,19 +171,15 @@ test-sequences-oddp
   (should-not (seq-find #'null '(1 nil 2)))
   (should-not (seq-find #'null '(1 nil 2) t))
   (should-not (seq-find #'null '(1 2 3)))
-  (should (seq-find #'null '(1 2 3) 'sentinel)))
-
-(ert-deftest test-seq-contains ()
+  (should (seq-find #'null '(1 2 3) 'sentinel))
   (with-test-sequences (seq '(3 4 5 6))
-    (should (seq-contains seq 3))
-    (should-not (seq-contains seq 7)))
+    (should (seq-find (lambda (e) (equal e 3)) seq))
+    (should-not (seq-find (lambda (e) (equal e 7)) seq)))
   (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 ()
+    (should-not (seq-find (lambda (e) (equal e 3)) seq))
+    (should-not (seq-find (lambda (e) (equal e nil)) seq)))
   (with-test-sequences (seq '(3 4 5 6))
-    (should (= 5 (seq-contains seq 5)))))
+    (should (= 5 (seq-find (lambda (e) (equal e 5)) seq)))))

 (ert-deftest test-seq-every-p ()
   (with-test-sequences (seq '(43 54 22 1))
--
2.8.1

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.20.6)
 of 2016-07-31 built on calancha-pc
Repository revision: 7d58b02f363ab02961faa950d1ba727df96f2f19