unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Tino Calancha <tino.calancha@gmail.com>
To: nicolas@petton.fr
Cc: tino.calancha@gmail.com, emacs-devel@gnu.org
Subject: A few thoughts on seq.el
Date: Sun, 31 Jul 2016 18:31:15 +0900 (JST)	[thread overview]
Message-ID: <alpine.DEB.2.20.1607311829360.5244@calancha-pc> (raw)


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 <tino.calancha@gmail.com>
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 <tino.calancha@gmail.com>
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




             reply	other threads:[~2016-07-31  9:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-31  9:31 Tino Calancha [this message]
2016-07-31 17:36 ` A few thoughts on seq.el Nicolas Petton
2016-07-31 17:49   ` Clément Pit--Claudel
2016-07-31 19:19     ` Nicolas Petton
2016-08-02  2:49   ` Tino Calancha
2016-08-01  8:48 ` Nicolas Petton
2016-08-01 12:06   ` Clément Pit--Claudel
2016-08-01 12:24     ` Nicolas Petton
2016-08-01 21:56   ` Michael Heerdegen
2016-08-01 22:08     ` Michael Heerdegen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1607311829360.5244@calancha-pc \
    --to=tino.calancha@gmail.com \
    --cc=emacs-devel@gnu.org \
    --cc=nicolas@petton.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).