Subject: bug#57548: Add new function `seq-positions'

From: Eli Zaretskii <[email-masked]>
Date: Sat, 03 Sep 2022 13:16:32 +0300

> From: Damien Cassou <[email-masked]>
> Cc: [email-masked]
> Date: Sat, 03 Sep 2022 10:01:26 +0200
> 
> Eli Zaretskii <[email-masked]> 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?

LGTM, thanks.

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

As you changed the text to talk about indices, I don't think anything
else is needed.

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

Yes, please.  Will be appreciated.

> >> +(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?

No, I've missed that part

Thanks.