Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd `invalid: :replace` for `CSV.open` #130
Merged
Conversation
lib/csv/writer.rb
Outdated
| @@ -156,7 +156,7 @@ def quote(field) | |||
| else | |||
| field = String(field) # Stringify fields | |||
| # represent empty fields as empty quoted fields | |||
| if (@quote_empty and field.empty?) or @quotable_pattern.match?(field) | |||
| if (@quote_empty and field.empty?) or @quotable_pattern.match?(field.scrub) | |||
kou
Jun 3, 2020
Member
I don't think that this is a good approach.
If we have \uFFFD in @quotable_pattern, it matches all invalid characters.
We need to process this case carefully something like:
Suggested change
if (@quote_empty and field.empty?) or @quotable_pattern.match?(field.scrub)
if (@quote_empty and field.empty?) or (filed.valid_encoding? and @quotable_pattern.match?(field)
Could you create a separated pull request for this case and then we can back to this pull request?
I don't think that this is a good approach.
If we have \uFFFD in @quotable_pattern, it matches all invalid characters.
We need to process this case carefully something like:
Suggested change
| if (@quote_empty and field.empty?) or @quotable_pattern.match?(field.scrub) | |
| if (@quote_empty and field.empty?) or (filed.valid_encoding? and @quotable_pattern.match?(field) |
Could you create a separated pull request for this case and then we can back to this pull request?
koic
Jun 4, 2020
•
Author
Contributor
Thanks for your suggestion. I think that is definitely a safe approach and suitable. I've opened #131.
Thanks for your suggestion. I think that is definitely a safe approach and suitable. I've opened #131.
koic
Jun 4, 2020
Author
Contributor
I rebased this PR with the latest mater branch.
I rebased this PR with the latest mater branch.
koic
force-pushed the
koic:add_invalid_replace_option_for_csv_open
branch
from
76bdf53
to
59b4305
This PR adds `invalid: :replace` for `CSV.open`. It is a PR similar to #129. And this PR uses `String#scrub` to prevent the following `ArgumentError`. ```ruby /[,"]/.match?("\x82\xA0") #=> ArgumentError (invalid byte sequence in UTF-8) /[,"]/.match?("\x82\xA0".scrub) #=> false ```
koic
force-pushed the
koic:add_invalid_replace_option_for_csv_open
branch
from
59b4305
to
d1f9bc0
|
Thanks! |
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.
This PR adds
invalid: :replaceforCSV.open. It is a PR similar to #129.