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

Fix #9209 #9210

Merged
merged 1 commit into from
Dec 6, 2014
Merged

Fix #9209 #9210

merged 1 commit into from
Dec 6, 2014

Conversation

ivarne
Copy link
Member

@ivarne ivarne commented Dec 1, 2014

This fixes #9209, which is a regression from #9137. The problem was that the Latex completion code
might access an invalid index in the unicode string, if the previous
character from a \\ was a multibyte character.

cc @tkelman @Luthaf @dhoegh

@tkelman
Copy link
Contributor

tkelman commented Dec 1, 2014

Did you intend to cancel the appveyor run? As @Keno said here #9209 (comment) the fix for this should come with a regression test.

@ivarne
Copy link
Member Author

ivarne commented Dec 1, 2014

Yes, it ran once and failed on 64 bit, and I tried to rerun only that test. AppVeyor decided to run both again, so I canceled, but the link was already updated. I agree about a regression test, so I haven't merged this yet. I'll have to dive somewhat deeper into this to understand how I might write the test.

@dhoegh
Copy link
Contributor

dhoegh commented Dec 2, 2014

You could make the regresion test like s = "α\\alpha" on https://github.com/JuliaLang/julia/blob/master/test/replcompletions.jl#L70

@tkelman
Copy link
Contributor

tkelman commented Dec 2, 2014

@dhoegh useful trick when you're referencing lines of code, hit y when you're viewing the file and github will reload the page with the current sha in the link, that way the same line of code will make sense in the future

Regression from #9137. The problem was that the Latex completion code
might access an invalid index in the unicode string, if the previous
character from a `\\` was a multibyte character.
@ivarne
Copy link
Member Author

ivarne commented Dec 6, 2014

Sorry that it took so long to write the test. Now it's there and passes.

@tkelman
Copy link
Contributor

tkelman commented Dec 6, 2014

Was this not fixed by #9227 ?

@stevengj
Copy link
Member

stevengj commented Dec 6, 2014

@tkelman, no, that was a separate problem.

stevengj added a commit that referenced this pull request Dec 6, 2014
@stevengj stevengj merged commit 03919cb into master Dec 6, 2014
@ivarne ivarne deleted the in/complete branch December 6, 2014 09:34
ivarne added a commit that referenced this pull request Dec 7, 2014
Regression from #9137. The problem was that the Latex completion code
might access an invalid index in the unicode string, if the previous
character from a `\\` was a multibyte character.

ref: #9210
(cherry picked from commit 9c14147)
@ivarne
Copy link
Member Author

ivarne commented Dec 7, 2014

Backported in 6033478

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REPL crash with LaTeX completions
4 participants