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

Remove stripCR function #74

Merged
merged 2 commits into from
Mar 25, 2018
Merged

Conversation

mgsloan
Copy link
Contributor

@mgsloan mgsloan commented Mar 20, 2018

The main place this is useful is after use of some 'lines' function that splits
only on \n. I think for these cases it makes more sense to have new 'lines'
functions which handle both \r\n and \n.

The main place this is useful is after use of some 'lines' function that splits
only on \n.  I think for these cases it makes more sense to have new 'lines'
functions which handle both \r\n and \n.
@snoyberg
Copy link
Collaborator

What about adding such new lines functions at the same time? Note that we should use a different name to make the difference in behavior clear.

@mgsloan mgsloan mentioned this pull request Mar 20, 2018
4 tasks
@mgsloan
Copy link
Contributor Author

mgsloan commented Mar 20, 2018

Makes sense. Right now I'm having trouble thinking of a good name, though. I suppose could always throw a ' on it.

@snoyberg
Copy link
Collaborator

How about linesCR? It's clunky, but it gets the difference across really clearly. And for someone who doesn't know what CR stands for, it's just begging them to look at the docs.

@mgsloan
Copy link
Contributor Author

mgsloan commented Mar 23, 2018

Good idea! I've added a commit which adds those.

Copy link
Collaborator

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@snoyberg snoyberg merged commit c3d6b3c into commercialhaskell:master Mar 25, 2018
roman pushed a commit to roman/rio that referenced this pull request May 25, 2018
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