Fixes handling of double quotes for unescaped fields with tests #182
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…te text fields while containing quote characters. See new test case for expected functionality. Origional functionality was to fail on parse as field was terminated on doublequote.
|
Yes, this would be helpful! Currently, I have to maintain a cassava fork because of this |
|
Frankly, I am a bit uneasy with the change request, because the package prominently states that it implements RFC 4180: Lines 7 to 9 in 5a410c9 If we should deviate from RFC 4180, then only under a flag. (Alternatively, the faithful implementation could be under flag rfc4180 or strict-rfc-4180, and the more liberal/practical implementation the default.)@robwithhair @johannesgerer What do you think? |
|
What is the status here? I'd also really appreciate having this as an option! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
related to issue #98.
This is to fix a possible Oracle / MS Excel CSV export weirdness where fields can be created which are 'unescaped' but still contain double quote characters. While it violates RFC4180, there is no reason why we cannot parse these files correctly. I would argue that it is more correct to parse the field without error than to terminate a field early, hence splitting the field and providing confusing error message.
Example of such a field unescaped
BSV "Frohsinn" Mehr-Ork-Gest e.Example of such a field escaped
"BSV ""Frohsinn"" Mehr-Ork-Gest e".Both fields can appear in CSV files. Because the first character is not a double quote, the string is considered unquoted but it can still be parsed. Previous functionality was to error with a confusing error message due to fields being split in two at the double quote character.
I have run tests and all seem to be passing. I have also added a new test for this functionality.
Although all tests are passing there is a possibility that existing code depending on the error could be affected though I believe this is unlikely. It would be wise to test with some varied known working CSV files.