Update locale_core::preferences to stakeholder alignment

unicode-org · Oct 26, 2024 · 0dab081 · 0dab081
1 parent b78e279
commit 0dab081
Show file tree

Hide file tree

Showing 27 changed files with 627 additions and 732 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/components/experimental/src/duration/formatter.rs b/components/experimental/src/duration/formatter.rs
@@ -147,7 +147,7 @@ impl DurationUnitFormatter {
     }
 }
 
-impl From<BaseStyle> for icu_list::ListLength {
+impl From<BaseStyle> for icu_list::ListFormatterOptions {
     fn from(style: BaseStyle) -> Self {
         // Section 1.1.13
         // 1. Let lfOpts be OrdinaryObjectCreate(null).
@@ -157,11 +157,12 @@ impl From<BaseStyle> for icu_list::ListLength {
         //     a. Set listStyle to "short".
         // 5. Perform ! CreateDataPropertyOrThrow(lfOpts, "style", listStyle).
         // 6. Let lf be ! Construct(%ListFormat%, « durationFormat.[[Locale]], lfOpts »).
-        match style {
+        let style = match style {
             BaseStyle::Long => ListLength::Wide,
             BaseStyle::Short | BaseStyle::Digital => ListLength::Short,
             BaseStyle::Narrow => ListLength::Narrow,
-        }
+        };
+        Self::new().style(style)
     }
 }
 
@@ -198,7 +199,7 @@ impl DurationFormatter {
             digital,
             options,
             unit: DurationUnitFormatter::try_new(locale, options)?,
-            list: ListFormatter::try_new_unit_with_length(locale, options.base.into())?,
+            list: ListFormatter::try_new_unit_with_length(locale.into(), options.base.into())?,
             fdf: FixedDecimalFormatter::try_new(locale, Default::default())?,
         })
     }
@@ -229,7 +230,7 @@ impl DurationFormatter {
             unit: DurationUnitFormatter::try_new_unstable(provider, locale, options)?,
             list: ListFormatter::try_new_unit_with_length_unstable(
                 provider,
-                locale,
+                locale.into(),
                 options.base.into(),
             )?,
             fdf: FixedDecimalFormatter::try_new_unstable(provider, locale, Default::default())?,

diff --git a/components/list/README.md b/components/list/README.md
diff --git a/components/list/examples/and_list.rs b/components/list/examples/and_list.rs
@@ -6,12 +6,15 @@
 icu_benchmark_macros::instrument!();
 use icu_benchmark_macros::println;
 
-use icu::list::{ListFormatter, ListLength};
+use icu::list::{ListFormatter, ListFormatterOptions, ListLength};
 use icu::locale::locale;
 
 fn main() {
-    let list_formatter =
-        ListFormatter::try_new_and_with_length(&locale!("es").into(), ListLength::Wide).unwrap();
+    let list_formatter = ListFormatter::try_new_and_with_length(
+        locale!("es").into(),
+        ListFormatterOptions::new().style(ListLength::Wide),
+    )
+    .unwrap();
 
     println!(
         "{}",

diff --git a/components/list/src/lib.rs b/components/list/src/lib.rs
@@ -12,13 +12,14 @@
 //! ## Formatting *and* lists in Spanish
 //!
 //! ```
-//! # use icu::list::{ListFormatter, ListLength};
+//! # use icu::list::{ListFormatter, ListFormatterOptions, ListLength};
 //! # use icu::locale::locale;
 //! # use writeable::*;
 //! #
 //! let list_formatter = ListFormatter::try_new_and_with_length(
-//!     &locale!("es").into(),
-//!     ListLength::Wide,
+//!     locale!("es").into(),
+//!     ListFormatterOptions::new()
+//!       .style(ListLength::Wide)
 //! )
 //! .expect("locale should be present");
 //!
@@ -37,13 +38,14 @@
 //! ## Formatting *or* lists in Thai
 //!
 //! ```
-//! # use icu::list::{ListFormatter, ListLength};
+//! # use icu::list::{ListFormatter, ListFormatterOptions, ListLength};
 //! # use icu::locale::locale;
 //! # use writeable::*;
 //! #
 //! let list_formatter = ListFormatter::try_new_or_with_length(
-//!     &locale!("th").into(),
-//!     ListLength::Short,
+//!     locale!("th").into(),
+//!     ListFormatterOptions::new()
+//!       .style(ListLength::Short)
 //! )
 //! .expect("locale should be present");
 //!
@@ -54,13 +56,14 @@
 //! ## Formatting unit lists in English
 //!
 //! ```
-//! # use icu::list::{ListFormatter, ListLength};
+//! # use icu::list::{ListFormatter, ListFormatterOptions, ListLength};
 //! # use icu::locale::locale;
 //! # use writeable::*;
 //! #
 //! let list_formatter = ListFormatter::try_new_unit_with_length(
-//!     &locale!("en").into(),
-//!     ListLength::Wide,
+//!     locale!("en").into(),
+//!     ListFormatterOptions::new()
+//!       .style(ListLength::Wide)
 //! )
 //! .expect("locale should be present");
 //!
@@ -91,25 +94,10 @@ extern crate alloc;
 
 mod lazy_automaton;
 mod list_formatter;
+mod options;
 mod patterns;
 
 pub mod provider;
 
 pub use list_formatter::*;
-
-/// Represents the style of a list. See the
-/// [CLDR spec](https://unicode.org/reports/tr35/tr35-general.html#ListPatterns)
-/// for an explanation of the different styles.
-#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, Default)]
-#[non_exhaustive]
-pub enum ListLength {
-    /// A typical list
-    #[default]
-    Wide,
-    /// A shorter list
-    Short,
-    /// The shortest type of list
-    Narrow,
-    // *Important*: When adding a variant here, make sure the code in
-    // ListFormatterPatterns::{start, middle, end, pair} stays panic-free!
-}
+pub use options::*;
diff --git a/components/list/src/list_formatter.rs b/components/list/src/list_formatter.rs
@@ -5,13 +5,28 @@
 use crate::provider::*;
 use crate::ListLength;
 use core::fmt::{self, Write};
+use icu_locale_core::preferences::{define_options, define_preferences};
 use icu_provider::marker::ErasedMarker;
 use icu_provider::prelude::*;
 use writeable::*;
 
 #[cfg(doc)]
 extern crate writeable;
 
+define_preferences!(
+    /// The preferences for list formatting.
+    ListFormatterPreferences,
+    {}
+);
+define_options!(
+    /// The options for list formatting.
+    ListFormatterOptions,
+    {
+        /// Style
+        style: ListLength
+    }
+);
+
 /// A formatter that renders sequences of items in an i18n-friendly way. See the
 /// [crate-level documentation](crate) for more details.
 #[derive(Debug)]
@@ -22,7 +37,7 @@ pub struct ListFormatter {
 macro_rules! constructor {
     ($name: ident, $name_any: ident, $name_buffer: ident, $name_unstable: ident, $marker: ty, $doc: literal) => {
         icu_provider::gen_any_buffer_data_constructors!(
-            (locale, style: ListLength) ->  error: DataError,
+            (prefs: ListFormatterPreferences, options: ListFormatterOptions) ->  error: DataError,
             #[doc = concat!("Creates a new [`ListFormatter`] that produces a ", $doc, "-type list using compiled data.")]
             ///
             /// See the [CLDR spec](https://unicode.org/reports/tr35/tr35-general.html#ListPatterns) for
@@ -43,18 +58,20 @@ macro_rules! constructor {
         #[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, Self::$name)]
         pub fn $name_unstable(
             provider: &(impl DataProvider<$marker> + ?Sized),
-            locale: &DataLocale,
-            length: ListLength,
+            prefs: ListFormatterPreferences,
+            options: ListFormatterOptions,
         ) -> Result<Self, DataError> {
+            let length = match options.style.unwrap_or_default() {
+                ListLength::Narrow => ListFormatterPatternsV2::NARROW,
+                ListLength::Short => ListFormatterPatternsV2::SHORT,
+                ListLength::Wide => ListFormatterPatternsV2::WIDE,
+            };
+            let locale = get_data_locale_from_prefs(prefs);
             let data = provider
                 .load(DataRequest {
                     id: DataIdentifierBorrowed::for_marker_attributes_and_locale(
-                        match length {
-                            ListLength::Narrow => ListFormatterPatternsV2::NARROW,
-                            ListLength::Short => ListFormatterPatternsV2::SHORT,
-                            ListLength::Wide => ListFormatterPatternsV2::WIDE,
-                        },
-                        locale),
+                        length,
+                        &locale),
                     ..Default::default()
                 })?
                 .payload
@@ -64,6 +81,16 @@ macro_rules! constructor {
     };
 }
 
+fn get_data_locale_from_prefs(prefs: ListFormatterPreferences) -> DataLocale {
+    DataLocale::from_subtags(
+        prefs.language,
+        prefs.script,
+        prefs.region,
+        prefs.variant,
+        prefs.subdivision,
+    )
+}
+
 impl ListFormatter {
     constructor!(
         try_new_and_with_length,
@@ -103,8 +130,9 @@ impl ListFormatter {
     /// # use icu::locale::locale;
     /// # use writeable::*;
     /// let formatteur = ListFormatter::try_new_and_with_length(
-    ///     &locale!("fr").into(),
-    ///     ListLength::Wide,
+    ///     locale!("fr").into(),
+    ///     ListFormatterOptions::new()
+    ///         .style(ListLength::Wide)
     /// )
     /// .unwrap();
     /// let pays = ["Italie", "France", "Espagne", "Allemagne"];
@@ -356,8 +384,8 @@ mod tests {
     macro_rules! test {
         ($locale:literal, $type:ident, $(($input:expr, $output:literal),)+) => {
             let f = ListFormatter::$type(
-                &icu::locale::locale!($locale).into(),
-                ListLength::Wide
+                icu::locale::locale!($locale).into(),
+                Default::default(),
             ).unwrap();
             $(
                 assert_writeable_eq!(f.format($input.iter()), $output);

diff --git a/components/list/src/options.rs b/components/list/src/options.rs
@@ -0,0 +1,20 @@
+// This file is part of ICU4X. For terms of use, please see the file
+// called LICENSE at the top level of the ICU4X source tree
+// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
+
+/// Represents the style of a list. See the
+/// [CLDR spec](https://unicode.org/reports/tr35/tr35-general.html#ListPatterns)
+/// for an explanation of the different styles.
+#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, Default)]
+#[non_exhaustive]
+pub enum ListLength {
+    /// A typical list
+    #[default]
+    Wide,
+    /// A shorter list
+    Short,
+    /// The shortest type of list
+    Narrow,
+    // *Important*: When adding a variant here, make sure the code in
+    // ListFormatterPatterns::{start, middle, end, pair} stays panic-free!
+}
diff --git a/components/locale_core/Cargo.toml b/components/locale_core/Cargo.toml
@@ -25,15 +25,16 @@ litemap = { workspace = true, features = ["alloc"] }
 tinystr = { workspace = true, features = ["alloc"] }
 writeable = { workspace = true }
 
-databake = { workspace = true, features = ["derive"], optional = true}
+databake = { workspace = true, features = ["derive"], optional = true }
 serde = { workspace = true, features = ["alloc", "derive"], optional = true }
 zerovec = { workspace = true, optional = true }
 
 [dev-dependencies]
 iai = { workspace = true }
 icu = { path = "../../components/icu", default-features = false }
+icu_provider = { workspace = true }
 icu_benchmark_macros = { path = "../../tools/benchmark/macros" }
-litemap = { path = "../../utils/litemap", features = ["testing"]}
+litemap = { path = "../../utils/litemap", features = ["testing"] }
 postcard = { workspace = true, features = ["use-std"] }
 potential_utf = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
@@ -51,7 +52,7 @@ zerovec = ["dep:zerovec", "tinystr/zerovec"]
 bench = ["serde"]
 
 [lib]
-bench = false  # This option is required for Benchmark CI
+bench = false # This option is required for Benchmark CI
 
 [package.metadata.cargo-all-features]
 # Bench feature gets tested separately and is only relevant for CI