Skip to content
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

Bikeshed names of time zone field sets #6022

Open
sffc opened this issue Jan 21, 2025 · 5 comments
Open

Bikeshed names of time zone field sets #6022

sffc opened this issue Jan 21, 2025 · 5 comments
Labels
2.0-breaking Changes that are breaking API changes discuss Discuss at a future ICU4X-SC meeting needs-approval One or more stakeholders need to approve proposal

Comments

@sffc
Copy link
Member

sffc commented Jan 21, 2025

          We should bikeshed the name of this.

Originally posted by @sffc in #6018 (comment)

@sffc sffc added the 2.0-breaking Changes that are breaking API changes label Jan 21, 2025
@sffc sffc added this to the ICU4X 2.0 ⟨P1⟩ milestone Jan 21, 2025
@sffc sffc added the discuss Discuss at a future ICU4X-SC meeting label Jan 21, 2025
@robertbastian
Copy link
Member

How about this

Format UTS-35 symbol ICU fieldset
Specific location z..zzzz Z, Zs
Generic location v..vvvv V, Vs
Offset O..OOOO O 1
Location VVVV L
Exemplar city VVV C
IANA ID VV not implemented
BCP-47 ID V ?
ISO X..XXXXX X
aliases Z..ZZZZZ -

We got lucky that Z is used for aliases only, so we can use it instead of z. However, using V instead of v means we need to find different fieldsets for all the V symbols. L is the standalone month in UTS-35, and lowercase l is deprecated, which I guess frees it up, C is an "input skeleton symbol" and c is a standalone day-of-week, which I don't think we need either.

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 IanaMapper, or requiring a TimeZoneInfo<WithIana>).

The BCP-47 ID and ISO formats actually have the same bounds, they don't need data and work with a Base timezone info, so we could put them in one field set and discriminate by length, but I'd rather not do that.

Footnotes

  1. There's no data diff between O and Os, so I don't think we need Os.

@sffc
Copy link
Member Author

sffc commented Jan 23, 2025

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.

@robertbastian
Copy link
Member

The fieldsets are not just for combining with a time, they can be used standalone. It's a much nicer API than going through DateTimePattern:

let non_location_formatter = DateTimeFormatter::try_new(prefs, fieldsets::V::new()).unwrap();
let city_formatter = DateTimeFormatter::try_new(prefs, fieldsets::X::new()).unwrap();

Also, "19:00 Paris" makes sense, that's why the VVV pattern exists. The ISO format also makes sense in certain situations. I don't understand why we'd hide this functionality.

@sffc
Copy link
Member Author

sffc commented Jan 31, 2025

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.

@sffc sffc added the needs-approval One or more stakeholders need to approve proposal label Feb 4, 2025
@sffc
Copy link
Member Author

sffc commented Feb 4, 2025

As suggested in #6029 (comment):

I would support replacing fieldsets::V and friends with fieldsets::zone::GenericNonLocationLong.

Thoughts? @robertbastian @Manishearth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0-breaking Changes that are breaking API changes discuss Discuss at a future ICU4X-SC meeting needs-approval One or more stakeholders need to approve proposal
Projects
None yet
Development

No branches or pull requests

2 participants