Implement parsing Instant with offset #107
Conversation
dkhalanskyjb
force-pushed
the
instant-parse-with-offset
branch
3 times, most recently
from
a2539dc to
c35a0b8
Compare
core/common/test/InstantTest.kt
Outdated
| Triple("2020-01-01T00:01:01.123456789-17:59:59", 1577901660L, 123456789), | ||
| Triple("2020-01-01T00:01:01.010203040+17:59:59", 1577772062L, 10203040), | ||
| Triple("2020-01-01T00:01:01.010203040+17:59", 1577772121L, 10203040), | ||
| // Triple("2020-01-01T00:01:01+00", 1577836861L, 0), // fails on JS, passes everywhere else |
There was a problem hiding this comment.
Improvement proposal: it's hard to correlate the expected epochSecond values with the source strings. Perhaps better use a pair of values as input: the value with an offset and the corresponding value with Z, and compare results of their parsing for equality.
There was a problem hiding this comment.
Yeah, this is the proper way to write this test. Fixed.
core/common/src/Instant.kt
Outdated
| * In addition to allowing the `Z` designator of the UTC+0 time zone to be passed as the time zone offset, which | ||
| * is required by the ISO-8601, this parser also accepts the other possible offsets, but does not provide a way | ||
| * to query the result for which offset was specified in the string. |
There was a problem hiding this comment.
I propose to rephrase this simpler, something like:
Supports the following ways of specifying the time zone offset:
Zdesignator for the UTC+0 time zone,- a custom time zone offset specified with
+hh, or+hh:mm, or+hh:mm:ss, where negative offsets are prefixed with-instead of+.
There was a problem hiding this comment.
Agreed, I was clever by half there. However, now that we are so explicit about the accepted offsets, we can't just ignore that parsing +hh fails on JS, so I added a workaround for that.
There was a problem hiding this comment.
Turns out that JDK8 also fails to parse +hh, as evidenced by the build failing. Added a workaround for that, too.
dkhalanskyjb
force-pushed
the
instant-parse-with-offset
branch
from
c35a0b8 to
3e11ed8
Compare
dkhalanskyjb
force-pushed
the
instant-parse-with-offset
branch
from
afa1678 to
3cf0fa5
Compare
Fixes #56