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 DateTimeFormatter field set getter? #6243

Open
sffc opened this issue Mar 6, 2025 · 0 comments
Open

Add DateTimeFormatter field set getter? #6243

sffc opened this issue Mar 6, 2025 · 0 comments
Labels
C-datetime Component: datetime, calendars, time zones

Comments

@sffc
Copy link
Member

sffc commented Mar 6, 2025

In order to implement the version of the DateTime FFI described in #5940 (comment), I need to add a field set getter on DateTimeFormatter.

Most of the information required to implement this getter is already retained in the DateTimeFormatter: there is a RawOptions field in most of the variants of DateTimeZonePatternSelectionData.

However, we don't currently retain the FSet itself. This is primarily an issue for dynamic field sets; for example, one cannot reconstruct a DateFieldSet::YMD from a DateTimeFormatter<DateFieldSet> that was constructed with a YMD field set.

A couple of directions here:

  1. (probably what I will end up doing) Save the date fields in the DateTimeFormatter data model somewhere, maybe adding them to the RawOptions struct. I will cross my fingers that there are enough niches that maybe this doesn't have any cost.
  2. (theoretically possible) Infer the date fields from the pattern data payload.
  3. (seems unnecessarily complex and has side-effects) Add the date fields to the body of the data struct, costing ~1 byte per payload.

I think the easiest would be for the function to return a CompositeFieldSet. I could also work to make it return FSet.

This issue might also include doing something I've wanted to do for a while: flatten the internal DateTimeZonePatternSelectionData enum into a struct, which will reduce code complexity without adversely affecting anything I can think of.

@sffc sffc added the C-datetime Component: datetime, calendars, time zones label Mar 6, 2025
@sffc sffc added this to the ICU4X 2.0 ⟨P1⟩ milestone Mar 6, 2025
sffc added a commit that referenced this issue Mar 6, 2025
Organizes the inner workings without impacting stack size. I've wanted
to do this for a while and #6243 was the impetus.
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
Projects
None yet
Development

No branches or pull requests

1 participant