-
Notifications
You must be signed in to change notification settings - Fork 185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvements to field set APIs #6022
Comments
How about this
We got lucky that Now BCP-47 ID and IANA ID remain. Those will need two different field sets, because the IANA ID will need more data from somewhere (either loading an The BCP-47 ID and ISO formats actually have the same bounds, they don't need data and work with a Footnotes
|
We don't necessarily need to add every style to the field sets, at least not right away. We should prioritize the ones that make sense when combined with datetime. Users who want full access to time zone names can use DateTimePattern and bypass the field sets. So, for example, the city style probably doesn't need a field set, because "19:00 Paris" doesn't make sense (should be at least "19:00 Paris Time"), and I also left out the ISO format because the Localized Offset format is just better. |
The fieldsets are not just for combining with a time, they can be used standalone. It's a much nicer API than going through icu4x/components/datetime/examples/timezone_picker.rs Lines 19 to 20 in 3cad93c
Also, "19:00 Paris" makes sense, that's why the |
Field sets are supposed to make it easy to do the right thing. Using VVV instead of VVVV is usually the wrong thing. If you want a Google Calendar style time zone picker, DateTimePattern is not a bad option, I think. But if you can design a semantic datetime option that does VVV only when it is "right", and otherwise uses VVVV, I'm all for it. |
As suggested in #6029 (comment): I would support replacing Thoughts? @robertbastian @Manishearth |
I'm in support of clearer names. I don't really have opinions on what those names should be , but it seems like y'all have some clear ideas. |
Proposal: // Things we currently have, but exported with new names:
icu::datetime::fieldsets::zone::ExemplarCity
icu::datetime::fieldsets::zone::GenericShort
icu::datetime::fieldsets::zone::GenericLong
icu::datetime::fieldsets::zone::SpecificShort
icu::datetime::fieldsets::zone::SpecificLong
icu::datetime::fieldsets::zone::LocalizedOffsetShort
icu::datetime::fieldsets::zone::LocalizedOffsetLong
(icu::datetime::fieldsets::zone::LocalizedOffsetFixed)
icu::datetime::fieldsets::zone::Location
// We might add other styles when people ask for them.
// These are less human readable, so the use cases as field sets are not obvious
icu::datetime::fieldsets::zone::IanaId
icu::datetime::fieldsets::zone::Bcp47Id
icu::datetime::fieldsets::zone::Iso
// Conversion functions
// currently: YMDT::with_zone_generic(self) -> Combo<YMDT, GenericShort>
// Note: the bound is not _necessary_ but it is useful for documentation purposes
YMDT::with_zone<Z: ZoneMarkers>(self, Z) -> Combo<YMDT, Z>
fieldsets::YMDT::short().zone(fieldsets::zone::SpecificLong)
// Field set expressions, removing the "with":
fieldsets::T::short().hm()
fieldsets::T::short().hms()
fieldsets::T::short().precision(TimePrecision::OptionalMinute)
fieldsets::YMD::short().time(TimePrecision::OptionalMinute).zone(fieldsets::zone::SpecificLong)
fieldsets::YMD::short().hm().zone(fieldsets::zone::SpecificLong)
fieldsets::YMD::short().hms().zone(fieldsets::zone::SpecificLong)
// Consider replacing YMDT with `Combo<YMD, T>`. Also implies `Combo<Combo<YMD, T>, SpecificLong>`
// Scratch pad of other concepts:
fieldsets::YMD::short().time(fieldsets::T::short().hm().with_alignment(...)).zone(fieldsets::zone::SpecificLong)
fieldsets::YMD::with(fieldsets::T.hm()).long().alignment(Alignment::Standard).with(fieldsets::zone::SpecificLong)
fielsets::YMD.long().with(fielsets::zone::SpecificLong) |
Originally posted by @sffc in #6018 (comment)
The text was updated successfully, but these errors were encountered: