Skip to content

Commit

Permalink
Apply feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
zbraniecki committed Nov 5, 2024
1 parent dc99560 commit b64b52a
Show file tree
Hide file tree
Showing 15 changed files with 260 additions and 397 deletions.
10 changes: 6 additions & 4 deletions components/experimental/src/duration/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl From<BaseStyle> for icu_list::ListFormatterOptions {
BaseStyle::Short | BaseStyle::Digital => ListLength::Short,
BaseStyle::Narrow => ListLength::Narrow,
};
Self::new().with_length(length)
Self::default().with_length(length)
}
}

Expand Down Expand Up @@ -195,11 +195,12 @@ impl DurationFormatter {
})?
.payload;

let temp_loc = locale.clone().into_locale();
Ok(Self {
digital,
options,
unit: DurationUnitFormatter::try_new(locale, options)?,
list: ListFormatter::try_new_unit_with_length(locale.into(), options.base.into())?,
list: ListFormatter::try_new_unit(temp_loc.into(), options.base.into())?,
fdf: FixedDecimalFormatter::try_new(locale, Default::default())?,
})
}
Expand All @@ -224,13 +225,14 @@ impl DurationFormatter {
})?
.payload;

let temp_loc = locale.clone().into_locale();
Ok(Self {
digital,
options,
unit: DurationUnitFormatter::try_new_unstable(provider, locale, options)?,
list: ListFormatter::try_new_unit_with_length_unstable(
list: ListFormatter::try_new_unit_unstable(
provider,
locale.into(),
temp_loc.into(),
options.base.into(),
)?,
fdf: FixedDecimalFormatter::try_new_unstable(provider, locale, Default::default())?,
Expand Down
12 changes: 6 additions & 6 deletions components/list/README.md

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

4 changes: 2 additions & 2 deletions components/list/examples/and_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use icu::list::{ListFormatter, ListFormatterOptions, ListLength};
use icu::locale::locale;

fn main() {
let list_formatter = ListFormatter::try_new_and_with_length(
let list_formatter = ListFormatter::try_new_and(
locale!("es").into(),
ListFormatterOptions::new().with_length(ListLength::Wide),
ListFormatterOptions::default().with_length(ListLength::Wide),
)
.unwrap();

Expand Down
12 changes: 6 additions & 6 deletions components/list/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
//! # use icu::locale::locale;
//! # use writeable::*;
//! #
//! let list_formatter = ListFormatter::try_new_and_with_length(
//! let list_formatter = ListFormatter::try_new_and(
//! locale!("es").into(),
//! ListFormatterOptions::new()
//! ListFormatterOptions::default()
//! .with_length(ListLength::Wide)
//! )
//! .expect("locale should be present");
Expand All @@ -42,9 +42,9 @@
//! # use icu::locale::locale;
//! # use writeable::*;
//! #
//! let list_formatter = ListFormatter::try_new_or_with_length(
//! let list_formatter = ListFormatter::try_new_or(
//! locale!("th").into(),
//! ListFormatterOptions::new()
//! ListFormatterOptions::default()
//! .with_length(ListLength::Short)
//! )
//! .expect("locale should be present");
Expand All @@ -60,9 +60,9 @@
//! # use icu::locale::locale;
//! # use writeable::*;
//! #
//! let list_formatter = ListFormatter::try_new_unit_with_length(
//! let list_formatter = ListFormatter::try_new_unit(
//! locale!("en").into(),
//! ListFormatterOptions::new()
//! ListFormatterOptions::default()
//! .with_length(ListLength::Wide)
//! )
//! .expect("locale should be present");
Expand Down
54 changes: 25 additions & 29 deletions components/list/src/list_formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,38 +74,38 @@ macro_rules! constructor {
}

fn get_data_locale_from_prefs(prefs: ListFormatterPreferences) -> DataLocale {
// XXX: This should utilize region source priority.
// TODO(#5764): This should utilize region source priority.
DataLocale::from_subtags(
prefs.language,
prefs.script,
prefs.region,
prefs.variant,
prefs.subdivision,
prefs.locale_prefs.language,
prefs.locale_prefs.script,
prefs.locale_prefs.region,
prefs.locale_prefs.variant,
prefs.locale_prefs.subdivision,
)
}

impl ListFormatter {
constructor!(
try_new_and_with_length,
try_new_and_with_length_with_any_provider,
try_new_and_with_length_with_buffer_provider,
try_new_and_with_length_unstable,
try_new_and,
try_new_and_with_any_provider,
try_new_and_with_buffer_provider,
try_new_and_unstable,
AndListV2Marker,
"and"
);
constructor!(
try_new_or_with_length,
try_new_or_with_length_with_any_provider,
try_new_or_with_length_with_buffer_provider,
try_new_or_with_length_unstable,
try_new_or,
try_new_or_with_any_provider,
try_new_or_with_buffer_provider,
try_new_or_unstable,
OrListV2Marker,
"or"
);
constructor!(
try_new_unit_with_length,
try_new_unit_with_length_with_any_provider,
try_new_unit_with_length_with_buffer_provider,
try_new_unit_with_length_unstable,
try_new_unit,
try_new_unit_with_any_provider,
try_new_unit_with_buffer_provider,
try_new_unit_unstable,
UnitListV2Marker,
"unit"
);
Expand All @@ -122,9 +122,9 @@ impl ListFormatter {
/// use icu::list::*;
/// # use icu::locale::locale;
/// # use writeable::*;
/// let formatteur = ListFormatter::try_new_and_with_length(
/// let formatteur = ListFormatter::try_new_and(
/// locale!("fr").into(),
/// ListFormatterOptions::new()
/// ListFormatterOptions::default()
/// .with_length(ListLength::Wide)
/// )
/// .unwrap();
Expand Down Expand Up @@ -388,14 +388,14 @@ mod tests {

#[test]
fn test_basic() {
test!("fr", try_new_or_with_length, (["A", "B"], "A ou B"),);
test!("fr", try_new_or, (["A", "B"], "A ou B"),);
}

#[test]
fn test_spanish() {
test!(
"es",
try_new_and_with_length,
try_new_and,
(["x", "Mallorca"], "x y Mallorca"),
(["x", "Ibiza"], "x e Ibiza"),
(["x", "Hidalgo"], "x e Hidalgo"),
Expand All @@ -404,7 +404,7 @@ mod tests {

test!(
"es",
try_new_or_with_length,
try_new_or,
(["x", "Ibiza"], "x o Ibiza"),
(["x", "Okinawa"], "x u Okinawa"),
(["x", "8 más"], "x u 8 más"),
Expand All @@ -421,18 +421,14 @@ mod tests {
(["x", "11.000,92"], "x u 11.000,92"),
);

test!(
"es-AR",
try_new_and_with_length,
(["x", "Ibiza"], "x e Ibiza"),
);
test!("es-AR", try_new_and, (["x", "Ibiza"], "x e Ibiza"),);
}

#[test]
fn test_hebrew() {
test!(
"he",
try_new_and_with_length,
try_new_and,
(["x", "יפו"], "x ויפו"),
(["x", "Ibiza"], "x ו‑Ibiza"),
);
Expand Down
8 changes: 4 additions & 4 deletions components/list/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
/// ```
/// use icu::list::{ListFormatterOptions, ListLength};
///
/// let options = ListFormatterOptions::new()
/// let options = ListFormatterOptions::default()
/// .with_length(ListLength::Wide);
/// ```
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[non_exhaustive]
pub struct ListFormatterOptions {
/// The length variant should reflect available space for the list.
Expand All @@ -20,13 +20,13 @@ pub struct ListFormatterOptions {

impl Default for ListFormatterOptions {
fn default() -> Self {
Self::new()
Self::default()
}
}

impl ListFormatterOptions {
/// Constructs a new [`ListFormatterOptions`] struct.
pub const fn new() -> Self {
pub const fn default() -> Self {
Self { length: None }
}

Expand Down
13 changes: 2 additions & 11 deletions components/locale_core/src/extensions/unicode/subdivision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

use core::str::FromStr;

use tinystr::TinyAsciiStr;

use crate::parser::ParseError;
use crate::subtags::{Region, Subtag};

Expand Down Expand Up @@ -136,15 +134,8 @@ impl SubdivisionId {

/// Convert to [`Subtag`]
pub fn into_subtag(self) -> Subtag {
use writeable::Writeable;

// XXX: This can be optimized to concatenate two TinyAsciiStr.
let mut result = alloc::string::String::with_capacity(8);
let _ = self.write_to(&mut result);
#[allow(clippy::expect_used)]
let tinystr = TinyAsciiStr::try_from_str(&result)
.expect("Constructing 8 chars TinyAsciiStr from two 4 char ones");
Subtag::from_tinystr_unvalidated(tinystr)
let result = self.region.to_tinystr().concat(self.suffix.to_tinystr());
Subtag::from_tinystr_unvalidated(result)
}
}

Expand Down
Loading

0 comments on commit b64b52a

Please sign in to comment.