Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nextind("", 1, 0) should probably work #31319

Open
StefanKarpinski opened this issue Mar 11, 2019 · 1 comment
Open

nextind("", 1, 0) should probably work #31319

StefanKarpinski opened this issue Mar 11, 2019 · 1 comment
Assignees
Labels
strings "Strings!"

Comments

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Mar 11, 2019

#31312 is a fix for chop that seems to be hiding a deeper problem, namely that nextind("", 1, 0) should probably work instead of throwing an error. In general, when the number of characters to move is zero, I believe that any 0 ≤ i ≤ ncodeunits(s)+1 should be allowed in order to agree with thisind. When n > 0 then it makes sense to require 0 ≤ i < ncodeunits(s)+1. Motivation: well, the code that #31312 fixes is a perfect example. Similarly, prevind("", 0, 0) should also work.

cc @bkamins

@StefanKarpinski StefanKarpinski added the strings "Strings!" label Mar 11, 2019
@StefanKarpinski StefanKarpinski self-assigned this Mar 11, 2019
@StefanKarpinski StefanKarpinski added this to the 1.x milestone Mar 11, 2019
@bkamins
Copy link
Member

bkamins commented Mar 11, 2019

If we mentally agree that 0 and ncodeunits(s)+1 are "would be" valid indices then this is consistent.
As the design for n=0 assumed that you can call it using a valid index only and will get a valid index in return. I do not see the risk when it would hurt to extend the definition the way you propose.

When this is fixed in the docstring s should be replaced by str both in prevind and nextind (wrong variable name was used in 3 places).

@DilumAluthge DilumAluthge removed this from the 1.x milestone Mar 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
strings "Strings!"
Projects
None yet
Development

No branches or pull requests

3 participants