GNU bug report logs - #44205
[PATCH] Add new function seq-remove-item

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 44205 in the body.
You can then email your comments to 44205 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add new function seq-remove-item
Date: Sat, 24 Oct 2020 17:52:04 -0700

[Message part 1 (text/plain, inline)]

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?

[0001-Add-new-function-seq-remove-item.patch (text/x-diff, attachment)]

Message #8 received at 44205 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44205 <at> debbugs.gnu.org
Subject: Re: bug#44205: [PATCH] Add new function seq-remove-item
Date: Sun, 25 Oct 2020 13:49:55 +0100

Stefan Kangas <stefan <at> 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

Message #11 received at 44205 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Martín <mardani29 <at> yahoo.es>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44205 <at> debbugs.gnu.org
Subject: Re: bug#44205: [PATCH] Add new function seq-remove-item
Date: Sun, 25 Oct 2020 17:25:02 +0100

Stefan Kangas <stefan <at> 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?

Message #14 received at 44205 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44205 <at> debbugs.gnu.org
Subject: Re: bug#44205: [PATCH] Add new function seq-remove-item
Date: Sun, 25 Oct 2020 21:12:46 +0200

> 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.

Message #17 received at 44205 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44205 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#44205: [PATCH] Add new function seq-remove-item
Date: Tue, 27 Oct 2020 13:36:07 +0000

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Stefan Kangas <stefan <at> 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

Message #20 received at 44205 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44205 <at> debbugs.gnu.org
Subject: Re: bug#44205: [PATCH] Add new function seq-remove-item
Date: Tue, 27 Oct 2020 13:45:46 +0000

Stefan Kangas <stefan <at> 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

Message #23 received at 44205 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44205 <at> debbugs.gnu.org
Subject: Re: bug#44205: [PATCH] Add new function seq-remove-item
Date: Thu, 12 Nov 2020 19:07:52 -0500

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

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> Stefan Kangas <stefan <at> 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.

Message #26 received at 44205 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 44205 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#44205: [PATCH] Add new function seq-remove-item
Date: Wed, 18 Nov 2020 09:17:53 -0800

tags 44205 wontfix
close 44205
thanks

Stefan Kangas <stefan <at> marxist.se> writes:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>>
>>> Stefan Kangas <stefan <at> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.