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

Handle split CRLF #156

Closed
wants to merge 4 commits into from
Closed

Handle split CRLF #156

wants to merge 4 commits into from

Conversation

alexeits
Copy link

@alexeits alexeits commented Sep 22, 2016

Modify the parser's end of the buffer handling by keeping the line unparsed if the last token is \r and there is more data.

As a result, if the \r\n token is split in half between two buffers it will be joined together in the processing of the second buffer.

This solution is fully backward compatible and much simpler than auto-detection of a row delimiter, which also might be not fully backward compatible.

Fixes issues #150, #146

Alex Tsibulya added 4 commits September 21, 2016 18:00
- Add a test for issue C2FO#150 to specify the expected behavior
- Mark it `skip` pending implementation
- Add a test for CRLF split between two buffers (a.k.a issue C2FO#150) to
  specify the expected behavior
- Mark it `skip` pending implementation
- Modify existing tests for `\r` row delimiter to specify the behavior
  in case of CR vs.CRLF ambiguity issue C2FO#150
- Mark them `skip` pending implementation
Modify the parser to
- parse CRLF as a single token
- keep the current line unparsed if it ends in CR and there's more data

This solves the issues C2FO#146 and C2FO#150 by ensuring that CRLF split by a
buffer boundary doesn't get treated as two row delimiters CR+LF
@doug-martin doug-martin mentioned this pull request Sep 22, 2016
@doug-martin
Copy link
Contributor

Merged and will be published under v2.2.0

@alexeits alexeits deleted the split-crlf branch September 22, 2016 20:28
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