Implement Short Compact Currency Formatter Provider and Populate Associated Data #5361
Conversation
younies
requested review from
robertbastian,
Manishearth and
a team
as code owners
younies
changed the title
| /// `"1000-count-one-alt-alphaNextToNumber": "¤ 0K"` | ||
| /// -> key1 = 3, key2 = CompactCount::OneAlt, value = "¤ 0K" | ||
| #[cfg_attr(feature = "serde", serde(borrow))] | ||
| pub compact_patterns: ZeroMap2d<'data, i8, CompactCount, str>, |
There was a problem hiding this comment.
unless you need to do the lookup in two steps, prefer ZeroMap<(i8, CompactCount), str> over ZeroMap2d, as the 2d version has bigger metadata.
| fn try_from(value: &str) -> Result<Self, Self::Error> { | ||
| match value { | ||
| "zero" => Ok(CompactCount::Zero), | ||
| "zero-alt-alphaNextToNumber" => Ok(CompactCount::ZeroAlt), |
There was a problem hiding this comment.
this is highly CLDR-JSON specific and should live in icu_provider_source.
| /// CompactPattern `zero`. | ||
| Zero = 0, | ||
| /// Compact Pattern `zero` alternative. | ||
| ZeroAlt = 1, |
There was a problem hiding this comment.
how will the Alt data be accessed? Consider putting them behind a locale variant instead (i.e. en-gb-alt), instead of modelling them in the same data struct
There was a problem hiding this comment.
done, we made it AlphaNextToNumber
| @@ -33,11 +33,9 @@ pub struct ShortCurrencyCompactV1<'data> { | |||
| /// `"1000-count-one-alt-alphaNextToNumber": "¤ 0K"` | |||
| /// -> key1 = 3, key2 = CompactCount::OneAlt, value = "¤ 0K" | |||
| #[cfg_attr(feature = "serde", serde(borrow))] | |||
| pub compact_patterns: ZeroMap2d<'data, i8, CompactCount, str>, | |||
| pub compact_patterns: ZeroMap<'data, (i8, CompactCount), str>, | |||
There was a problem hiding this comment.
you have 4 bits left in CompactCountULE, what's the range of the log10?
There was a problem hiding this comment.
there is no limit for log10, but I think to put the limit 16 is kinda a low limit
There was a problem hiding this comment.
yeah it'd be -8 to 7 as well.
| @@ -46,56 +44,35 @@ pub struct ShortCurrencyCompactV1<'data> { | |||
| databake(path = icu_experimental::dimension::provider::currency_compact) | |||
| )] | |||
| #[repr(u8)] | |||
| pub enum CompactCount { | |||
| pub enum Count { | |||
There was a problem hiding this comment.
can you use an existing Count? This type exists at least twice already.
| @@ -33,11 +33,9 @@ pub struct ShortCurrencyCompactV1<'data> { | |||
| /// `"1000-count-one-alt-alphaNextToNumber": "¤ 0K"` | |||
| /// -> key1 = 3, key2 = CompactCount::OneAlt, value = "¤ 0K" | |||
| #[cfg_attr(feature = "serde", serde(borrow))] | |||
| pub compact_patterns: ZeroMap2d<'data, i8, CompactCount, str>, | |||
| pub compact_patterns: ZeroMap<'data, (i8, CompactCount), str>, | |||
There was a problem hiding this comment.
yeah it'd be -8 to 7 as well.
| @@ -46,56 +44,35 @@ pub struct ShortCurrencyCompactV1<'data> { | |||
| databake(path = icu_experimental::dimension::provider::currency_compact) | |||
| )] | |||
| #[repr(u8)] | |||
| pub enum CompactCount { | |||
| pub enum Count { | |||
Tasks
In this PR, the following tasks are accomplished:
numbers.json.ShortCompactCurrencyV1struct.bakeddataandtestdataforShortCompactCurrencyV1Marker.