[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#57548: Add new function `seq-positions'
From: |
Damien Cassou |
Subject: |
bug#57548: Add new function `seq-positions' |
Date: |
Sat, 03 Sep 2022 10:01:26 +0200 |
Eli Zaretskii <eliz@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@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
Description: Text Data
bug#57548: Add new function `seq-positions', Michael Heerdegen, 2022/09/02