-
Notifications
You must be signed in to change notification settings - Fork 176
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
DateTimeFormatter still lacks power user APIs #3347
Comments
Relatedly: I'm warming up to the idea that the DateTimePattern constructor should resolve the pattern parts into a |
Discuss with: Optional: |
The new API should enable the locale-specific symbols to be pre-loaded but provide a pattern at runtime. This is not dissimilar to the approach we're taking with FixedDecimalFormatter, where we push locale-invariant settings from construction time into format time. |
Tentative conclusion:
LGTM: @sffc @zbraniecki (@robertbastian) (@Manishearth) |
Additional notes on the design:
struct NeoDateTimeFormatter {
interpolator: DateTimePatternInterpolator,
patterns: ShortVec<runtime::Pattern>
}
impl NeoDateTimeFormatter {
pub fn format<'l>(&'l self, input: impl DateTimeInput) -> NeoFormattedDateTime<'l> {
// First: select the correct pattern. OK to special-case when there is only a single pattern.
self.interpolator.format_with_pattern(&pattern, input)
}
impl DateTimePatternInterpolator {
// One of these per DataKey:
pub fn load_month_symbols(&mut self, provider: &P, length: FieldLength) -> Result<&mut Self> {
// ...
}
// also include_month_symbols for the compiled_data version
pub fn load_for_pattern<'a>(&'a mut self, provider: &P, pattern: &'a runtime::Pattern) -> Result<DateTimePatternInterpolatorForPattern<'a>> {
// load the data, then pack the references into the return value
}
// compiled_data: include_for_pattern
pub fn with_pattern<'a>(&'a self, pattern: &'a runtime::Pattern) -> Result<DateTimePatternInterpolatorWithPattern<'a>> {
// returns an error if the data isn't loaded
}
pub fn format_with_pattern<'l>(&'l self, pattern: &'l runtime::Pattern, input: impl DateTimeInput) -> NeoFormattedDateTime<'l> { ... }
}
struct DateTimePatternInterpolatorWithPattern<'a> {
pattern: &'a runtime::Pattern,
interpolator: DateTimePatternInterpolatorBorrowed<'a>,
}
impl<'a> DateTimePatternInterpolatorWithPattern<'a> {
pub fn format<'l>(&'l self, input: impl DateTimeInput) -> NeoFormattedDateTime<'l> {
// pack the additional references into NeoFormattedDateTime
}
} The call site would look like this: // Option 1: Use the same interpolator for multiple patterns (always in the same locale and calendar system)
let mut interpolator = DateTimePatternInterpolator::<Gregorian>::new(&locale!("en").into());
interpolator.load_with_pattern(provider, "M/d/y".parse().unwrap())
.unwrap()
.format("2023-10-25".parse().unwrap())
// this won't fail if using an ICU4X impl of DateTimeInput:
.write_to_string();
interpolator.load_with_pattern(provider, "MMM d, y".parse().unwrap())
.unwrap()
.format("2023-10-25".parse().unwrap())
.write_to_string();
// Option 2: All in a single line (less cached data)
DateTimePatternInterpolator::<Gregorian>::new(&locale!("en").into())
.load_with_pattern(provider, "MMM d, y".parse().unwrap())
.unwrap()
.format("2023-10-25".parse().unwrap())
.write_to_string();
// Option 3: Without the intermediate type:
let mut interpolator = DateTimePatternInterpolator::<Gregorian>::new(&locale!("en").into());
let pattern = "MMM d, y".parse().unwrap();
interpolator.load_with_pattern(provider, &pattern).unwrap();
// Here we don't have a place to keep the invariant that we have loaded the correct symbols, so this needs to be fallible (with a core::fmt::Error):
interpolator.format_with_pattern(&pattern, "2023-10-25".parse().unwrap()); Any additional feedback before I implement this? |
|
Naming bikeshed for DateTimePatternInterpolator:
Will temporarily go with:
LGTM: (@sffc) (@Manishearth) |
My PR #4204 uses the name Some other ideas:
|
Also we can add helper functions on |
In #380 and the corresponding design doc, I suggested that DateTimeFormatter have multiple layers:
It seems this got dropped and not resolved in that older issue.
The text was updated successfully, but these errors were encountered: