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

Added "\r" to the valid line terminators. ... #116

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rickowens
Copy link

"\r" still exists in many places in the wild, mainly because of Mac
OS. For instance we recently hit a case where Gmail would export "\r"
delimited CSV when exporting contacts.

"\r" still exists in many places in the wild, mainly because of Mac
OS. For instance we recently hit a case where Gmail would export "\r"
delimited CSV when exporting contacts.
@bgamari
Copy link
Collaborator

bgamari commented Apr 8, 2016

Hmm, it seems like this may break existing users' code in non-obvious ways. It seems like if you need to handle these cases saying so explicitly may be the better option.

@rickowens
Copy link
Author

Hmm, it seems like this may break existing users' code in non-obvious ways.

Possibly yes. On the other hand, if the purpose of this function is to provide an easy eol catchall, then code that relies on this already is broken, and this change may cause more subtle fixes than it will breakages? Also, the future where a developer has to continually explain why they are using attoparsec-eol or whatever, "because attoparsec's endOfLine doesn't work" doesn't seem great.

In the case where you are relying on third-party libraries that rely on endOfLine, you would have to either 1) convince the maintainers to adopt attoparsec-eol, 2) rewrite the functionality of those libraries if the maintainers won't adopt it, or 3) run some kind of transformation on the bytestrings to make them conform to what endOfLine understands. None of those options seem very sexy either. It also does not seem great that people in the future might see a function named endOfLine, think "Aha! just what I need", but fail to read the fine print.

If the purpose of endOfLine is not to provide catchall compatibility, what would you think about specifically calling that fact out in the docs, and adding catchallEOL whose purpose is to be as broad as possible? This would at least save me a lot of explanation in PRs, and make it easier to convince other maintainers to switch over when it is appropriate for them to do so. :-)

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