DurationFormatter: Implement DurationFormatter #5321
Conversation
and add testdata
add documentation to hour and minute format
previously using incorrect function to set maximum fractional digits
| struct PartSink { | ||
| string: String, | ||
| parts: Vec<(usize, usize, writeable::Part)>, | ||
| } | ||
|
|
There was a problem hiding this comment.
PartSink is very similar to the TestSink the writeable library internally uses. I create it here to add part information to ListFormatter-emitted parts. (changing parts::Element to parts::HOUR, etc.)
It would be nice if ListFormatter could optionally preserve part annotations for the Writeables it formats. I'd be happy to try to work on that if that sounds useful.
There was a problem hiding this comment.
Good point, @sffc , what do you think ?
There was a problem hiding this comment.
You shouldn't ever need to allocate anything, including into a Vec or String, when formatting in ICU4X. If we can do it in datetime format, which is an order of magnitude more complex than any other ICU4X formatter right now, we can do it here in DurationFormatter. :)
Please note that ListFormatter is already wired up to allow you to do this. The ListFormatter invokes the write_to_parts function of the writeables passed to its format function. You should make sure those writeables add the unit part. You'll get the ELEMENT and FIELD parts from icu_list, but that's fine.
| pub(crate) fn as_fixed_decimal_sign(&self) -> fixed_decimal::Sign { | ||
| match self { | ||
| DurationSign::Positive => fixed_decimal::Sign::Positive, | ||
| impl From<DurationSign> for fixed_decimal::Sign { |
| pub(crate) unit: DurationUnitFormatter, | ||
| pub(crate) list: ListFormatter, | ||
| pub(crate) fdf: FixedDecimalFormatter, | ||
| } | ||
|
|
||
| pub(crate) struct DurationUnitFormatter { | ||
| pub(crate) year: UnitsFormatter, | ||
| pub(crate) month: UnitsFormatter, | ||
| pub(crate) week: UnitsFormatter, | ||
| pub(crate) day: UnitsFormatter, | ||
| pub(crate) hour: UnitsFormatter, | ||
| pub(crate) minute: UnitsFormatter, | ||
| pub(crate) second: UnitsFormatter, | ||
| pub(crate) millisecond: UnitsFormatter, | ||
| pub(crate) microsecond: UnitsFormatter, | ||
| pub(crate) nanosecond: UnitsFormatter, | ||
| } |
There was a problem hiding this comment.
This is need to be only one units formatter
| pub(crate) unit: DurationUnitFormatter, | ||
| pub(crate) list: ListFormatter, |
There was a problem hiding this comment.
units_formatters: DurationUnitFormatters
| pub struct DurationFormatter { | ||
| /// Options for configuring the formatter. | ||
| pub(crate) options: ValidatedDurationFormatterOptions, | ||
| pub(crate) digital: DataPayload<provider::DigitalDurationDataV1Marker>, | ||
| pub(crate) unit: DurationUnitFormatter, | ||
| pub(crate) list: ListFormatter, |
| pub(crate) fdf: FixedDecimalFormatter, | ||
| } | ||
|
|
||
| pub(crate) struct DurationUnitFormatter { |
There was a problem hiding this comment.
/// Contains all the needed formatters for formatting the duration units
DurationUnitFormatters
follow up to #5263
will add documentation, benchmarks, and more tests in separate PRs