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.

Sign up for GitHub

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

Jump to bottom

Readability improvements #4928

Closed
wants to merge 5 commits into from
Closed

Readability improvements #4928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
196172d
wip
robertbastian May 22, 2024
a19d8fc
fix
robertbastian May 23, 2024
7ac814b
less allocating
robertbastian May 24, 2024
a412d5d
mo
robertbastian May 24, 2024
4641030
Merge branch 'main' into turbo
robertbastian May 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/calendar/src/duration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl<C: Calendar + ?Sized> DateDuration<C> {
/// ```rust
/// # use icu::calendar::*;
/// // two years, three months, and five days
/// let duration: DateDuration<Iso> = DateDuration::new(2, 3, 0, 5);
/// let duration = DateDuration::<Iso>::new(2, 3, 0, 5);
/// ```
pub fn new(years: i32, months: i32, weeks: i32, days: i32) -> Self {
DateDuration {
Expand Down
9 changes: 6 additions & 3 deletions components/calendar/src/provider.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,12 @@ impl serde::Serialize for WeekdaySet {
S: serde::Serializer,
{
if serializer.is_human_readable() {
crate::week_of::WeekdaySetIterator::new(IsoWeekday::Monday, *self)
.collect::<alloc::vec::Vec<_>>()
.serialize(serializer)
use serde::ser::SerializeSeq;
let mut seq = serializer.serialize_seq(None)?;
for day in crate::week_of::WeekdaySetIterator::new(IsoWeekday::Monday, *self) {
seq.serialize_element(&day)?;
}
seq.end()
} else {
self.0.serialize(serializer)
}
Expand Down
44 changes: 24 additions & 20 deletions components/calendar/src/week_of.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,34 +714,38 @@ fn test_weekend() {
use icu_locid::locale;

assert_eq!(
WeekCalculator::try_new(&locale!("und").into())
.unwrap()
.weekend()
.collect::<Vec<_>>(),
Vec::from_iter(
WeekCalculator::try_new(&locale!("und").into())
.unwrap()
.weekend()
),
Comment on lines +717 to +721
Copy link
Member

Choose a reason for hiding this comment

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

Here's a case where we need to increase indentation.

// Max indent 3
        WeekCalculator::try_new(&locale!("und").into())
            .unwrap()
            .weekend()
            .collect::<Vec<_>>(),

// Max indent 4
        Vec::from_iter(
            WeekCalculator::try_new(&locale!("und").into())
                .unwrap()
                .weekend()
        ),

vec![IsoWeekday::Saturday, IsoWeekday::Sunday],
);

assert_eq!(
WeekCalculator::try_new(&locale!("und-FR").into())
.unwrap()
.weekend()
.collect::<Vec<_>>(),
Vec::from_iter(
WeekCalculator::try_new(&locale!("und-FR").into())
.unwrap()
.weekend()
),
vec![IsoWeekday::Saturday, IsoWeekday::Sunday],
);

assert_eq!(
WeekCalculator::try_new(&locale!("und-IQ").into())
.unwrap()
.weekend()
.collect::<Vec<_>>(),
Vec::from_iter(
WeekCalculator::try_new(&locale!("und-IQ").into())
.unwrap()
.weekend()
),
vec![IsoWeekday::Saturday, IsoWeekday::Friday],
);

assert_eq!(
WeekCalculator::try_new(&locale!("und-IR").into())
.unwrap()
.weekend()
.collect::<Vec<_>>(),
Vec::from_iter(
WeekCalculator::try_new(&locale!("und-IR").into())
.unwrap()
.weekend()
),
vec![IsoWeekday::Friday],
);
}
Expand All @@ -752,11 +756,11 @@ fn test_weekdays_iter() {

// Weekend ends one day before week starts
let default_weekend = WeekdaySetIterator::new(Monday, WeekdaySet::new(&[Saturday, Sunday]));
assert_eq!(vec![Saturday, Sunday], default_weekend.collect::<Vec<_>>());
assert_eq!(vec![Saturday, Sunday], Vec::from_iter(default_weekend));

// Non-contiguous weekend
let fri_sun_weekend = WeekdaySetIterator::new(Monday, WeekdaySet::new(&[Friday, Sunday]));
assert_eq!(vec![Friday, Sunday], fri_sun_weekend.collect::<Vec<_>>());
assert_eq!(vec![Friday, Sunday], Vec::from_iter(fri_sun_weekend));

let multiple_contiguous_days = WeekdaySetIterator::new(
Monday,
Expand All @@ -769,7 +773,7 @@ fn test_weekdays_iter() {
);
assert_eq!(
vec![Tuesday, Wednesday, Thursday, Friday],
multiple_contiguous_days.collect::<Vec<_>>()
Vec::from_iter(multiple_contiguous_days)
);

// Non-contiguous days and iterator yielding elements based off first_weekday
Expand All @@ -784,6 +788,6 @@ fn test_weekdays_iter() {
);
assert_eq!(
vec![Thursday, Friday, Sunday, Tuesday],
multiple_non_contiguous_days.collect::<Vec<_>>()
Vec::from_iter(multiple_non_contiguous_days)
);
}
13 changes: 5 additions & 8 deletions components/casemap/src/provider/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,11 @@ impl<'data> CaseMapV1<'data> {
let trie_index = ZeroVec::alloc_from_slice(trie_index);

#[allow(clippy::unwrap_used)] // datagen only
let trie_data = trie_data
.iter()
.map(|&i| {
CaseMapData::try_from_icu_integer(i)
.unwrap()
.with_updated_exception(&idx_map)
})
.collect::<ZeroVec<_>>();
let trie_data = ZeroVec::from_iter(trie_data.iter().map(|&i| {
CaseMapData::try_from_icu_integer(i)
.unwrap()
.with_updated_exception(&idx_map)
}));

let trie = CodePointTrie::try_new(trie_header, trie_index, trie_data)
.map_err(|_| DataError::custom("Casemapping data does not form valid trie"))?;
Expand Down
16 changes: 8 additions & 8 deletions components/casemap/src/titlecase.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ use writeable::Writeable;
/// let cm = TitlecaseMapper::new();
/// let root = langid!("und");
///
/// let default_options = Default::default();
/// let mut preserve_case: TitlecaseOptions = Default::default();
/// let default_options = TitlecaseOptions::default();
/// let mut preserve_case = TitlecaseOptions::default();
/// preserve_case.trailing_case = TrailingCase::Unchanged;
///
/// // Exhibits trailing case when set:
Expand Down Expand Up @@ -72,8 +72,8 @@ pub enum TrailingCase {
/// let root = langid!("und");
///
/// let default_options = Default::default(); // head adjustment set to Auto
/// let mut no_adjust: TitlecaseOptions = Default::default();
/// let mut adjust_to_cased: TitlecaseOptions = Default::default();
/// let mut no_adjust = TitlecaseOptions::default();
/// let mut adjust_to_cased = TitlecaseOptions::default();
/// no_adjust.leading_adjustment = LeadingAdjustment::None;
/// adjust_to_cased.leading_adjustment = LeadingAdjustment::ToCased;
///
Expand Down Expand Up @@ -383,8 +383,8 @@ impl<CM: AsRef<CaseMapper>> TitlecaseMapper<CM> {
/// let cm = TitlecaseMapper::new();
/// let root = langid!("und");
///
/// let default_options = Default::default();
/// let mut no_adjust: TitlecaseOptions = Default::default();
/// let default_options = TitlecaseOptions::default();
/// let mut no_adjust = TitlecaseOptions::default();
/// no_adjust.leading_adjustment = LeadingAdjustment::None;
///
/// // Exhibits leading adjustment when set:
Expand Down Expand Up @@ -423,8 +423,8 @@ impl<CM: AsRef<CaseMapper>> TitlecaseMapper<CM> {
/// let cm = TitlecaseMapper::new();
/// let root = langid!("und");
///
/// let default_options = Default::default();
/// let mut preserve_case: TitlecaseOptions = Default::default();
/// let default_options = TitlecaseOptions::default();
/// let mut preserve_case = TitlecaseOptions::default();
/// preserve_case.trailing_case = TrailingCase::Unchanged;
///
/// // Exhibits trailing case when set:
Expand Down
8 changes: 4 additions & 4 deletions components/collator/README.md
View file
Open in desktop

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

104 changes: 56 additions & 48 deletions components/collator/benches/bench.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,69 +17,77 @@ fn to_data_locale(locale_str: &str) -> DataLocale {

pub fn collator_with_locale(criterion: &mut Criterion) {
// Load file content in reverse order vector.
let content_latin: (&str, Vec<&str>) = (
let content_latin = (
"TestNames_Latin",
include_str!("data/TestNames_Latin.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev()
.collect::<Vec<&str>>(),
Vec::from_iter(
include_str!("data/TestNames_Latin.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev(),
),
);
let content_asian: (&str, Vec<&str>) = (
let content_asian = (
"TestNames_Asian",
include_str!("data/TestNames_Asian.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev()
.collect(),
Vec::from_iter(
include_str!("data/TestNames_Asian.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev(),
),
);
let content_russian: (&str, Vec<&str>) = (
let content_russian = (
"TestNames_Russian",
include_str!("data/TestNames_Russian.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev()
.collect(),
Vec::from_iter(
include_str!("data/TestNames_Russian.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev(),
),
);
let content_chinese: (&str, Vec<&str>) = (
let content_chinese = (
"TestNames_Chinese",
include_str!("data/TestNames_Chinese.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev()
.collect(),
Vec::from_iter(
include_str!("data/TestNames_Chinese.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev(),
),
);
let content_jp_h: (&str, Vec<&str>) = (
let content_jp_h = (
"TestNames_Japanese_h",
include_str!("data/TestNames_Japanese_h.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev()
.collect::<Vec<&str>>(),
Vec::from_iter(
include_str!("data/TestNames_Japanese_h.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev(),
),
);
let content_jp_k: (&str, Vec<&str>) = (
let content_jp_k = (
"TestNames_Japanese_k",
include_str!("data/TestNames_Japanese_k.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev()
.collect::<Vec<&str>>(),
Vec::from_iter(
include_str!("data/TestNames_Japanese_k.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev(),
),
);
let content_korean: (&str, Vec<&str>) = (
let content_korean = (
"TestNames_Korean",
include_str!("data/TestNames_Korean.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev()
.collect::<Vec<&str>>(),
Vec::from_iter(
include_str!("data/TestNames_Korean.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev(),
),
);
let content_thai: (&str, Vec<&str>) = (
let content_thai = (
"TestNames_Thai",
include_str!("data/TestNames_Thai.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev()
.collect::<Vec<&str>>(),
Vec::from_iter(
include_str!("data/TestNames_Thai.txt")
.lines()
.filter(|&s| !s.starts_with('#'))
.rev(),
),
);

// hsivonen@ : All five strengths are benched.
Expand Down
8 changes: 4 additions & 4 deletions components/collator/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@
//! let locale_es = locale!("es-u-co-trad").into();
//! let mut options = CollatorOptions::new();
//! options.strength = Some(Strength::Primary);
//! let collator_es: Collator = Collator::try_new(&locale_es, options).unwrap();
//! let collator_es = Collator::try_new(&locale_es, options).unwrap();
//!
//! // "pollo" > "polvo" in traditional Spanish
//! assert_eq!(collator_es.compare("pollo", "polvo"), Ordering::Greater);
//!
//! let locale_en = locale!("en").into();
//! let mut options = CollatorOptions::new();
//! options.strength = Some(Strength::Primary);
//! let collator_en: Collator = Collator::try_new(&locale_en, options).unwrap();
//! let collator_en = Collator::try_new(&locale_en, options).unwrap();
//!
//! // "pollo" < "polvo" according to English rules
//! assert_eq!(collator_en.compare("pollo", "polvo"), Ordering::Less);
Expand Down Expand Up @@ -254,15 +254,15 @@
//!
//! let mut options_num_off = CollatorOptions::new();
//! options_num_off.numeric = Some(Numeric::Off);
//! let collator_num_off: Collator =
//! let collator_num_off =
//! Collator::try_new(&Default::default(), options_num_off).unwrap();
//! assert_eq!(collator_num_off.compare("a10b", "a2b"), Ordering::Less);
//!
//! // Numerical sorting on
//!
//! let mut options_num_on = CollatorOptions::new();
//! options_num_on.numeric = Some(Numeric::On);
//! let collator_num_on: Collator =
//! let collator_num_on =
//! Collator::try_new(&Default::default(), options_num_on).unwrap();
//! assert_eq!(collator_num_on.compare("a10b", "a2b"), Ordering::Greater);
//! ```
Expand Down
Loading
Loading