Support basic datetime format in Calendar.ISO parsing functions

[christhekeele]

This PR adds support for an optional second parameter to Calendar.ISO parsing functions, allowing them to parse the :basic format as well as the (default) :extended.

[josevalim]

Hi @christhekeele, can we please go only with the :basic option without :any? With with you can easily try both or even other formats. :) Thank you.

[christhekeele]

@josevalim :any removed! 🙂

[christhekeele]

Should I add something to the changelog about this within this PR, or is that handled during release?

[josevalim]

Thanks for asking, changelogs are handled later on, to avoid conflicts!

[wojtekmach]

I'm not sure if this was already mentioned, but this is a nice way of solving [Proposal] Add ability to parse ISO8601 "basic" format. In that thread, it wasn't clear how the API would look like, ideally it would be option 1 & 2 which are ambiguous.

Date.from_iso8601(string, format)
Date.from_iso8601(string, calendar)
Date.from_iso8601(string, calendar, format)

Well, it's Calendar.ISO.parse_date(string, format) which is unambiguous!

[josevalim]

@wojtekmach don't celebrate too much because next step is to figure out a why to surface that upstream. :D But I think we can look at the second argument and decide accordingly. So it will be:

Date.from_iso8601(string, format_or_calendar)
Date.from_iso8601(string, format, calendar)

[christhekeele]

Yeah, the Date.from_iso8601 function signatures are a little funky.

I've always been a little confused about the use case of the signature of Date.from_iso8601(string, calendar). It seems to me that iso8601 specifies the calendar already, and basic/extended formats are specifically defined only within the context of that calendar.

So Date.from_iso8601(string, format) makes sense to me, and Date.from_string(string, calendar \\ Calendar.ISO), but not really the whole thing in one.

[AndrewDryga]

As a side topic would RFC1123 datetime format ("D, j M Y G:i:s O" ex. Wed, 21 Oct 2015 19:19:53 +0000) decoder would be accepted to Elixir? There is at least one large service (Twilio) that uses it and parsing it is a pain.

[christhekeele]

@AndrewDryga I think it's worth bringing up on the core mailing list--this PR is mostly for parsing/formatting parity--but my suspicion is that that's better suited for an external library.

My way of thinking about this is that the core Calendar protocol is really about providing a way to work with dates and times correctly, and since we need some means of conveying data in and out of that format, the default calendar supports some standard ISO interfaces to constructing them. But the parsing stuff takes a backseat and is more focused on being fast than flexible, since it must support the sigil literals for the language.

[josevalim]

Negative dates are only allowed in the extended format, so we should handle the +/- within :extended. :)

[christhekeele]

Good point!

[christhekeele]

I just revised the spec on this--there are two extended formats in play.

One is the general "extended" (as opposed to "basic") format--ie with separators or not. That's what the :basic and :extended flags here entail.

The other we have decided to support, to work with a wider range of dates, is the "year extension". But our support for that is independent of basic vs extended separator format, and should work with both.

TL;DR I believe this is correct as writ

[josevalim]

You are correct. I confused expanded with extended. :)

[josevalim]

It looks great! I have added only two comments but they will also apply for naive dt stuff too!

[josevalim]

💚 💙 💜 💛 ❤️