Add chrono and jiff integrations#7617
Add chrono and jiff integrations#7617robertbastian wants to merge 8 commits intounicode-org:mainfrom
chrono and jiff integrations#7617Conversation
c8fb314 to
41777a7
Compare
41777a7 to
e08545d
Compare
| use crate::Date; | ||
| use crate::Gregorian; | ||
|
|
||
| impl From<chrono::NaiveDate> for Date<Gregorian> { |
There was a problem hiding this comment.
Issue: I'm not super happy using Date<Gregorian> instead of Date<Iso> as the canonical representation in icu4x.
The main value proposition of the icu4x calendar crate relative to chrono/jiff/time is our ability to represent different calendar systems.
Date<Iso> means "a date that has not yet been converted into a human-readable formattable calendar". If people want to format that as Gregorian, make them explicitly do the conversion.* That is the intended design: the formatted calendar type should be explicit.
In other words, I want to prevent people from creating a FixedCalendarDateTimeFormatter that formats with Gregorian without ever having the word "gregorian" written in their code.
Another footgun, which is admittedly narrow, is that DateTimeFormatter::format_same_calendar will succeed for most locales by default but fail for the non-gregorian locales. Although I guess since you aren't implementing the InSameCalendar trait, users won't bit this footgun.
* I think I remember approving a PR a few months ago that made that conversion free, i.e. not going through RD.
There was a problem hiding this comment.
Both chrono and jiff understand themselves to be implementing the Gregorian calendar. The "ISO calendar" is a weird Temporal thing, it's not some magic interchange format.
There was a problem hiding this comment.
@sffc I agree. The Jiff docs could be improved on this point, and it's probably just down to a mixture of mild sloppiness and end user familiarity that it says "Gregorian calendar" everywhere. But AFAIK, Jiff does actually implement the ISO 8601 proleptic calendar because it has a year 0. Similarly for chrono.
There was a problem hiding this comment.
Our Gregorian calendar uses a year 0 in both its constructor as well as its internal representation. The whole BC/CE thing (which is the only difference between Gregorian and Iso) is a human-display difference. The reason why we want to integrate jiff with icu is for human display, so it makes sense to use the Calendar which is suitable for that, not the Calendar which implements a technical interchange format.
c6d4986 to
9168779
Compare
|
Slight worry that can probably be resolved with some research: Do we know the MSRV policy of these packages and their deps? So far we have managed to limit the number of larger deps in Fortunately, it seems like these are small deptrees, so it's probably not a big deal. I suspect anything written by Andrew is sufficiently MSRV-conservative for us. |
9168779 to
06cb599
Compare
|
|
|
@Manishearth Speaking for Jiff, what policy I do have is documented here: https://github.com/BurntSushi/jiff?tab=readme-ov-file#minimum-rust-version-policy --- Which is basically the same as I can't imagine a scenario in which I'll bump to anything newer than N-9 (i.e., 1 year old Rust). I should probably just add that to the policy so that folks can rely on it. OK, I put up a PR for that: BurntSushi/jiff#512 (I did carve out a possible exception, although it seems unlikely to happen? It kind of depends on, e.g., how crates like In practice, I suspect I'll never bump to anything newer than what's in Debian stable. |
|
N-9 is much wider than our window, so that works! |
This adds
chronoandjifffeatures toicu_datetime, which allows using those crate's types as inputs toDateTimeFormatter.It also adds
chronoandjifffeatures toicu_timeandicu_calendarthat allow converting toicutypes.