Skip to content

Make byLine handle carriage newline and just newline equally. #31

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

Merged
merged 1 commit into from
Mar 16, 2016

Conversation

abersnaze
Copy link
Contributor

based on a discussion on #30.

@Crystark
Copy link

You may want to handle \r only too as if I remember correctly, Mac OS 9 has only \r as a new line separator.

@abersnaze
Copy link
Contributor Author

I don't know if I even want to think about what it would take to support running RxJava on what ever version of the JDK on OS 9.

I'll think about regex though.

@Crystark
Copy link

I'm not supposing it would run on OS 9 just that a file may be originating from there (for instance, reading a client's file who runs on an old OS 9)

@abersnaze
Copy link
Contributor Author

@Crystark I got all three line ending forms able to coexist in one regex. If you could give a 👍 I'll merge it.

@Crystark
Copy link

It seems OK to me. Although I would write the pattern like that:
"(\\r\\n)|(\\n)|(\\r)" (or "\\r?\\n|\\r").
If you do not double the anti-slashes, you're actually putting the the real CR and LF characters in the string instead of using the regexp constructs \r and \n. I'm really not sure what are the implications of using one or the other. It may just be fine.

Thanks alot.

@abersnaze
Copy link
Contributor Author

@Crystark made additional changes to the regex to include all of the line terminating characters as listed in java.util.Pattern

@Crystark
Copy link

👍

abersnaze added a commit that referenced this pull request Mar 16, 2016
Make byLine handle carriage newline and just newline equally.
@abersnaze abersnaze merged commit 0a5fb5d into ReactiveX:1.x Mar 16, 2016
@abersnaze abersnaze mentioned this pull request Mar 16, 2016
@abersnaze abersnaze deleted the byLine branch March 17, 2016 07:17
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