Implement LongCurrencyFormatter for Long Currency Formatting
#5351
Conversation
younies
changed the title
LongCurrencyFormatterLongCurrencyFormatter for Long Currency Formatting
| let pattern = patterns | ||
| .get(&pattern_count) | ||
| .or_else(|| patterns.get(&PatternCount::Other)) | ||
| .ok_or(core::fmt::Error)?; |
There was a problem hiding this comment.
don't ever produce core::fmt::Error: https://doc.rust-lang.org/std/fmt/index.html#:~:text=Additionally%2C%20the%20return,up%20the%20stack.
There was a problem hiding this comment.
mmmmm
in this case we have two options,
- to not return error at all and use default pattern
- move this logic to
format_fixed_decimalmethod
I prefer option 2.
There was a problem hiding this comment.
So PatternCount::Other should always exist, so it would make sense to not have it in the map, but put it as a required field on the struct. You'd have
other: Pattern,
addtional_cases: ZeroMap<PatternCount, Pattern>,There was a problem hiding this comment.
Actually, other must be present. I added this condition to the datagen`
There was a problem hiding this comment.
I'd prefer if this was represented in the type system, instead of with a trust-me-unwrap.
There was a problem hiding this comment.
good point: #5374 (comment)
components/experimental/src/dimension/provider/currency_patterns.rs
Outdated
Show resolved
Hide resolved
… as reference" This reverts commit 9ac101e.
… Writeable implementation
| currency/patterns@1, blo, 82B, da128faf13fad0d5 | ||
| currency/patterns@1, ceb, 72B, db7c7c84fecaf3d0 | ||
| currency/patterns@1, es-GT, 72B, -> ceb | ||
| currency/patterns@1, ja, 61B, 9af35aad6ebc7d70 | ||
| currency/patterns@1, my, 62B, 86cee97085536d8b | ||
| currency/patterns@1, ro, 85B, 629b2b7dcea97875 | ||
| currency/patterns@1, sd, 48B, 3a6d9523170345d0 |
There was a problem hiding this comment.
this seems to be a CLDR bug (missing Other), can you file an issue?
| let pattern = patterns | ||
| .get(&pattern_count) | ||
| .or_else(|| patterns.get(&PatternCount::Other)) | ||
| .ok_or(core::fmt::Error)?; |
There was a problem hiding this comment.
I'd prefer if this was represented in the type system, instead of with a trust-me-unwrap.
| let plural_category = self.plural_rules.category_for(self.value); | ||
| let display_name = self.extended.display_names.get_str(plural_category); | ||
| let pattern = self.patterns.patterns.get_pattern(plural_category); | ||
| let formatted_value = self.fixed_decimal_formatter.format(self.value); | ||
| let interpolated = pattern.interpolate((formatted_value, display_name)); | ||
| interpolated.write_to(sink) |
There was a problem hiding this comment.
Praise: good code here, just loading things and interpolating them. No allocations.
Task:
In this PR, implement
LongCurrencyFormatterwhich handles formatting currencies in long formats.Related issues:
#5353