bug#44205: [PATCH] Add new function seq-remove-item

bug#44205: [PATCH] Add new function seq-remove-item

[Stefan Kangas]

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

I found myself reaching for a version of `seq-remove' where I don't have
to supply a lambda but can just give an item.  Ergo, the attached.

    (seq-remove-item 2 '(1 2 3))  => (1 3)

I find it a whole lot nicer than:

    (seq-remove (lambda (a) (= a 2)) '(1 2 3))  => (1 3)

Turns out it could already be used to simplify some code in tab-line.el,
even if seq is arguably not yet very widely used in our sources.

I did not yet add it to NEWS or the manual; I will do that if people
agree that this is a good addition.

Comments?

[-- Attachment #2: 0001-Add-new-function-seq-remove-item.patch --]
[-- Type: text/x-diff, Size: 3729 bytes --]

From 628753c2fe03f86fec39bba97c11000bc1cce130 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Sun, 25 Oct 2020 02:30:26 +0200
Subject: [PATCH] Add new function seq-remove-item

* lisp/emacs-lisp/seq.el (seq-remove-item): New function.
* lisp/emacs-lisp/shortdoc.el (sequence): Document it.

* lisp/tab-line.el (tab-line-tabs-window-buffers)
(tab-line-select-tab-buffer): Use seq-remove-item instead of
seq-remove to simplify the code.
---
 lisp/emacs-lisp/seq.el      |  7 +++++++
 lisp/emacs-lisp/shortdoc.el |  2 ++
 lisp/tab-line.el            | 16 ++++++++--------
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 4656277ea1..82daae6f48 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -331,6 +331,13 @@ seq-remove
   (seq-filter (lambda (elt) (not (funcall pred elt)))
               sequence))
 
+;;;###autoload
+(cl-defgeneric seq-remove-item (item sequence)
+  "Return a list of all the elements in SEQUENCE that are not ITEM.
+The comparison is done using `equal'. "
+  (seq-filter (lambda (elt) (not (equal item elt)))
+           sequence))
+
 ;;;###autoload
 (cl-defgeneric seq-reduce (function sequence initial-value)
   "Reduce the function FUNCTION across SEQUENCE, starting with INITIAL-VALUE.
diff --git a/lisp/emacs-lisp/shortdoc.el b/lisp/emacs-lisp/shortdoc.el
index acc7d13195..20cfd6e295 100644
--- a/lisp/emacs-lisp/shortdoc.el
+++ b/lisp/emacs-lisp/shortdoc.el
@@ -730,6 +730,8 @@ sequence
    :eval (seq-filter #'numberp '(a b 3 4 f 6)))
   (seq-remove
    :eval (seq-remove #'numberp '(1 2 c d 5)))
+  (seq-remove-item
+   :eval (seq-remove-item 3 '(1 2 3 4 5)))
   (seq-group-by
    :eval (seq-group-by #'cl-plusp '(-1 2 3 -4 -5 6)))
   (seq-difference
diff --git a/lisp/tab-line.el b/lisp/tab-line.el
index 46bf89f14e..26f5f750b4 100644
--- a/lisp/tab-line.el
+++ b/lisp/tab-line.el
@@ -385,11 +385,11 @@ tab-line-tabs-window-buffers
 variable `tab-line-tabs-function'."
   (let* ((window (selected-window))
          (buffer (window-buffer window))
-         (next-buffers (seq-remove (lambda (b) (eq b buffer))
-                                   (window-next-buffers window)))
+         (next-buffers (seq-remove-item buffer
+                                        (window-next-buffers window)))
          (next-buffers (seq-filter #'buffer-live-p next-buffers))
-         (prev-buffers (seq-remove (lambda (b) (eq b buffer))
-                                   (mapcar #'car (window-prev-buffers window))))
+         (prev-buffers (seq-remove-item buffer
+                                        (mapcar #'car (window-prev-buffers window))))
          (prev-buffers (seq-filter #'buffer-live-p prev-buffers))
          ;; Remove next-buffers from prev-buffers
          (prev-buffers (seq-difference prev-buffers next-buffers)))
@@ -622,10 +622,10 @@ tab-line-select-tab
 
 (defun tab-line-select-tab-buffer (buffer &optional window)
   (let* ((window-buffer (window-buffer window))
-         (next-buffers (seq-remove (lambda (b) (eq b window-buffer))
-                                   (window-next-buffers window)))
-         (prev-buffers (seq-remove (lambda (b) (eq b window-buffer))
-                                   (mapcar #'car (window-prev-buffers window))))
+         (next-buffers (seq-remove-item window-buffer
+                                        (window-next-buffers window)))
+         (prev-buffers (seq-remove-item window-buffer
+                                        (mapcar #'car (window-prev-buffers window))))
          ;; Remove next-buffers from prev-buffers
          (prev-buffers (seq-difference prev-buffers next-buffers)))
     (cond
-- 
2.28.0

bug#44205: [PATCH] Add new function seq-remove-item

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> I found myself reaching for a version of `seq-remove' where I don't have
> to supply a lambda but can just give an item.  Ergo, the attached.
>
>     (seq-remove-item 2 '(1 2 3))  => (1 3)
>
> I find it a whole lot nicer than:
>
>     (seq-remove (lambda (a) (= a 2)) '(1 2 3))  => (1 3)

Isn't this just

(remove 2 '(1 2 3)) => (1 3)

though?  I don't think seq.el needs to replicate the basic list
functions...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44205: [PATCH] Add new function seq-remove-item

[Unknown]

Stefan Kangas <stefan@marxist.se> writes:

> I found myself reaching for a version of `seq-remove' where I don't have
> to supply a lambda but can just give an item.  Ergo, the attached.
>
>     (seq-remove-item 2 '(1 2 3))  => (1 3)
>
> I find it a whole lot nicer than:
>
>     (seq-remove (lambda (a) (= a 2)) '(1 2 3))  => (1 3)
>
> Turns out it could already be used to simplify some code in tab-line.el,
> even if seq is arguably not yet very widely used in our sources.
>
> I did not yet add it to NEWS or the manual; I will do that if people
> agree that this is a good addition.
>
> Comments?

I don't see a clear need for this new function, because there's already

(delete 2 '(1 2 3)) -> Destructs the original list, compares with equal.
(remove 2 '(1 2 3)) -> Keeps the original list, compares with equal.
(delq 2 '(1 2 3)) -> Destructs the argument list, compares with eq.
(remq 2 '(1 2 3)) -> Keeps the original list, compares with eq.

Maybe we could add or improve cross-references in the documentation so
that these functions are more discoverable?

bug#44205: [PATCH] Add new function seq-remove-item

[Juri Linkov]

> I found myself reaching for a version of `seq-remove' where I don't have
> to supply a lambda but can just give an item.  Ergo, the attached.
>
>     (seq-remove-item 2 '(1 2 3))  => (1 3)

There is no name 'item' used in function names of seq.el.  The term used
here is 'elt', so maybe 'seq-remove-elt' would be a better name.

> I find it a whole lot nicer than:
>
>     (seq-remove (lambda (a) (= a 2)) '(1 2 3))  => (1 3)
>
> Turns out it could already be used to simplify some code in tab-line.el,
> even if seq is arguably not yet very widely used in our sources.

The initial versions of tab-line.el used a more complex condition,
but now a simpler remove/remq could be used instead.

bug#44205: [PATCH] Add new function seq-remove-item

[Basil L. Contovounesios]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> I found myself reaching for a version of `seq-remove' where I don't have
>> to supply a lambda but can just give an item.  Ergo, the attached.
>>
>>     (seq-remove-item 2 '(1 2 3))  => (1 3)
>>
>> I find it a whole lot nicer than:
>>
>>     (seq-remove (lambda (a) (= a 2)) '(1 2 3))  => (1 3)
>
> Isn't this just
>
> (remove 2 '(1 2 3)) => (1 3)
>
> though?  I don't think seq.el needs to replicate the basic list
> functions...

I think the idea is that seq.el functions are generic and can thus be
extended to work with e.g. streams[1].

[1]: http://elpa.gnu.org/packages/stream.html

-- 
Basil

bug#44205: [PATCH] Add new function seq-remove-item

[Basil L. Contovounesios]

Stefan Kangas <stefan@marxist.se> writes:

> diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
> index 4656277ea1..82daae6f48 100644
> --- a/lisp/emacs-lisp/seq.el
> +++ b/lisp/emacs-lisp/seq.el
> @@ -331,6 +331,13 @@ seq-remove
>    (seq-filter (lambda (elt) (not (funcall pred elt)))
>                sequence))
>  
> +;;;###autoload
> +(cl-defgeneric seq-remove-item (item sequence)
> +  "Return a list of all the elements in SEQUENCE that are not ITEM.
> +The comparison is done using `equal'. "
> +  (seq-filter (lambda (elt) (not (equal item elt)))
> +           sequence))

Why not replace seq-filter with seq-remove and do away with the 'not'?

Is the indentation right here?

>  ;;;###autoload
>  (cl-defgeneric seq-reduce (function sequence initial-value)
>    "Reduce the function FUNCTION across SEQUENCE, starting with INITIAL-VALUE.
> diff --git a/lisp/emacs-lisp/shortdoc.el b/lisp/emacs-lisp/shortdoc.el
> index acc7d13195..20cfd6e295 100644
> --- a/lisp/emacs-lisp/shortdoc.el
> +++ b/lisp/emacs-lisp/shortdoc.el
> @@ -730,6 +730,8 @@ sequence
>     :eval (seq-filter #'numberp '(a b 3 4 f 6)))
>    (seq-remove
>     :eval (seq-remove #'numberp '(1 2 c d 5)))
> +  (seq-remove-item
> +   :eval (seq-remove-item 3 '(1 2 3 4 5)))
>    (seq-group-by
>     :eval (seq-group-by #'cl-plusp '(-1 2 3 -4 -5 6)))
>    (seq-difference
> diff --git a/lisp/tab-line.el b/lisp/tab-line.el
> index 46bf89f14e..26f5f750b4 100644
> --- a/lisp/tab-line.el
> +++ b/lisp/tab-line.el
> @@ -385,11 +385,11 @@ tab-line-tabs-window-buffers
>  variable `tab-line-tabs-function'."
>    (let* ((window (selected-window))
>           (buffer (window-buffer window))
> -         (next-buffers (seq-remove (lambda (b) (eq b buffer))
> -                                   (window-next-buffers window)))
> +         (next-buffers (seq-remove-item buffer
> +                                        (window-next-buffers window)))
>           (next-buffers (seq-filter #'buffer-live-p next-buffers))
> -         (prev-buffers (seq-remove (lambda (b) (eq b buffer))
> -                                   (mapcar #'car (window-prev-buffers window))))
> +         (prev-buffers (seq-remove-item buffer
> +                                        (mapcar #'car (window-prev-buffers window))))
>           (prev-buffers (seq-filter #'buffer-live-p prev-buffers))
>           ;; Remove next-buffers from prev-buffers
>           (prev-buffers (seq-difference prev-buffers next-buffers)))
> @@ -622,10 +622,10 @@ tab-line-select-tab
>  
>  (defun tab-line-select-tab-buffer (buffer &optional window)
>    (let* ((window-buffer (window-buffer window))
> -         (next-buffers (seq-remove (lambda (b) (eq b window-buffer))
> -                                   (window-next-buffers window)))
> -         (prev-buffers (seq-remove (lambda (b) (eq b window-buffer))
> -                                   (mapcar #'car (window-prev-buffers window))))
> +         (next-buffers (seq-remove-item window-buffer
> +                                        (window-next-buffers window)))
> +         (prev-buffers (seq-remove-item window-buffer
> +                                        (mapcar #'car (window-prev-buffers window))))

As others have pointed out, these would be better served by remq, and/or
fusing multiple loops into a single one.

Thanks,

-- 
Basil

bug#44205: [PATCH] Add new function seq-remove-item

[Stefan Kangas]

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

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Stefan Kangas <stefan@marxist.se> writes:
>>
>>> I found myself reaching for a version of `seq-remove' where I don't have
>>> to supply a lambda but can just give an item.  Ergo, the attached.
>>>
>>>     (seq-remove-item 2 '(1 2 3))  => (1 3)
>>>
>>> I find it a whole lot nicer than:
>>>
>>>     (seq-remove (lambda (a) (= a 2)) '(1 2 3))  => (1 3)
>>
>> Isn't this just
>>
>> (remove 2 '(1 2 3)) => (1 3)
>>
>> though?  I don't think seq.el needs to replicate the basic list
>> functions...
>
> I think the idea is that seq.el functions are generic and can thus be
> extended to work with e.g. streams[1].

That was the idea, yes.

It is fairly minor even in my use, so I have no qualms with just closing
this as wontfix if people think it's not worthwhile enough to add.

bug#44205: [PATCH] Add new function seq-remove-item

[Stefan Kangas]

tags 44205 wontfix
close 44205
thanks

Stefan Kangas <stefan@marxist.se> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>
>>> Stefan Kangas <stefan@marxist.se> writes:
>>>
>>>> I found myself reaching for a version of `seq-remove' where I don't have
>>>> to supply a lambda but can just give an item.  Ergo, the attached.
>>>>
>>>>     (seq-remove-item 2 '(1 2 3))  => (1 3)
>>>>
>>>> I find it a whole lot nicer than:
>>>>
>>>>     (seq-remove (lambda (a) (= a 2)) '(1 2 3))  => (1 3)
>>>
>>> Isn't this just
>>>
>>> (remove 2 '(1 2 3)) => (1 3)
>>>
>>> though?  I don't think seq.el needs to replicate the basic list
>>> functions...
>>
>> I think the idea is that seq.el functions are generic and can thus be
>> extended to work with e.g. streams[1].
>
> That was the idea, yes.
>
> It is fairly minor even in my use, so I have no qualms with just closing
> this as wontfix if people think it's not worthwhile enough to add.

It seems like there was no great enthusiasm for this change.  I'm
therefore closing this bug report.