Skip to content

Commit

Permalink
Remove implicit LocaleFallbackProvider constructors (unicode-org#5183)
Browse files Browse the repository at this point in the history
Now that `LocaleFallbacker` is an explicit argument to `ExportDriver`,
it might make sense to always make it explicit on the
`LocaleFallbackProvider` as well.

The `_unstable`, `_with_buffer_provider`, and `_with_any_provider`
constructors are a bit odd for this crate anyway, no other adapter has
the constructor trinity. Removing them also has the added benefit of
removing the `serde` feature from the crate.
  • Loading branch information
robertbastian authored Jul 9, 2024
1 parent 2936110 commit 60eca99
Show file tree
Hide file tree
Showing 15 changed files with 44 additions and 194 deletions.
6 changes: 4 additions & 2 deletions components/icu/README.md

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

6 changes: 4 additions & 2 deletions components/icu/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
//! ```no_run
//! use icu::datetime::DateTimeFormatter;
//! use icu::locale::locale;
//! use icu::locale::fallback::LocaleFallbacker;
//! use icu_provider_adapters::fallback::LocaleFallbackProvider;
//! use icu_provider_blob::BlobDataProvider;
//!
Expand All @@ -61,10 +62,11 @@
//! let provider = BlobDataProvider::try_new_from_blob(data)
//! .expect("data should be valid");
//!
//! let provider =
//! LocaleFallbackProvider::try_new_with_buffer_provider(provider)
//! let fallbacker = LocaleFallbacker::try_new_with_buffer_provider(&provider)
//! .expect("provider should include fallback data");
//!
//! let provider = LocaleFallbackProvider::new(provider, fallbacker);
//!
//! let dtf = DateTimeFormatter::try_new_with_buffer_provider(
//! &provider,
//! &locale!("es-US").into(),
Expand Down
3 changes: 0 additions & 3 deletions ffi/capi/bindings/c/ICU4XDataProvider.h

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

2 changes: 0 additions & 2 deletions ffi/capi/bindings/cpp/ICU4XDataProvider.d.hpp

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

8 changes: 0 additions & 8 deletions ffi/capi/bindings/cpp/ICU4XDataProvider.hpp

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

24 changes: 1 addition & 23 deletions ffi/capi/bindings/dart/DataProvider.g.dart

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

15 changes: 1 addition & 14 deletions ffi/capi/bindings/js/ICU4XDataProvider.d.ts

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

17 changes: 0 additions & 17 deletions ffi/capi/bindings/js/ICU4XDataProvider.mjs

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

40 changes: 5 additions & 35 deletions ffi/capi/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,39 +176,8 @@ pub mod ffi {
Ok(())
}

/// Enables locale fallbacking for data requests made to this provider.
///
/// Note that the test provider (from `create_test`) already has fallbacking enabled.
#[diplomat::rust_link(
icu_provider_adapters::fallback::LocaleFallbackProvider::try_new,
FnInStruct
)]
#[diplomat::rust_link(
icu_provider_adapters::fallback::LocaleFallbackProvider,
Struct,
compact
)]
pub fn enable_locale_fallback(&mut self) -> Result<(), ICU4XDataError> {
use ICU4XDataProviderInner::*;
*self = match core::mem::replace(&mut self.0, Destroyed) {
Destroyed => Err(icu_provider::DataError::custom(
"This provider has been destroyed",
))?,
#[cfg(feature = "compiled_data")]
Compiled => Err(icu_provider::DataError::custom(
"The compiled provider cannot be modified",
))?,
Empty => Err(icu_provider::DataErrorKind::MarkerNotFound.into_error())?,
#[cfg(feature = "buffer_provider")]
Buffer(inner) => convert_buffer_provider(
LocaleFallbackProvider::try_new_with_buffer_provider(inner)?,
),
};
Ok(())
}

#[diplomat::rust_link(
icu_provider_adapters::fallback::LocaleFallbackProvider::new_with_fallbacker,
icu_provider_adapters::fallback::LocaleFallbackProvider::new,
FnInStruct
)]
#[diplomat::rust_link(
Expand All @@ -233,9 +202,10 @@ pub mod ffi {
))?,
Empty => Err(icu_provider::DataErrorKind::MarkerNotFound.into_error())?,
#[cfg(feature = "buffer_provider")]
Buffer(inner) => convert_buffer_provider(
LocaleFallbackProvider::new_with_fallbacker(inner, fallbacker.0.clone()),
),
Buffer(inner) => convert_buffer_provider(LocaleFallbackProvider::new(
inner,
fallbacker.0.clone(),
)),
};
Ok(())
}
Expand Down
3 changes: 0 additions & 3 deletions provider/adapters/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,3 @@ serde = { workspace = true, features = ["derive", "alloc"], optional = true }
icu_provider = { path = "../../provider/core", features = ["macros", "deserialize_json"] }
icu_locale = { path = "../../components/locale" }
writeable = { path = "../../utils/writeable" }

[features]
serde = ["dep:serde", "zerovec/serde", "icu_locale/serde", "icu_provider/serde"]
89 changes: 13 additions & 76 deletions provider/adapters/src/fallback/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
//! A data provider wrapper that performs locale fallback.
use crate::helpers::result_is_err_missing_locale;
use icu_locale::provider::*;
use icu_locale::LocaleFallbacker;
#[doc(no_inline)]
pub use icu_locale::LocaleFallbacker;
use icu_provider::prelude::*;
use icu_provider::DryDataProvider;
use icu_provider::DynamicDryDataProvider;
Expand All @@ -20,13 +20,9 @@ use icu_provider::DynamicDryDataProvider;
/// use icu_locale::langid;
/// use icu_provider::prelude::*;
/// use icu_provider::hello_world::*;
/// # let provider = HelloWorldProvider;
/// # struct LocaleFallbackProvider;
/// # impl LocaleFallbackProvider {
/// # fn try_new_unstable(provider: HelloWorldProvider) -> Result<icu_provider_adapters::fallback::LocaleFallbackProvider<HelloWorldProvider>, ()> {
/// # Ok(icu_provider_adapters::fallback::LocaleFallbackProvider::new_with_fallbacker(provider, icu_locale::LocaleFallbacker::new().static_to_owned()))
/// # }
/// # }
/// use icu_provider_adapters::fallback::LocaleFallbackProvider;
///
/// let provider = HelloWorldProvider;
///
/// let id = DataIdentifierCow::from_locale(langid!("ja-JP").into());
///
Expand All @@ -37,8 +33,7 @@ use icu_provider::DynamicDryDataProvider;
/// }).expect_err("No fallback");
///
/// // But if we wrap the provider in a fallback provider...
/// let provider = LocaleFallbackProvider::try_new_unstable(provider)
/// .expect("Fallback data present");
/// let provider = LocaleFallbackProvider::new(provider, icu_locale::LocaleFallbacker::new().static_to_owned());
///
/// // ...then we can load "ja-JP" based on "ja" data
/// let response =
Expand All @@ -62,67 +57,11 @@ pub struct LocaleFallbackProvider<P> {
fallbacker: LocaleFallbacker,
}

impl<P> LocaleFallbackProvider<P>
where
P: DataProvider<LocaleFallbackLikelySubtagsV1Marker>
+ DataProvider<LocaleFallbackParentsV1Marker>
+ DataProvider<CollationFallbackSupplementV1Marker>,
{
/// Create a [`LocaleFallbackProvider`] by wrapping another data provider and then loading
/// fallback data from it.
///
/// If the data provider being wrapped does not contain fallback data, use
/// [`LocaleFallbackProvider::new_with_fallbacker`].
pub fn try_new_unstable(provider: P) -> Result<Self, DataError> {
let fallbacker = LocaleFallbacker::try_new_unstable(&provider)?;
Ok(Self {
inner: provider,
fallbacker,
})
}
}

impl<P> LocaleFallbackProvider<P>
where
P: AnyProvider,
{
/// Create a [`LocaleFallbackProvider`] by wrapping another data provider and then loading
/// fallback data from it.
///
/// If the data provider being wrapped does not contain fallback data, use
/// [`LocaleFallbackProvider::new_with_fallbacker`].
pub fn try_new_with_any_provider(provider: P) -> Result<Self, DataError> {
let fallbacker = LocaleFallbacker::try_new_with_any_provider(&provider)?;
Ok(Self {
inner: provider,
fallbacker,
})
}
}

#[cfg(feature = "serde")]
impl<P> LocaleFallbackProvider<P>
where
P: BufferProvider,
{
/// Create a [`LocaleFallbackProvider`] by wrapping another data provider and then loading
/// fallback data from it.
///
/// If the data provider being wrapped does not contain fallback data, use
/// [`LocaleFallbackProvider::new_with_fallbacker`].
pub fn try_new_with_buffer_provider(provider: P) -> Result<Self, DataError> {
let fallbacker = LocaleFallbacker::try_new_with_buffer_provider(&provider)?;
Ok(Self {
inner: provider,
fallbacker,
})
}
}

impl<P> LocaleFallbackProvider<P> {
/// Wrap a provider with an arbitrary fallback engine.
/// Wraps a provider with a provider performing fallback under the given fallbacker.
///
/// This relaxes the requirement that the wrapped provider contains its own fallback data.
/// If the underlying provider contains deduplicated data, it is important to use the
/// same fallback data that `ExportDriver` used.
///
/// # Examples
///
Expand All @@ -147,7 +86,7 @@ impl<P> LocaleFallbackProvider<P> {
/// // `HelloWorldProvider` does not contain fallback data,
/// // but we can construct a fallbacker with `icu_locale`'s
/// // compiled data.
/// let provider = LocaleFallbackProvider::new_with_fallbacker(
/// let provider = LocaleFallbackProvider::new(
/// provider,
/// LocaleFallbacker::new().static_to_owned(),
/// );
Expand All @@ -162,7 +101,7 @@ impl<P> LocaleFallbackProvider<P> {
///
/// assert_eq!("Hallo Welt", german_hello_world.payload.get().message);
/// ```
pub fn new_with_fallbacker(provider: P, fallbacker: LocaleFallbacker) -> Self {
pub fn new(provider: P, fallbacker: LocaleFallbacker) -> Self {
Self {
inner: provider,
fallbacker,
Expand Down Expand Up @@ -340,10 +279,8 @@ fn dry_test() {
}
}

let provider = LocaleFallbackProvider::new_with_fallbacker(
TestProvider,
LocaleFallbacker::new().static_to_owned(),
);
let provider =
LocaleFallbackProvider::new(TestProvider, LocaleFallbacker::new().static_to_owned());

assert_eq!(
provider
Expand Down
2 changes: 1 addition & 1 deletion provider/blob/benches/auxkey_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ fn auxkey_bench_for_version(c: &mut Criterion, blob: &[u8], version_id: &str) {
b.iter(|| BlobDataProvider::try_new_from_blob(black_box(blob).into()).unwrap());
});

let provider = LocaleFallbackProvider::new_with_fallbacker(
let provider = LocaleFallbackProvider::new(
BlobDataProvider::try_new_from_blob(black_box(blob).into()).unwrap(),
LocaleFallbacker::new().static_to_owned(),
);
Expand Down
7 changes: 5 additions & 2 deletions provider/source/src/collator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,13 @@ collation_provider!(
fn test_zh_non_baked() {
use core::cmp::Ordering;
use icu::collator::{Collator, CollatorOptions};
use icu::locale::fallback::LocaleFallbacker;
use icu_provider_adapters::fallback::LocaleFallbackProvider;

let provider =
LocaleFallbackProvider::try_new_unstable(SourceDataProvider::new_testing()).unwrap();
let provider = LocaleFallbackProvider::new(
SourceDataProvider::new_testing(),
LocaleFallbacker::new_without_data(),
);

// Note: ㄅ is Bopomofo.
{
Expand Down
Loading

0 comments on commit 60eca99

Please sign in to comment.