Skip to content

Add CSV::InvalidEncodingError - #287

Merged
kou merged 1 commit into
ruby:masterfrom
shibaaaa:add-csv-invalid-encoding-error
Sep 14, 2023
Merged

Add CSV::InvalidEncodingError#287
kou merged 1 commit into
ruby:masterfrom
shibaaaa:add-csv-invalid-encoding-error

Conversation

@shibaaaa

Copy link
Copy Markdown
Contributor

To handle encoding errors in CSV parsing with the appropriate error class

kou
kou approved these changes Sep 12, 2023
Comment threadlib/csv.rb

# The error thrown when the parser encounters invalid encoding in CSV.
class InvalidEncodingError < MalformedCSVError
def initialize(encoding, line_number)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about adding a reader for the target encoding?

Suggested change
definitialize(encoding,line_number)
attr_reader:encoding
definitialize(encoding,line_number)
@encoding=encoding

@koukouSep 13, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can't get the target encoding if we don't set it to @encoding. (See the above suggestion.)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

oops!

@shibaaaa
shibaaaaforce-pushed the add-csv-invalid-encoding-error branch from cb0a680 to f2369dbCompareSeptember 13, 2023 09:03
@shibaaaa
shibaaaa requested a review from kouSeptember 13, 2023 09:04
Comment threadlib/csv.rb

# The error thrown when the parser encounters invalid encoding in CSV.
class InvalidEncodingError < MalformedCSVError
def initialize(encoding, line_number)

@koukouSep 13, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can't get the target encoding if we don't set it to @encoding. (See the above suggestion.)

end
CSV.open(@input.path, encoding: "EUC-JP") do |csv|
error = assert_raise(CSV::MalformedCSVError) do
error = assert_raise(CSV::InvalidEncodingError) do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you also change the below assert_equal(...) to the following?

assert_equal([Encoding::EUC_JP,"Invalid byte sequence in EUC-JP in line 1."],[error.encoding,error.message])

csv = CSV.new("valid,x\rinvalid,\xF8\r".force_encoding("UTF-8"),
encoding: "UTF-8", row_sep: "\r")
error = assert_raise(CSV::MalformedCSVError) do
error = assert_raise(CSV::InvalidEncodingError) do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto.

To handle encoding errors in CSV parsing with the appropriate error class
@shibaaaa
shibaaaaforce-pushed the add-csv-invalid-encoding-error branch from f2369db to 0aa7d2fCompareSeptember 13, 2023 22:18
@shibaaaa
shibaaaa requested a review from kouSeptember 13, 2023 22:18
@kou
kou merged commit 68b4488 into ruby:masterSep 14, 2023
@kou

kou commented Sep 14, 2023

Copy link
Copy Markdown
Member

Thanks!

@kou

kou commented Sep 14, 2023

Copy link
Copy Markdown
Member

Do you want a new version soon?

@shibaaaa

Copy link
Copy Markdown
ContributorAuthor

@kou

Do you want a new version soon?

No I don't, thanks.

@shibaaaa
shibaaaa deleted the add-csv-invalid-encoding-error branch September 14, 2023 01:06
matzbot pushed a commit to ruby/ruby that referenced this pull request Oct 3, 2023
(ruby/csv#287)
To handle encoding errors in CSV parsing with the appropriate error
class
ruby/csv@68b44887e5
Sign up for freeto 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

@shibaaaa@kou