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

normalize literal \r and \r\n chars in string literals to \n #14073

Merged
merged 1 commit into from
Nov 20, 2015

Conversation

stevengj
Copy link
Member

This fixes #11988, by normalizing literal \r and \r\n characters in multiline string literals to \n. Explicitly escaped \r chars are unaffected.

Also fixes #10833, I think (unix2dos test/markdown.jl && make -C test markdown passes).

@stevengj stevengj added the strings "Strings!" label Nov 20, 2015
@stevengj
Copy link
Member Author

@tkelman, is this something that should be backported? I would tend to view it as a bugfix; I'd be very suspicious of any code that relied on the old behavior.

@StefanKarpinski
Copy link
Member

I agree that this should be backported.

@StefanKarpinski
Copy link
Member

CI failure looks unrelated and incidental.

stevengj added a commit that referenced this pull request Nov 20, 2015
normalize literal \r and \r\n chars in string literals to \n
@stevengj stevengj merged commit e6cd2bc into JuliaLang:master Nov 20, 2015
@stevengj stevengj deleted the crlf_literals branch November 20, 2015 20:09
@tkelman
Copy link
Contributor

tkelman commented Nov 20, 2015

This should probably have been left open for review a little longer than 2 hours. I'm not entirely convinced forcing this behavior at the parser level is right. Seems like the output bytes should maybe match the input bytes here, and string handling routines in Markdown and elsewhere should be made smarter wrt CRLF handling.

@tkelman
Copy link
Contributor

tkelman commented Nov 20, 2015

This is probably fine, especially considering Python behaves this way, but it should really be documented as the way the language parses multiline strings - somewhat related to the handling of escape codes.

It also doesn't fix any issues stemming from Markdown or other string handling methods when reading from an external file, just hides the underlying issue for julia source files.

@stevengj
Copy link
Member Author

I thought that there was a clear consensus in #11988 that this was the desired behavior, sorry I jumped the gun on the merge button. I'll push a documentation update.

Also, the hornéd god hath spoken upon this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
strings "Strings!"
Projects
None yet
3 participants