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.

Sign up for GitHub

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

Jump to bottom

Correct length to endof in SubString call #530

Merged
merged 2 commits into from
Jul 19, 2017
Merged

Correct length to endof in SubString call #530

merged 2 commits into from
Jul 19, 2017

Conversation

bkamins
Copy link
Contributor

@bkamins bkamins commented Jul 2, 2017 •
edited by mortenpi
Loading

length will fail on multibyte Char.
E.g.

x = "∀∀∀"
length(x) # 3 - results incorrect indexing
endof(x) # 7 - correct index of last Char

This is part of the fix needed for JuliaLang/julia#22511.

Fixes #470.

@mortenpi
Copy link
Member

mortenpi commented Jul 2, 2017

Any ideas whether this might fix #470?

@bkamins bkamins mentioned this pull request Jul 2, 2017
Merged
@bkamins
Copy link
Contributor Author

bkamins commented Jul 2, 2017

@mortenpi The old code was dropping characters for multibyte Chars in strings (as @tkelman suggested in #470).
The reason is that length(s) is not the index for the last character in a string. After JuliaLang/julia#22511 this causes fail to build at all as this PR enforces strict checking of indexing into SubString (currently it silently returned wrong result).

@nalimilan
Copy link
Contributor

So the stricter indexing rules have already caught one bug? That's great news!

nalimilan
nalimilan reviewed Jul 2, 2017
View reviewed changes
src/Utilities/TextDiff.jl Outdated
@@ -48,7 +48,7 @@ function splitby(reg::Regex, text::AbstractString)
push!(out, SubString(text, last, each.match.offset + each.match.endof))
last = each.match.endof + each.offset
end
local laststr = SubString(text, last, length(text))
local laststr = SubString(text, last, endof(text))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

endof(text) is redundant, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right 😄

@mortenpi mortenpi added this to the v0.11.2 milestone Jul 19, 2017
@mortenpi
Copy link
Member

LGTM, thanks! I can confirm that it fixes #470 as well. I'll try to put out a (long overdue) patch release with this one shortly so that JuliaLang/julia#22511 could be merged.

@mortenpi mortenpi merged commit ed1f8bf into JuliaDocs:master Jul 19, 2017
mortenpi pushed a commit that referenced this pull request Jul 19, 2017
@bkamins @mortenpi
Correct length to endof in SubString call (#530)
4493b7d 
(cherry picked from commit ed1f8bf)
@mortenpi mortenpi mentioned this pull request Jul 19, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nalimilan nalimilan nalimilan left review comments

Assignees
No one assigned
Labels
Type: Bug
Projects
None yet
Milestone
0.11.2
Development

Successfully merging this pull request may close these issues.

Doctest failure diff not showing up right on current Julia master
3 participants
@bkamins @mortenpi @nalimilan