search

rtfeldman / elm-iso8601-date-strings

Convert between ISO-8601 date strings and Time.Posix
https://package.elm-lang.org/packages/rtfeldman/elm-iso8601-date-strings/latest
BSD 3-Clause "New" or "Revised" License
34 stars 21 forks source link

Does not work for datetime string without time zone designator #12

Closed simerlec closed 5 years ago

simerlec commented 5 years ago

Hello,

If I'm not mistaken 2018-09-06T00:00:00 should be a valid ISO8601 format.

I shamelessly copied and tweaked the Ellie example from @gyzerok. https://ellie-app.com/3jRZM4f34Hja1

thanks

gyzerok commented 5 years ago

I might be missing something, but according to Wikipedia it's not a valid format.

gyzerok commented 5 years ago

screenshot_2018-09-12 iso 8601 - wikipedia

simerlec commented 5 years ago

Thanks for your response!

It is described further below in the wiki article.

screenshot 2018-09-12 15 58 53

It further states that:

A single point in time can be represented by concatenating a complete date expression, the letter T as a delimiter, and a valid time expression. For example, "2007-04-05T14:30". If a time zone designator is required, it follows the combined date and time. For example, "2007-04-05T14:30Z" or "2007-04-05T12:30-02:00".

So I believe this means that the time zone designator is optional and the string 2018-09-06T00:00:00 should work.

gyzerok commented 5 years ago

Seems like I am wrong then. Would probably make sense to check standard itself to be double sure.

pdamoc commented 5 years ago

NaiveDateTime.to_iso8601 function of Elixir produces a string without Z at the end that fails with the current parser.

NaiveDateTime is the default for the timestamps in Ecto (Elixir's db wrapper)

r-k-b commented 3 years ago

2018-09-06T00:00:00 is valid ISO8601, but it's an error to turn treat it as an Absolute time.

Because it represents Local Time (the offset is ambiguous), there is not enough information to construct a Posix time.

Parsing Local Time ISO8601 values should either fail, or require a fallback timezone to be supplied.

choonkeat commented 11 months ago

as @r-k-b pointed out, it's incorrect to default UTC when time zone designator is omitted. it should be "local time"

https://github.com/rtfeldman/elm-iso8601-date-strings/blob/3587ff12251261514d0da49837f3c170d0249448/src/Iso8601.elm#L368-L370

Though yes it's ambiguous and it'll be great if folks don't generate such strings in the first place, but 3rd party apis do. It's unfortunate that elm devs will obtain a wrong Time.Posix

:. as such, perhaps it would've been better before #18 -- at least elm devs know the parsing failed and could've done something about it (like appending their local time zone designator to the string before retrying parsing)