change string indexing to avoid invalid indexes #3770
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eliminate the thisind() function
For example, search("∀ x", "∀", 1) used to give 1:3, which doesn't
make much sense since the "3" is an artifact of the encoding and isn't
a valid index. Now it gives 1:1, or in general a:b where a is the index
of the first found character and b is the index of the last found character.
|
Is this in scope for 0.2? |
|
I guess we can vote. I think this is really important. @ViralBShah @StefanKarpinski |
|
In that case, I vote in favor. |
…hanges Conflicts: base/exports.jl
…hanges Conflicts: base/exports.jl
…jb/strindexchanges
|
bump |
|
Yeah, I'm cool with this. |
JeffBezanson
added a commit
that referenced
this pull request
Aug 9, 2013
change string indexing to avoid invalid indexes
IanButterworth
pushed a commit
that referenced
this pull request
Jan 24, 2024
…5539d (#53028) Stdlib: Pkg URL: https://github.com/JuliaLang/Pkg.jl.git Stdlib branch: release-1.10 Julia branch: backports-release-1.10 Old commit: 11cf00df7 New commit: 70525539d Julia version: 1.10.0 Pkg version: 1.10.0 Bump invoked by: @IanButterworth Powered by: [BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl) Diff: JuliaLang/Pkg.jl@11cf00d...7052553 ``` $ git log --oneline 11cf00df7..70525539d 70525539d Merge pull request #3770 from JuliaLang/backports-release-1.10 729ebe1e3 Avoid deleting existing artifacts when ignoring hashes. (#3768) ``` Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
Drvi
pushed a commit
to RelationalAI/julia
that referenced
this pull request
Jun 7, 2024
…5539d (JuliaLang#53028) Stdlib: Pkg URL: https://github.com/JuliaLang/Pkg.jl.git Stdlib branch: release-1.10 Julia branch: backports-release-1.10 Old commit: 11cf00df7 New commit: 70525539d Julia version: 1.10.0 Pkg version: 1.10.0 Bump invoked by: @IanButterworth Powered by: [BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl) Diff: JuliaLang/Pkg.jl@11cf00d...7052553 ``` $ git log --oneline 11cf00df7..70525539d 70525539d Merge pull request JuliaLang#3770 from JuliaLang/backports-release-1.10 729ebe1e3 Avoid deleting existing artifacts when ignoring hashes. (JuliaLang#3768) ``` Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I felt it was strange that using invalid indexes was fairly routine in our string code and API. Some functions returned them, and some functions practically expected them as input. There were
i+1andi-1s sprinkled here and there, to be fixed up eventually by a call tothisind. I don't think this is a good practice, and we should just use valid indexes. The only exception is the special indexnextind(s, endof(s)), which identifies the empty space at the end of the string (for example when searching for the empty string).The clearest example is in the commit message;
search("∀ x", "∀", 1)gave1:3. Usinglast(r)of that range could result in an invalid index error. Now it gives1:1, a range of character indexes that is inclusive like all other index ranges. This can of course still be passed togetindex, which then no longer needs to callthisind.Another way to look at this is that some functions tolerated invalid string indexes, and others gave errors, which is a bit confusing. It also made the behavior of
isvalid,nextind, andprevindmore ambiguous, since some indexes were completely invalid due to being out of bounds, while others were "less" invalid.This also makes
nextandnextindmore consistent: they give the same value when called onendof(s).