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

Replace PluralElementsV1 with PluralElementsPackedULE #5510

Merged
merged 39 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
1f151e0
Add TinyVarVar
sffc Sep 8, 2024
79582ad
Fix typo bug on main
sffc Sep 8, 2024
0f7223b
Add PluralElementsPackedULE and use it in relativetime
sffc Sep 8, 2024
1ccd8e0
Make PluralElementsPackedULE not depend on TinyVarVarULE
sffc Sep 9, 2024
0be2b08
Revert "Add TinyVarVar"
sffc Sep 9, 2024
dae6881
Clippy: create more intermediate types
sffc Sep 9, 2024
121339f
More refactoring
sffc Sep 9, 2024
e2d3fdd
features
sffc Sep 9, 2024
0dfe79c
clippy
sffc Sep 9, 2024
38c778b
cargo make testdata
sffc Sep 9, 2024
9f21225
cargo make bakeddata experimental
sffc Sep 9, 2024
6712c93
Fix baked safety comment
sffc Sep 9, 2024
e8faf2f
cargo make bakeddata experimental
sffc Sep 9, 2024
922b1a7
Clean up PluralCategoryAndMetadata
sffc Sep 9, 2024
0570498
More safety comments
sffc Sep 10, 2024
5ec4aac
fmt
sffc Sep 10, 2024
964562d
Try to write deserialize_with fn
sffc Sep 10, 2024
3a99f60
feature gate
Manishearth Sep 10, 2024
7e6cb9d
superfluous bound
Manishearth Sep 10, 2024
6e429a1
lol this works?????
Manishearth Sep 10, 2024
7043da7
link to rustc bug
Manishearth Sep 10, 2024
fdd273e
deserialize_with in relativetime
sffc Sep 10, 2024
fa70e60
Review feedback
sffc Sep 10, 2024
9b30c63
rm PluralElementsV1; replace in currency formatter
sffc Sep 10, 2024
38f6676
cargo make testdata
sffc Sep 10, 2024
ba45af4
cargo make bakeddata experimental
sffc Sep 10, 2024
effaef0
get_specials_tuples
sffc Sep 10, 2024
687cf68
add PluralElementsPackedCowStr
sffc Sep 10, 2024
5a3101f
Revert all changes in the experimental crate
sffc Sep 10, 2024
c33063c
Small changes to experimental crate call sites
sffc Sep 10, 2024
594863b
cargo make bakeddata experimental
sffc Sep 10, 2024
1b055db
doc, fmt, missing_apis
sffc Sep 10, 2024
7714c3b
Don't expose serde impl on `PluralElements`
sffc Sep 10, 2024
e858f66
rm PluralElementsField
sffc Sep 10, 2024
382c846
Move struct definitions closer together
sffc Sep 10, 2024
f8c10e5
clippy
sffc Sep 10, 2024
9670ab8
Make PluralElementsPackedCow generic
sffc Sep 10, 2024
3b307df
cargo make bakeddata experimental
sffc Sep 10, 2024
55c6d25
Make the fn private now that PluralElementsPackedCow is generic
sffc Sep 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
//!
//! Read more about data providers: [`icu_provider`]

use icu_plurals::provider::PluralElementsV1;
use icu_plurals::provider::PluralElementsPackedCowStr;
use icu_provider::prelude::*;

#[cfg(feature = "compiled_data")]
Expand Down Expand Up @@ -43,5 +43,5 @@ pub struct CurrencyExtendedDataV1<'data> {
/// Regards to the [Unicode Report TR35](https://unicode.org/reports/tr35/tr35-numbers.html#Currencies),
/// If no matching for specific count, the `other` count will be used.
#[cfg_attr(feature = "serde", serde(borrow))]
pub display_names: PluralElementsV1<'data>,
pub display_names: PluralElementsPackedCowStr<'data>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a fan of calling this PackedCowStr. I think PluralElementsV1 was fine as it's our first iteration of PluralElements. The user here really doesn't need to care about encoding details.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't typically call things V1 unless they are actually data structs. I let it land with this name initially because it's unstable, but I consider it an improvement to call the type by what it actually is.

}
8 changes: 4 additions & 4 deletions components/experimental/src/relativetime/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub struct PluralCategoryStr<'data>(pub PluralCategory, pub Cow<'data, str>);
pub struct PluralPatterns<'data, B> {
#[cfg_attr(feature = "serde", serde(borrow))]
#[doc(hidden)] // databake only
pub strings: icu_plurals::provider::PluralElementsV1<'data>,
pub strings: icu_plurals::provider::PluralElementsPackedCowStr<'data>,
#[cfg_attr(feature = "serde", serde(skip))]
#[doc(hidden)] // databake only
pub _phantom: PhantomData<B>,
Expand All @@ -126,16 +126,16 @@ impl<'data, B: PatternBackend<Store = str>> PluralPatterns<'data, B> {
}

#[cfg(feature = "datagen")]
impl<'data, B: PatternBackend<Store = str>> TryFrom<PluralElements<'data, str>>
impl<'data, B: PatternBackend<Store = str>> TryFrom<PluralElements<&'data str>>
robertbastian marked this conversation as resolved.
Show resolved Hide resolved
robertbastian marked this conversation as resolved.
Show resolved Hide resolved
for PluralPatterns<'static, B>
where
B::PlaceholderKeyCow<'data>: FromStr,
<B::PlaceholderKeyCow<'data> as FromStr>::Err: Debug,
{
type Error = icu_pattern::PatternError;

fn try_from(elements: PluralElements<str>) -> Result<Self, Self::Error> {
let make_pattern = |s: &str|
fn try_from(elements: PluralElements<&'data str>) -> Result<Self, Self::Error> {
let make_pattern = |s: &&str|
// TODO: Make pattern support apostrophes
Pattern::<B, String>::from_str(&s.replace('\'', "''")).map(|p| p.take_store());

Expand Down
1 change: 1 addition & 0 deletions components/plurals/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ icu_benchmark_macros = { path = "../../tools/benchmark/macros" }
icu_locale_core = { path = "../../components/locale_core" }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
postcard = { workspace = true, features = ["alloc"] }

[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
criterion = { workspace = true }
Expand Down
151 changes: 88 additions & 63 deletions components/plurals/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,21 +877,29 @@ where
}

#[derive(Debug)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
/// A bag of values for different plural cases.
pub struct PluralElements<'a, T: ?Sized> {
zero: Option<&'a T>,
one: Option<&'a T>,
two: Option<&'a T>,
few: Option<&'a T>,
many: Option<&'a T>,
other: &'a T,
explicit_zero: Option<&'a T>,
explicit_one: Option<&'a T>,
pub struct PluralElements<T> {
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
robertbastian marked this conversation as resolved.
Show resolved Hide resolved
zero: Option<T>,
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
one: Option<T>,
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
two: Option<T>,
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
few: Option<T>,
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
many: Option<T>,
other: T,
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
explicit_zero: Option<T>,
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
explicit_one: Option<T>,
}

impl<'a, T: ?Sized + PartialEq> PluralElements<'a, T> {
impl<T> PluralElements<T> {
/// Creates a new [`PluralElements`] with the given default value.
pub fn new(other: &'a T) -> Self {
pub fn new(other: T) -> Self {
Self {
other,
zero: None,
Expand All @@ -904,99 +912,116 @@ impl<'a, T: ?Sized + PartialEq> PluralElements<'a, T> {
}
}

/// The value for [`PluralCategory::Zero`]
pub fn zero(&self) -> &T {
self.zero.as_ref().unwrap_or(&self.other)
}

/// The value for [`PluralCategory::One`]
pub fn one(&self) -> &T {
self.one.as_ref().unwrap_or(&self.other)
}

/// The value for [`PluralCategory::Two`]
pub fn two(&self) -> &T {
self.two.as_ref().unwrap_or(&self.other)
}

/// The value for [`PluralCategory::Few`]
pub fn few(&self) -> &T {
self.few.as_ref().unwrap_or(&self.other)
}

/// The value for [`PluralCategory::Many`]
pub fn many(&self) -> &T {
self.many.as_ref().unwrap_or(&self.other)
}

/// The value for [`PluralCategory::Other`]
pub fn other(&self) -> &T {
&self.other
}

/// The value used when the [`PluralOperands`] are exactly 0.
pub fn explicit_zero(&self) -> Option<&T> {
self.explicit_zero.as_ref()
}

/// The value used when the [`PluralOperands`] are exactly 1.
pub fn explicit_one(&self) -> Option<&T> {
self.explicit_one.as_ref()
}

/// Applies a function `f` to map all values to another type.
pub fn map<B, F: Fn(T) -> B>(self, f: F) -> PluralElements<B> {
let f = &f;
PluralElements {
other: f(self.other),
zero: self.zero.map(f),
one: self.one.map(f),
two: self.two.map(f),
few: self.few.map(f),
many: self.many.map(f),
explicit_zero: self.explicit_zero.map(f),
explicit_one: self.explicit_one.map(f),
}
}
}

impl<T: PartialEq> PluralElements<T> {
/// Sets the value for [`PluralCategory::Zero`].
pub fn with_zero_value(self, zero: Option<&'a T>) -> Self {
pub fn with_zero_value(self, zero: Option<T>) -> Self {
Self {
zero: zero.filter(|&t| t != self.other),
zero: zero.filter(|t| *t != self.other),
..self
}
}

/// Sets the value for [`PluralCategory::One`].
pub fn with_one_value(self, one: Option<&'a T>) -> Self {
pub fn with_one_value(self, one: Option<T>) -> Self {
Self {
one: one.filter(|&t| t != self.other),
one: one.filter(|t| *t != self.other),
..self
}
}

/// Sets the value for [`PluralCategory::Two`].
pub fn with_two_value(self, two: Option<&'a T>) -> Self {
pub fn with_two_value(self, two: Option<T>) -> Self {
Self {
two: two.filter(|&t| t != self.other),
two: two.filter(|t| *t != self.other),
..self
}
}

/// Sets the value for [`PluralCategory::Few`].
pub fn with_few_value(self, few: Option<&'a T>) -> Self {
pub fn with_few_value(self, few: Option<T>) -> Self {
Self {
few: few.filter(|&t| t != self.other),
few: few.filter(|t| *t != self.other),
..self
}
}

/// Sets the value for [`PluralCategory::Many`].
pub fn with_many_value(self, many: Option<&'a T>) -> Self {
pub fn with_many_value(self, many: Option<T>) -> Self {
Self {
many: many.filter(|&t| t != self.other),
many: many.filter(|t| *t != self.other),
..self
}
}

/// Sets the value for explicit 0.
pub fn with_explicit_zero_value(self, explicit_zero: Option<&'a T>) -> Self {
pub fn with_explicit_zero_value(self, explicit_zero: Option<T>) -> Self {
Self {
explicit_zero,
..self
}
}

/// Sets the value for explicit 1.
pub fn with_explicit_one_value(self, explicit_one: Option<&'a T>) -> Self {
pub fn with_explicit_one_value(self, explicit_one: Option<T>) -> Self {
Self {
explicit_one,
..self
}
}

/// The value for [`PluralCategory::Zero`]
pub fn zero(&self) -> &'a T {
self.zero.unwrap_or(self.other)
}

/// The value for [`PluralCategory::One`]
pub fn one(&self) -> &'a T {
self.one.unwrap_or(self.other)
}

/// The value for [`PluralCategory::Two`]
pub fn two(&self) -> &'a T {
self.two.unwrap_or(self.other)
}

/// The value for [`PluralCategory::Few`]
pub fn few(&self) -> &'a T {
self.few.unwrap_or(self.other)
}

/// The value for [`PluralCategory::Many`]
pub fn many(&self) -> &'a T {
self.many.unwrap_or(self.other)
}

/// The value for [`PluralCategory::Other`]
pub fn other(&self) -> &'a T {
self.other
}

/// The value used when the [`PluralOperands`] are exactly 0.
pub fn explicit_zero(&self) -> Option<&'a T> {
self.explicit_zero
}

/// The value used when the [`PluralOperands`] are exactly 1.
pub fn explicit_one(&self) -> Option<&'a T> {
self.explicit_one
}
}
Loading
Loading