GNU bug report logs - #57548
Add new function `seq-positions'

Previous Next

Full log

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

From: Damien Cassou <damien <at> cassou.me>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 57548 <at> debbugs.gnu.org
Subject: Re: bug#57548: Add new function `seq-positions'
Date: Sat, 03 Sep 2022 10:01:26 +0200

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

Eli Zaretskii <eliz <at> gnu.org> writes:
> We use "index", not "position".

I changed the text in the manual, NEWS and docstring to talk about
"index" instead of "position".  I kept the word "positions" in the
function name because there is already a `seq-position` function and the
2 are so similar that I think they deserve a similar name. What do you
think?


> In any case, the documentation should explain what you mean by that,


I haven't found another such explanation in seq.texi so I'm not sure
what you means.  I would be happy to write something if you feel
something is still missing though.


> and it should definitely say that the index/position are zero-based.


done in the manual, NEWS and docstring.


>> +@group
>> +(seq-position '(a b c a d) 'z)
>> +@result{} nil
>
> seq-position or seq-positions?


you are so right.  Thank you for the careful review.


> Our style is to say
>   Equality is defined by the function TESTFN, which defaults to `equal'.


fixed.  If you want, I can prepare another patch to apply the same
change to the docstring of the already existing `seq-position`: it
contains the same phrasing.


>> +(ert-deftest test-seq-positions ()
>> +  (with-test-sequences (seq '(1 2 3 1 4))
>> +    (should (equal '(0 3) (seq-positions seq 1)))
>> +    (should (seq-empty-p (seq-positions seq 9)))))
>
> Should we test more than just one type of sequences?


The `with-test-sequences` call checks 3 types of sequences already as
far as I understand. Do you mean something else?


Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> We do not need to limit this to equivalence relations.  A TESTFUN of, say,
> (apply-partially #'<= 10) could be similarly useful.


Indeed, such a function would certainly make sense.  I would refrain
from calling it `seq-positions` though because there is already a
`seq-position` function that is very similar.  I guess the function you
suggest wouldn't take an element as argument either.  In any case, I
think the current function makes sense on its own and what you are
asking for is a new function that seems out of scope of this current
patch.  If you give me a name (what about `(seq-matching-positions seq
pred)`?), I volunteer to send a new patch in a new mail thread.

Best

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

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

Previous Next

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