Add support for RFC 3339 style time #248
Conversation
|
@ericgpks Do you want to review this? |
|
I would like to try review. |
lib/csv.rb
Outdated
| \d{4}-\d{2}-\d{2} | ||
| (?:T\d{2}:\d{2}(?::\d{2}(?:\.\d+)?(?:[+-]\d{2}(?::\d{2})|Z)?)?)? | ||
| (?:[T ]\d{2}:\d{2}(?::\d{2}(?:\.\d+)?(?:[+-]\d{2}(?::\d{2})|Z)?)?)? |
There was a problem hiding this comment.
I don't know if this meets a condition mentioned in this comment
There was a problem hiding this comment.
@thyresias It seems that this approach is the latter approach not the former approach. Is it intentional?
Anyway, could you refer RFC 3339 instead of "near-ISO" for "YYYY-MM-DD hh:mm:ss" format?
This rejects "YYYY-MM-DD\thh:mm:dd" that is accepted without this change. Could you use [T\s] instead of [T ]?
test/csv/test_data_converters.rb
Outdated
| @@ -103,4 +103,19 @@ def test_builtin_date_time_converter_iso8601_utc | |||
| assert_equal(datetime, | |||
| CSV::Converters[:date_time][iso8601_string]) | |||
| end | |||
|
|
|||
| def test_builtin_date_time_converter_near_iso | |||
There was a problem hiding this comment.
Could you split this test to multiple tests like existing test_bulitin_date_time_converter_iso8601_* for easy to debug on failure?
If "2018-01-14 22:25" is failed, the rest cases such as "2018-01-14 22:25:19" aren't executed by this style. The results of all cases are helpful to debug.
* support any whitespace instead of T * split tests for space, add tests for tab
|
@kou |
test/csv/test_data_converters.rb
Outdated
| @@ -103,4 +103,89 @@ def test_builtin_date_time_converter_iso8601_utc | |||
| assert_equal(datetime, | |||
| CSV::Converters[:date_time][iso8601_string]) | |||
| end | |||
|
|
|||
| def test_builtin_date_time_converter_rfc3339_minute | |||
| iso8601_string = "2018-01-14 22:25" | |||
There was a problem hiding this comment.
Could you use rfc3339_string instead of iso8601_string because this is not ISO 8601 format?
kou
changed the title
|
Thanks! |
(ruby/csv#248) fix ruby/csv#247 ISO 8601 uses "T" for separator of date and time. RFC 3339 also accepts a space for separator of date and time. ruby/csv@1255d8c304
fix #247
ISO 8601 uses "T" for separator of date and time.
RFC 3339 also accepts a space for separator of date and time.