From a2a006029220684340cf2adf6c764bb7f8b19b37 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Mon, 16 Dec 2024 19:05:35 -0800 Subject: [PATCH] Change icu_pattern PlaceholderValueProvider and parts behavior (#5908) See discussion in #5897 --- components/pattern/src/common.rs | 141 +++++++++++++++++++------ components/pattern/src/double.rs | 47 +++++---- components/pattern/src/frontend/mod.rs | 42 +++----- components/pattern/src/lib.rs | 2 - components/pattern/src/multi_named.rs | 50 +++++---- components/pattern/src/single.rs | 46 ++++---- ffi/capi/tests/missing_apis.txt | 6 +- 7 files changed, 213 insertions(+), 121 deletions(-) diff --git a/components/pattern/src/common.rs b/components/pattern/src/common.rs index a2e328e4baa..951cea986ff 100644 --- a/components/pattern/src/common.rs +++ b/components/pattern/src/common.rs @@ -3,7 +3,7 @@ // (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ). use crate::Error; -use writeable::{Part, TryWriteable}; +use writeable::{TryWriteable, Writeable}; #[cfg(feature = "alloc")] use alloc::{borrow::Cow, boxed::Box}; @@ -110,30 +110,97 @@ pub trait PatternBackend: crate::private::Sealed + 'static + core::fmt::Debug { fn empty() -> &'static Self::Store; } -/// Default annotation for the literal portion of a pattern. +/// Trait implemented on collections that can produce [`TryWriteable`]s for interpolation. /// -/// For more information, see [`PlaceholderValueProvider`]. For an example, see [`Pattern`]. +/// This trait can add [`Part`]s for individual literals or placeholders. The implementations +/// of this trait on standard types do not add any [`Part`]s. /// -/// [`Pattern`]: crate::Pattern -pub const PATTERN_LITERAL_PART: Part = Part { - category: "pattern", - value: "literal", -}; - -/// Default annotation for the placeholder portion of a pattern. +/// # Examples /// -/// For more information, see [`PlaceholderValueProvider`]. For an example, see [`Pattern`]. +/// A custom implementation that adds parts: /// -/// [`Pattern`]: crate::Pattern -pub const PATTERN_PLACEHOLDER_PART: Part = Part { - category: "pattern", - value: "placeholder", -}; - -/// Trait implemented on collections that can produce [`TryWriteable`]s for interpolation. +/// ``` +/// use core::str::FromStr; +/// use icu_pattern::Pattern; +/// use icu_pattern::DoublePlaceholder; +/// use icu_pattern::DoublePlaceholderKey; +/// use icu_pattern::PlaceholderValueProvider; +/// use writeable::adapters::WithPart; +/// use writeable::adapters::WriteableAsTryWriteableInfallible; +/// use writeable::assert_writeable_parts_eq; +/// use writeable::Part; +/// use writeable::Writeable; +/// +/// let pattern = Pattern::::try_from_str( +/// "Hello, {0} and {1}!", +/// Default::default(), +/// ) +/// .unwrap(); +/// +/// struct ValuesWithParts<'a>(&'a str, &'a str); +/// +/// const PART_PLACEHOLDER_0: Part = Part { +/// category: "custom", +/// value: "placeholder0", +/// }; +/// const PART_PLACEHOLDER_1: Part = Part { +/// category: "custom", +/// value: "placeholder1", +/// }; +/// const PART_LITERAL: Part = Part { +/// category: "custom", +/// value: "literal", +/// }; +/// +/// impl PlaceholderValueProvider for ValuesWithParts<'_> { +/// type Error = core::convert::Infallible; +/// +/// type W<'a> = WriteableAsTryWriteableInfallible> +/// where +/// Self: 'a; +/// +/// type L<'a, 'l> = WithPart<&'l str> +/// where +/// Self: 'a; +/// +/// #[inline] +/// fn value_for(&self, key: DoublePlaceholderKey) -> Self::W<'_> { +/// let writeable = match key { +/// DoublePlaceholderKey::Place0 => WithPart { +/// writeable: self.0, +/// part: PART_PLACEHOLDER_0, +/// }, +/// DoublePlaceholderKey::Place1 => WithPart { +/// writeable: self.1, +/// part: PART_PLACEHOLDER_1, +/// }, +/// }; +/// WriteableAsTryWriteableInfallible(writeable) +/// } +/// +/// #[inline] +/// fn map_literal<'a, 'l>(&'a self, literal: &'l str) -> Self::L<'a, 'l> { +/// WithPart { +/// writeable: literal, +/// part: PART_LITERAL, +/// } +/// } +/// } /// -/// This trait determines the [`Part`]s produced by the writeable. In this crate, implementations -/// of this trait default to using [`PATTERN_LITERAL_PART`] and [`PATTERN_PLACEHOLDER_PART`]. +/// assert_writeable_parts_eq!( +/// pattern.interpolate(ValuesWithParts("Alice", "Bob")), +/// "Hello, Alice and Bob!", +/// [ +/// (0, 7, PART_LITERAL), +/// (7, 12, PART_PLACEHOLDER_0), +/// (12, 17, PART_LITERAL), +/// (17, 20, PART_PLACEHOLDER_1), +/// (20, 21, PART_LITERAL), +/// ] +/// ); +/// ``` +/// +/// [`Part`]: writeable::Part pub trait PlaceholderValueProvider { type Error; @@ -141,11 +208,19 @@ pub trait PlaceholderValueProvider { where Self: 'a; - const LITERAL_PART: Part; + type L<'a, 'l>: Writeable + where + Self: 'a; - /// Returns the [`TryWriteable`] to substitute for the given placeholder - /// and the [`Part`] representing it. - fn value_for(&self, key: K) -> (Self::W<'_>, Part); + /// Returns the [`TryWriteable`] to substitute for the given placeholder. + /// + /// See [`PatternItem::Placeholder`] + fn value_for(&self, key: K) -> Self::W<'_>; + + /// Maps a literal string to a [`Writeable`] that could contain parts. + /// + /// See [`PatternItem::Literal`] + fn map_literal<'a, 'l>(&'a self, literal: &'l str) -> Self::L<'a, 'l>; } impl<'b, K, T> PlaceholderValueProvider for &'b T @@ -153,13 +228,19 @@ where T: PlaceholderValueProvider + ?Sized, { type Error = T::Error; - type W<'a> - = T::W<'a> + + type W<'a> = T::W<'a> where - T: 'a, - 'b: 'a; - const LITERAL_PART: Part = T::LITERAL_PART; - fn value_for(&self, key: K) -> (Self::W<'_>, Part) { + Self: 'a; + + type L<'a, 'l> = T::L<'a, 'l> + where + Self: 'a; + + fn value_for(&self, key: K) -> Self::W<'_> { (*self).value_for(key) } + fn map_literal<'a, 'l>(&'a self, literal: &'l str) -> Self::L<'a, 'l> { + (*self).map_literal(literal) + } } diff --git a/components/pattern/src/double.rs b/components/pattern/src/double.rs index e5129e490fc..d8314974f47 100644 --- a/components/pattern/src/double.rs +++ b/components/pattern/src/double.rs @@ -75,22 +75,26 @@ where W1: Writeable, { type Error = Infallible; - type W<'a> - = WriteableAsTryWriteableInfallible> + + type W<'a> = WriteableAsTryWriteableInfallible> + where + Self: 'a; + + type L<'a, 'l> = &'l str where - W0: 'a, - W1: 'a; - const LITERAL_PART: writeable::Part = crate::PATTERN_LITERAL_PART; + Self: 'a; + #[inline] - fn value_for(&self, key: DoublePlaceholderKey) -> (Self::W<'_>, writeable::Part) { + fn value_for(&self, key: DoublePlaceholderKey) -> Self::W<'_> { let writeable = match key { DoublePlaceholderKey::Place0 => Either::Left(&self.0), DoublePlaceholderKey::Place1 => Either::Right(&self.1), }; - ( - WriteableAsTryWriteableInfallible(writeable), - crate::PATTERN_PLACEHOLDER_PART, - ) + WriteableAsTryWriteableInfallible(writeable) + } + #[inline] + fn map_literal<'a, 'l>(&'a self, literal: &'l str) -> Self::L<'a, 'l> { + literal } } @@ -99,22 +103,27 @@ where W: Writeable, { type Error = Infallible; - type W<'a> - = WriteableAsTryWriteableInfallible<&'a W> + + type W<'a> = WriteableAsTryWriteableInfallible<&'a W> + where + Self: 'a; + + type L<'a, 'l> = &'l str where - W: 'a; - const LITERAL_PART: writeable::Part = crate::PATTERN_LITERAL_PART; + Self: 'a; + #[inline] - fn value_for(&self, key: DoublePlaceholderKey) -> (Self::W<'_>, writeable::Part) { + fn value_for(&self, key: DoublePlaceholderKey) -> Self::W<'_> { let [item0, item1] = self; let writeable = match key { DoublePlaceholderKey::Place0 => item0, DoublePlaceholderKey::Place1 => item1, }; - ( - WriteableAsTryWriteableInfallible(writeable), - crate::PATTERN_PLACEHOLDER_PART, - ) + WriteableAsTryWriteableInfallible(writeable) + } + #[inline] + fn map_literal<'a, 'l>(&'a self, literal: &'l str) -> Self::L<'a, 'l> { + literal } } diff --git a/components/pattern/src/frontend/mod.rs b/components/pattern/src/frontend/mod.rs index 39a22c6a31f..a8cf38dad85 100644 --- a/components/pattern/src/frontend/mod.rs +++ b/components/pattern/src/frontend/mod.rs @@ -15,11 +15,7 @@ use crate::Parser; use crate::ParserOptions; #[cfg(feature = "alloc")] use alloc::{borrow::ToOwned, boxed::Box, str::FromStr, string::String}; -use core::{ - convert::Infallible, - fmt::{self, Write}, - marker::PhantomData, -}; +use core::{convert::Infallible, fmt, marker::PhantomData}; use writeable::{adapters::TryWriteableInfallibleAsWriteable, PartsWrite, TryWriteable, Writeable}; /// A string pattern with placeholders. @@ -38,21 +34,18 @@ use writeable::{adapters::TryWriteableInfallibleAsWriteable, PartsWrite, TryWrit /// /// # Format to Parts /// -/// [`Pattern`] supports interpolating with [writeable::Part]s, annotations for whether the -/// substring was a placeholder or a literal. -/// -/// By default, the substrings are annotated with [`PATTERN_LITERAL_PART`] and -/// [`PATTERN_PLACEHOLDER_PART`]. This can be customized with [`PlaceholderValueProvider`]. +/// [`Pattern`] propagates [`Part`]s from inner writeables. In addition, it supports annotating +/// [`Part`]s for individual literals or placeholders via the [`PlaceholderValueProvider`] trait. /// /// # Examples /// -/// Interpolating a [`SinglePlaceholder`] pattern with parts: +/// Interpolating a [`SinglePlaceholder`] pattern: /// /// ``` /// use core::str::FromStr; /// use icu_pattern::Pattern; /// use icu_pattern::SinglePlaceholder; -/// use writeable::assert_writeable_parts_eq; +/// use writeable::assert_writeable_eq; /// /// let pattern = Pattern::::try_from_str( /// "Hello, {0}!", @@ -60,20 +53,16 @@ use writeable::{adapters::TryWriteableInfallibleAsWriteable, PartsWrite, TryWrit /// ) /// .unwrap(); /// -/// assert_writeable_parts_eq!( +/// assert_writeable_eq!( /// pattern.interpolate(["Alice"]), -/// "Hello, Alice!", -/// [ -/// (0, 7, icu_pattern::PATTERN_LITERAL_PART), -/// (7, 12, icu_pattern::PATTERN_PLACEHOLDER_PART), -/// (12, 13, icu_pattern::PATTERN_LITERAL_PART), -/// ] +/// "Hello, Alice!" /// ); /// ``` /// /// [`SinglePlaceholder`]: crate::SinglePlaceholder /// [`DoublePlaceholder`]: crate::DoublePlaceholder /// [`MultiNamedPlaceholder`]: crate::MultiNamedPlaceholder +/// [`Part`]: writeable::Part #[cfg_attr(feature = "yoke", derive(yoke::Yokeable))] #[repr(transparent)] pub struct Pattern { @@ -344,17 +333,14 @@ where for item in it { match item { PatternItem::Literal(s) => { - sink.with_part(P::LITERAL_PART, |sink| sink.write_str(s))?; + self.value_provider.map_literal(s).write_to_parts(sink)?; } PatternItem::Placeholder(key) => { - let (element_writeable, part) = self.value_provider.value_for(key); - sink.with_part(part, |sink| { - if let Err(e) = element_writeable.try_write_to_parts(sink)? { - // Keep the first error if there was one - error.get_or_insert(e); - } - Ok(()) - })?; + let element_writeable = self.value_provider.value_for(key); + if let Err(e) = element_writeable.try_write_to_parts(sink)? { + // Keep the first error if there was one + error.get_or_insert(e); + } } } #[cfg(debug_assertions)] diff --git a/components/pattern/src/lib.rs b/components/pattern/src/lib.rs index 78b9cad560b..192427adbb1 100644 --- a/components/pattern/src/lib.rs +++ b/components/pattern/src/lib.rs @@ -66,8 +66,6 @@ pub use common::PatternItem; #[cfg(feature = "alloc")] pub use common::PatternItemCow; pub use common::PlaceholderValueProvider; -pub use common::PATTERN_LITERAL_PART; -pub use common::PATTERN_PLACEHOLDER_PART; pub use double::DoublePlaceholder; pub use double::DoublePlaceholderKey; pub use error::PatternError; diff --git a/components/pattern/src/multi_named.rs b/components/pattern/src/multi_named.rs index ace5e3b2474..62a5faa1ae3 100644 --- a/components/pattern/src/multi_named.rs +++ b/components/pattern/src/multi_named.rs @@ -94,22 +94,25 @@ where W: Writeable, { type Error = MissingNamedPlaceholderError<'k>; - type W<'a> - = Result<&'a W, Self::Error> + + type W<'a> = Result<&'a W, Self::Error> + where + Self: 'a; + + type L<'a, 'l> = &'l str where - W: 'a, Self: 'a; - const LITERAL_PART: writeable::Part = crate::PATTERN_LITERAL_PART; + #[inline] - fn value_for<'a>( - &'a self, - key: MultiNamedPlaceholderKey<'k>, - ) -> (Self::W<'a>, writeable::Part) { - let writeable = match self.get(key.0) { + fn value_for<'a>(&'a self, key: MultiNamedPlaceholderKey<'k>) -> Self::W<'a> { + match self.get(key.0) { Some(value) => Ok(value), None => Err(MissingNamedPlaceholderError { name: key.0 }), - }; - (writeable, crate::PATTERN_PLACEHOLDER_PART) + } + } + #[inline] + fn map_literal<'a, 'l>(&'a self, literal: &'l str) -> Self::L<'a, 'l> { + literal } } @@ -121,22 +124,25 @@ where S: litemap::store::Store, { type Error = MissingNamedPlaceholderError<'k>; - type W<'a> - = Result<&'a W, Self::Error> + + type W<'a> = Result<&'a W, Self::Error> + where + Self: 'a; + + type L<'a, 'l> = &'l str where - W: 'a, Self: 'a; - const LITERAL_PART: writeable::Part = crate::PATTERN_LITERAL_PART; + #[inline] - fn value_for<'a>( - &'a self, - key: MultiNamedPlaceholderKey<'k>, - ) -> (Self::W<'a>, writeable::Part) { - let writeable = match self.get(key.0) { + fn value_for<'a>(&'a self, key: MultiNamedPlaceholderKey<'k>) -> Self::W<'a> { + match self.get(key.0) { Some(value) => Ok(value), None => Err(MissingNamedPlaceholderError { name: key.0 }), - }; - (writeable, crate::PATTERN_PLACEHOLDER_PART) + } + } + #[inline] + fn map_literal<'a, 'l>(&'a self, literal: &'l str) -> Self::L<'a, 'l> { + literal } } diff --git a/components/pattern/src/single.rs b/components/pattern/src/single.rs index 9f74913b317..70464f00fdd 100644 --- a/components/pattern/src/single.rs +++ b/components/pattern/src/single.rs @@ -65,16 +65,21 @@ where W: Writeable, { type Error = Infallible; - type W<'a> - = WriteableAsTryWriteableInfallible<&'a W> + + type W<'a> = WriteableAsTryWriteableInfallible<&'a W> + where + Self: 'a; + + type L<'a, 'l> = &'l str where - W: 'a; - const LITERAL_PART: writeable::Part = crate::PATTERN_LITERAL_PART; - fn value_for(&self, _key: SinglePlaceholderKey) -> (Self::W<'_>, writeable::Part) { - ( - WriteableAsTryWriteableInfallible(&self.0), - crate::PATTERN_PLACEHOLDER_PART, - ) + Self: 'a; + + fn value_for(&self, _key: SinglePlaceholderKey) -> Self::W<'_> { + WriteableAsTryWriteableInfallible(&self.0) + } + #[inline] + fn map_literal<'a, 'l>(&'a self, literal: &'l str) -> Self::L<'a, 'l> { + literal } } @@ -83,17 +88,22 @@ where W: Writeable, { type Error = Infallible; - type W<'a> - = WriteableAsTryWriteableInfallible<&'a W> + + type W<'a> = WriteableAsTryWriteableInfallible<&'a W> + where + Self: 'a; + + type L<'a, 'l> = &'l str where - W: 'a; - const LITERAL_PART: writeable::Part = crate::PATTERN_LITERAL_PART; - fn value_for(&self, _key: SinglePlaceholderKey) -> (Self::W<'_>, writeable::Part) { + Self: 'a; + + fn value_for(&self, _key: SinglePlaceholderKey) -> Self::W<'_> { let [value] = self; - ( - WriteableAsTryWriteableInfallible(value), - crate::PATTERN_PLACEHOLDER_PART, - ) + WriteableAsTryWriteableInfallible(value) + } + #[inline] + fn map_literal<'a, 'l>(&'a self, literal: &'l str) -> Self::L<'a, 'l> { + literal } } diff --git a/ffi/capi/tests/missing_apis.txt b/ffi/capi/tests/missing_apis.txt index 8e82c849e7c..8dcc9a06093 100644 --- a/ffi/capi/tests/missing_apis.txt +++ b/ffi/capi/tests/missing_apis.txt @@ -487,6 +487,7 @@ icu::normalizer::uts46::Uts46MapperBorrowed::normalize_validate#FnInStruct icu::pattern::DoublePlaceholder#Enum icu::pattern::DoublePlaceholderKey#Enum icu::pattern::DoublePlaceholderKey::from_str#FnInEnum +icu::pattern::DoublePlaceholderKey::map_literal#FnInEnum icu::pattern::DoublePlaceholderKey::value_for#FnInEnum icu::pattern::DoublePlaceholderPattern#Typedef icu::pattern::Error#Enum @@ -494,10 +495,9 @@ icu::pattern::MissingNamedPlaceholderError#Struct icu::pattern::MissingNamedPlaceholderError::write_to#FnInStruct icu::pattern::MultiNamedPlaceholder#Enum icu::pattern::MultiNamedPlaceholderKey#Struct +icu::pattern::MultiNamedPlaceholderKey::map_literal#FnInStruct icu::pattern::MultiNamedPlaceholderKey::value_for#FnInStruct icu::pattern::MultiNamedPlaceholderPattern#Typedef -icu::pattern::PATTERN_LITERAL_PART#Constant -icu::pattern::PATTERN_PLACEHOLDER_PART#Constant icu::pattern::ParsedPatternItem#Enum icu::pattern::Parser#Struct icu::pattern::Parser::new#FnInStruct @@ -518,11 +518,13 @@ icu::pattern::PatternError#Enum icu::pattern::PatternItem#Enum icu::pattern::PatternItemCow#Enum icu::pattern::PatternString#Struct +icu::pattern::PlaceholderValueProvider::map_literal#FnInTrait icu::pattern::PlaceholderValueProvider::value_for#FnInTrait icu::pattern::QuoteMode#Enum icu::pattern::SinglePlaceholder#Enum icu::pattern::SinglePlaceholderKey#Enum icu::pattern::SinglePlaceholderKey::from_str#FnInEnum +icu::pattern::SinglePlaceholderKey::map_literal#FnInEnum icu::pattern::SinglePlaceholderKey::value_for#FnInEnum icu::pattern::SinglePlaceholderPattern#Typedef icu::plurals::PluralElements#Struct