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

Add a ZonedTime type #5937

Open
sffc opened this issue Dec 20, 2024 · 1 comment
Open

Add a ZonedTime type #5937

sffc opened this issue Dec 20, 2024 · 1 comment
Labels
C-datetime Component: datetime, calendars, time zones C-time-zone Component: Time Zones

Comments

@sffc
Copy link
Member

sffc commented Dec 20, 2024

It can live next to ZonedDateTime.

Why: in order to use TimeFormatter to format a time and a zone together.

Other solutions considered but infeasible:

  1. Use ZonedDateTime: no, it has a calendar, but TimeFormatter has the () calendar
  2. Implement InFixedCalendar<()> for ZonedDateTime: no, this breaks type inference for everyone else
  3. Add format_any_calendar to TimeFormatter: possible, but seems confusing; ZonedTime is better

@Manishearth @robertbastian

@sffc sffc added C-datetime Component: datetime, calendars, time zones C-time-zone Component: Time Zones labels Dec 20, 2024
@sffc sffc added this to the ICU4X 2.0 Stretch ⟨P2⟩ milestone Dec 20, 2024
@Manishearth
Copy link
Member

no, this breaks type inference for everyone else

Important to note, this breaks type inference in an intractable way. A property we currently have is that let dtf = FixedCalendarDateTimeFormatter::new(YMD); dtf.format(datetime) automatically infers the correct calendar. There's no way to make FCDTF<(), ..> not care about calendars without having the above code also be valid for the no-calendar time case.

In general I consider passing e.g. a DateTime to a TimeFormatter to be a bit of an antipattern, it's nice that we support it and there are likely ways it will be convenient, but I don't want to encourage that. ZonedTime seems like a good way to avoid such constructs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-datetime Component: datetime, calendars, time zones C-time-zone Component: Time Zones
Projects
None yet
Development

No branches or pull requests

2 participants