-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Description
The behaviour of findnext
and findprev
are inconsistent between char/string pairs, bytearrays and arrays.
This is quite annoying and needless, and I'd like to fix it.
For background, here is the current behaviour for arrays:
findnext(a, b, c)
raises a BoundsError
if c < firstindex(b)
, but not if c > lastindex(b)
in which case is returns nothing
. Mirroring this, findprev(a, b, c)
raises an error if c > lastindex(b)
but not if c < firstindex(b)
where it returns nothing
.
However, for strings we have:
findnext(a, b, c)
raises aBoundsError
ifc < firstindex(b)
. However,findprev(a, b, c)
does not raise an error ifc > lastindex(b)
. Instead, this raises an error ifc > ncodeunits(b) + 1
which is doubly confusing (whyncodeunits
and notlastindex
? Why the +1?)- When
c == ncodeunits(b) + 1
,findprev(a, b, c)
returnsnothing
unconditionally. I believe this is a bug. findnext(a, b, c)
raises an error whenc > ncodeunits(b) + 1
, and returnsnothing
whenc == ncodeunits(b) + 1
. This is inconsistent with the behaviour for arrays, but consistent withfindprev
which raises an error ifc < firstindex(b)
and returns nothing whenc == firstindex(b) - 1
.- For strings,
findnext(a, b, c)
raises an error if!isvalid(b, c)
. However,findprev
does not do so.
This is a mess. I think there are two reasonable alternatives:
- The string behaviour is changed so it matches array behaviour, also using
lastindex
. There will be no check forisvalid
starting index. This is the most straightforward fix. - Array behaviour is changed such that
findnext(a, b, c)
wherec < firstindex(b)
is equivalent tofindnext(a, b, firstindex(c))
, and vice versa forfindprev
. String behaviour is updated to match this.
As far as I can tell, the string behaviour is specifically tested for, so it appears intentional. The array behaviour of these edge cases is, as far as I can tell, not tested for.
I can make a PR to fix this if the intended behaviour is agreed upon.