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

Previous Next

Full log

View this message in rfc822 format

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

> From: Damien Cassou <damien <at> cassou.me>
> Cc: 57548 <at> debbugs.gnu.org
> Date: Sat, 03 Sep 2022 10:01:26 +0200
> 
> 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?

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.

Previous Next

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