-
-
Notifications
You must be signed in to change notification settings - Fork 7
Support parsing multiline values starting with quoted newline #25
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
Conversation
|
Looked at internal implementation of |
src/Decoder.php
Outdated
| \reset($data); | ||
| if ($last === "\n" && ($newline === 1 || $this->buffer[$newline - 1] !== $this->enclosure)) { | ||
| $edgeCase = \substr($this->buffer, $newline - 2, 3); | ||
| if ($last === "\n" && ($newline === 1 || $this->buffer[$newline - 1] !== $this->enclosure || $edgeCase === $this->delimiter . $this->enclosure ."\n")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KamilBalwierz Thanks for looking into this 👍
Your casing seems a bit off, just a minor fix?
The rest looks fine to me :)
|
Thx for the update 👍. The only thing I have to note is that I don't think it's necessary to have two commits for these changes, maybe you could squash these two together? |
|
Nice work 👍 |
clue
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KamilBalwierz Thank you for looking into this, changes LGTM! ![]()
I agree that this is starting to look more complex than I'd like, but I agree that this would be a good first step to support parsing empty lines. Let's get this in and review future code improvements in a future PR 👍
I have encountered issue with multiline parsing.
str_getcsvfor line ending with single instance of encloser will return as correct content, so if field starts by newline this causes parser to stop assuming it read full correct fieldfgetcsvcorrectly looks for further input in case of following input:For now I am just checking for edgecase (single instance of encloser preceeded by delimiter) to handle this, but it looks like proper solution will have to stop relying on
str_getcsvand do full text parsing instead.