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 error regarding unescape_string introduced by #8838 #9143

Merged
merged 1 commit into from
Nov 25, 2014

Conversation

dhoegh
Copy link
Contributor

@dhoegh dhoegh commented Nov 25, 2014

@tkelman I have fixed the issue mentioned in #9137.

@dhoegh dhoegh force-pushed the fix_shell_completion_error branch from c9e46ba to 5f2ba35 Compare November 25, 2014 06:53
@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

Good, looks like this fixes the problem. Can you please add a test for this problem that would fail on current master?

@dhoegh dhoegh force-pushed the fix_shell_completion_error branch from 5f2ba35 to 52a8e39 Compare November 25, 2014 17:33
@dhoegh
Copy link
Contributor Author

dhoegh commented Nov 25, 2014

@tkelman I have adapted a test to test two things could you insure this is crashing on master, I do not have a nightly with it, and I do not have much time right now.

@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

Oddly this does not seem to error on master. I can experiment with things if you can suggest other ways to exercise this code path.

@dhoegh
Copy link
Contributor Author

dhoegh commented Nov 25, 2014

Thats odd because when i do the i gennerates the error:

julia> unescape_string("C:\\U \\alpha")
ERROR: \x used with no following hex digits
 in print_unescaped at string.jl:928

Which is what should be passed to the unescape at line 193, and is also coherent with what you reported.

@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

it looks like startpos is ending up as 10 for that string though, so unescape_string is only being called on \\alpha which works fine

@dhoegh dhoegh force-pushed the fix_shell_completion_error branch from 52a8e39 to 6e0e9d6 Compare November 25, 2014 18:04
@dhoegh
Copy link
Contributor Author

dhoegh commented Nov 25, 2014

It should not matter, I have uploaded a new with a specific test of you problem and it seem to crash on mine now.

@dhoegh
Copy link
Contributor Author

dhoegh commented Nov 25, 2014

You were right it only completes \alpha due to the startpos I’m not thinking clearly after a long day at the University.

@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

No worries, we've all been there. This fixes the error and completion of paths with spaces still works nicely, so it's good enough for me.

tkelman added a commit that referenced this pull request Nov 25, 2014
Fix error regarding unescape_string introduced by #8838
@tkelman tkelman merged commit dcd9ac2 into JuliaLang:master Nov 25, 2014
@tkelman
Copy link
Contributor

tkelman commented Nov 27, 2014

backport now pending at #9164

JeffBezanson added a commit that referenced this pull request Nov 30, 2014
@dhoegh dhoegh deleted the fix_shell_completion_error branch January 13, 2015 08:13
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.

2 participants