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

More foo32 consistency #5619

Merged
merged 2 commits into from
Oct 2, 2024
Merged

More foo32 consistency #5619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ca45bc
u32
robertbastian Sep 30, 2024
3adcb98
fix
robertbastian Oct 1, 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
36 changes: 18 additions & 18 deletions components/collections/src/codepointinvliststringlist/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,17 +142,17 @@ impl<'data> CodePointInversionListAndStringList<'data> {
///
/// let cpilsl = CodePointInversionListAndStringList::try_from(cp_list, str_list).unwrap();
///
/// assert!(cpilsl.contains("bmp_max"));
/// assert!(cpilsl.contains(""));
/// assert!(cpilsl.contains("A"));
/// assert!(cpilsl.contains("ቔ")); // U+1254 ETHIOPIC SYLLABLE QHEE
/// assert!(!cpilsl.contains("bazinga!"));
/// assert!(cpilsl.contains_str("bmp_max"));
/// assert!(cpilsl.contains_str(""));
/// assert!(cpilsl.contains_str("A"));
/// assert!(cpilsl.contains_str("ቔ")); // U+1254 ETHIOPIC SYLLABLE QHEE
/// assert!(!cpilsl.contains_str("bazinga!"));
/// ```
pub fn contains(&self, s: &str) -> bool {
pub fn contains_str(&self, s: &str) -> bool {
let mut chars = s.chars();
if let Some(first_char) = chars.next() {
if chars.next().is_none() {
return self.contains_char(first_char);
return self.contains(first_char);
}
}
self.str_list.binary_search(s).is_ok()
Expand Down Expand Up @@ -196,11 +196,11 @@ impl<'data> CodePointInversionListAndStringList<'data> {
///
/// let cpilsl = CodePointInversionListAndStringList::try_from(cp_list, str_list).unwrap();
///
/// assert!(cpilsl.contains_char('A'));
/// assert!(cpilsl.contains_char('ቔ')); // U+1254 ETHIOPIC SYLLABLE QHEE
/// assert!(!cpilsl.contains_char('\u{1_0000}'));
/// assert!(!cpilsl.contains_char('🨫')); // U+1FA2B NEUTRAL CHESS TURNED QUEEN
pub fn contains_char(&self, ch: char) -> bool {
/// assert!(cpilsl.contains('A'));
/// assert!(cpilsl.contains('ቔ')); // U+1254 ETHIOPIC SYLLABLE QHEE
/// assert!(!cpilsl.contains('\u{1_0000}'));
/// assert!(!cpilsl.contains('🨫')); // U+1FA2B NEUTRAL CHESS TURNED QUEEN
pub fn contains(&self, ch: char) -> bool {
self.contains32(ch as u32)
}

Expand Down Expand Up @@ -353,14 +353,14 @@ mod tests {
assert_eq!(cpilsl_1, cpilsl_2);

assert!(cpilsl_1.has_strings());
assert!(cpilsl_1.contains("abc"));
assert!(cpilsl_1.contains("xyz"));
assert!(!cpilsl_1.contains("def"));
assert!(cpilsl_1.contains_str("abc"));
assert!(cpilsl_1.contains_str("xyz"));
assert!(!cpilsl_1.contains_str("def"));

assert_eq!(1, cpilsl_1.cp_inv_list.size());
assert!(cpilsl_1.contains_char('a'));
assert!(!cpilsl_1.contains_char('0'));
assert!(!cpilsl_1.contains_char('q'));
assert!(cpilsl_1.contains('a'));
assert!(!cpilsl_1.contains('0'));
assert!(!cpilsl_1.contains('q'));

assert_eq!(3, cpilsl_1.size());
}
Expand Down
2 changes: 1 addition & 1 deletion components/experimental/src/personnames/specifications/derive_locale.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ where
.filter(|&name_field| name_field.kind == kind)
.find_map(|&name_field| {
person_name.get(name_field).chars().find_map(|c| {
let char_script = swe.get_script_val(c as u32);
let char_script = swe.get_script_val(c);
match char_script {
Script::Common | Script::Unknown | Script::Inherited => None,
_ => Some(char_script),
Expand Down
12 changes: 6 additions & 6 deletions components/experimental/src/transliterate/transliterator/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -922,10 +922,10 @@ impl<'a> SpecialMatcher<'a> {
// eprintln!("checking if set {set:?} matches input {matcher:?}");

if matcher.is_empty() {
if set.contains("") {
if set.contains_str("") {
return true;
}
if set.contains("\u{FFFF}") {
if set.contains_str("\u{FFFF}") {
if matcher.match_end_anchor() {
return true;
}
Expand Down Expand Up @@ -961,7 +961,7 @@ impl<'a> SpecialMatcher<'a> {

if let Some(input_c) = matcher.next_char() {
// eprintln!("checking if set {set:?} contains char {input_c:?}");
if set.contains_char(input_c) {
if set.contains(input_c) {
// eprintln!("contains!");
return matcher.consume(input_c.len_utf8());
}
Expand Down Expand Up @@ -1025,10 +1025,10 @@ impl<'a> SpecialMatcher<'a> {
// eprintln!("checking if set {set:?} reverse matches input {matcher:?}");

if matcher.is_empty() {
if set.contains("") {
if set.contains_str("") {
return true;
}
if set.contains("\u{FFFF}") {
if set.contains_str("\u{FFFF}") {
if matcher.match_end_anchor() {
return true;
}
Expand All @@ -1055,7 +1055,7 @@ impl<'a> SpecialMatcher<'a> {

if let Some(input_c) = matcher.next_char() {
// eprintln!("checking if set {set:?} contains char {input_c:?}");
if set.contains_char(input_c) {
if set.contains(input_c) {
// eprintln!("contains!");
return matcher.consume(input_c.len_utf8());
}
Expand Down
6 changes: 3 additions & 3 deletions components/experimental/src/unicodeset_parse/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1484,7 +1484,7 @@ where
///
/// let (set, _) =
/// parse(r"[[a-z{hello\ world}]&[^a-y{hello\ world}]]").unwrap();
/// assert!(set.contains_char('z'));
/// assert!(set.contains('z'));
/// assert_eq!(set.size(), 1);
/// assert!(!set.has_strings());
/// ```
Expand Down Expand Up @@ -1539,7 +1539,7 @@ pub fn parse(source: &str) -> Result<(CodePointInversionListAndStringList<'stati
/// let (set, consumed) = parse_with_variables(source, &variable_map).unwrap();
/// assert_eq!(consumed, source.len());
/// assert!(set.code_points().contains_range('d'..='z'));
/// assert!(set.contains("Hello World"));
/// assert!(set.contains_str("Hello World"));
/// assert_eq!(set.size(), 1 + ('d'..='z').count());
#[cfg(feature = "compiled_data")]
pub fn parse_with_variables(
Expand Down Expand Up @@ -1788,7 +1788,7 @@ mod tests {
for s in strings {
expected_size += 1;
assert!(
cpinvlistandstrlist.contains(s),
cpinvlistandstrlist.contains_str(s),
"missing string \"{}\" from parsed set \"{}\"",
s.escape_debug(),
source.escape_debug()
Expand Down
74 changes: 37 additions & 37 deletions components/locale/src/exemplar_chars.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
//! let exemplars_main = ExemplarCharacters::try_new_main(&locale)
//! .expect("locale should be present");
//!
//! assert!(exemplars_main.contains_char('a'));
//! assert!(exemplars_main.contains_char('z'));
//! assert!(exemplars_main.contains("a"));
//! assert!(!exemplars_main.contains("ä"));
//! assert!(!exemplars_main.contains("ng"));
//! assert!(exemplars_main.contains('a'));
//! assert!(exemplars_main.contains('z'));
//! assert!(exemplars_main.contains_str("a"));
//! assert!(!exemplars_main.contains_str("ä"));
//! assert!(!exemplars_main.contains_str("ng"));
//! ```

use crate::provider::*;
Expand Down Expand Up @@ -152,12 +152,12 @@ impl ExemplarCharacters {
/// let exemplars_main = ExemplarCharacters::try_new_main(&locale!("en").into())
/// .expect("locale should be present");
///
/// assert!(exemplars_main.contains_char('a'));
/// assert!(exemplars_main.contains_char('z'));
/// assert!(exemplars_main.contains("a"));
/// assert!(!exemplars_main.contains("ä"));
/// assert!(!exemplars_main.contains("ng"));
/// assert!(!exemplars_main.contains("A"));
/// assert!(exemplars_main.contains('a'));
/// assert!(exemplars_main.contains('z'));
/// assert!(exemplars_main.contains_str("a"));
/// assert!(!exemplars_main.contains_str("ä"));
/// assert!(!exemplars_main.contains_str("ng"));
/// assert!(!exemplars_main.contains_str("A"));
/// ```
pub fn try_new_main();
);
Expand All @@ -184,12 +184,12 @@ impl ExemplarCharacters {
/// ExemplarCharacters::try_new_auxiliary(&locale!("en").into())
/// .expect("locale should be present");
///
/// assert!(!exemplars_auxiliary.contains_char('a'));
/// assert!(!exemplars_auxiliary.contains_char('z'));
/// assert!(!exemplars_auxiliary.contains("a"));
/// assert!(exemplars_auxiliary.contains("ä"));
/// assert!(!exemplars_auxiliary.contains("ng"));
/// assert!(!exemplars_auxiliary.contains("A"));
/// assert!(!exemplars_auxiliary.contains('a'));
/// assert!(!exemplars_auxiliary.contains('z'));
/// assert!(!exemplars_auxiliary.contains_str("a"));
/// assert!(exemplars_auxiliary.contains_str("ä"));
/// assert!(!exemplars_auxiliary.contains_str("ng"));
/// assert!(!exemplars_auxiliary.contains_str("A"));
/// ```
pub fn try_new_auxiliary();
);
Expand All @@ -216,13 +216,13 @@ impl ExemplarCharacters {
/// ExemplarCharacters::try_new_punctuation(&locale!("en").into())
/// .expect("locale should be present");
///
/// assert!(!exemplars_punctuation.contains_char('0'));
/// assert!(!exemplars_punctuation.contains_char('9'));
/// assert!(!exemplars_punctuation.contains_char('%'));
/// assert!(exemplars_punctuation.contains_char(','));
/// assert!(exemplars_punctuation.contains_char('.'));
/// assert!(exemplars_punctuation.contains_char('!'));
/// assert!(exemplars_punctuation.contains_char('?'));
/// assert!(!exemplars_punctuation.contains('0'));
/// assert!(!exemplars_punctuation.contains('9'));
/// assert!(!exemplars_punctuation.contains('%'));
/// assert!(exemplars_punctuation.contains(','));
/// assert!(exemplars_punctuation.contains('.'));
/// assert!(exemplars_punctuation.contains('!'));
/// assert!(exemplars_punctuation.contains('?'));
/// ```
pub fn try_new_punctuation();
);
Expand All @@ -249,13 +249,13 @@ impl ExemplarCharacters {
/// ExemplarCharacters::try_new_numbers(&locale!("en").into())
/// .expect("locale should be present");
///
/// assert!(exemplars_numbers.contains_char('0'));
/// assert!(exemplars_numbers.contains_char('9'));
/// assert!(exemplars_numbers.contains_char('%'));
/// assert!(exemplars_numbers.contains_char(','));
/// assert!(exemplars_numbers.contains_char('.'));
/// assert!(!exemplars_numbers.contains_char('!'));
/// assert!(!exemplars_numbers.contains_char('?'));
/// assert!(exemplars_numbers.contains('0'));
/// assert!(exemplars_numbers.contains('9'));
/// assert!(exemplars_numbers.contains('%'));
/// assert!(exemplars_numbers.contains(','));
/// assert!(exemplars_numbers.contains('.'));
/// assert!(!exemplars_numbers.contains('!'));
/// assert!(!exemplars_numbers.contains('?'));
/// ```
pub fn try_new_numbers();
);
Expand All @@ -282,12 +282,12 @@ impl ExemplarCharacters {
/// ExemplarCharacters::try_new_index(&locale!("en").into())
/// .expect("locale should be present");
///
/// assert!(!exemplars_index.contains_char('a'));
/// assert!(!exemplars_index.contains_char('z'));
/// assert!(!exemplars_index.contains("a"));
/// assert!(!exemplars_index.contains("ä"));
/// assert!(!exemplars_index.contains("ng"));
/// assert!(exemplars_index.contains("A"));
/// assert!(!exemplars_index.contains('a'));
/// assert!(!exemplars_index.contains('z'));
/// assert!(!exemplars_index.contains_str("a"));
/// assert!(!exemplars_index.contains_str("ä"));
/// assert!(!exemplars_index.contains_str("ng"));
/// assert!(exemplars_index.contains_str("A"));
/// ```
pub fn try_new_index();
);
Expand Down
4 changes: 2 additions & 2 deletions components/properties/README.md
View file
Open in desktop

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

14 changes: 1 addition & 13 deletions components/properties/src/code_point_map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,19 +162,7 @@ impl<'a, T: TrieValue> CodePointMapDataBorrowed<'a, T> {
self.map.get32(ch as u32)
}

/// Get the value this map has associated with code point `ch`
///
/// # Example
///
/// ```
/// use icu::properties::CodePointMapData;
/// use icu::properties::props::GeneralCategory;
///
/// let gc = CodePointMapData::<GeneralCategory>::new();
///
/// assert_eq!(gc.get32(0x6728), GeneralCategory::OtherLetter); // U+6728 (木)
/// assert_eq!(gc.get32(0x1F383), GeneralCategory::OtherSymbol); // U+1F383 JACK-O-LANTERN
/// ```
/// See [`Self::get`].
pub fn get32(self, ch: u32) -> T {
self.map.get32(ch)
}
Expand Down
12 changes: 1 addition & 11 deletions components/properties/src/code_point_set.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,17 +142,7 @@ impl<'a> CodePointSetDataBorrowed<'a> {
self.set.contains(ch)
}

/// Check if the set contains a character as a UTF32 code unit
///
/// ```rust
/// use icu::properties::CodePointSetData;
/// use icu::properties::props::Alphabetic;
///
/// let alphabetic = CodePointSetData::new::<Alphabetic>();
///
/// assert!(!alphabetic.contains32(0x0A69)); // U+0A69 GURMUKHI DIGIT THREE
/// assert!(alphabetic.contains32(0x00C4)); // U+00C4 LATIN CAPITAL LETTER A WITH DIAERESIS
/// ```
/// See [`Self::contains`].
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion, here and elsewhere: Previously the docs said "as a UTF32 code unit" but now it doesn't say that any more.

Suggested change
/// See [`Self::contains`].
/// Returns whether the set contains a code point represented as a UTF-32 code unit.
///
/// For an example, see [`Self::contains`].

Copy link
Member Author

Choose a reason for hiding this comment

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

A u32 is not a UTF-32 code unit though, this was incorrect.

Copy link
Member

Choose a reason for hiding this comment

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

Huh?

It's important to say that the u32 is a UTF-32 code unit. Otherwise, I don't know what it is: it could be a zero-padded ASCII, a Latin1 byte, a TinyStr, …

Copy link
Member

Choose a reason for hiding this comment

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

If you're bothered by "not all u32s are code points", you could stick our agreed-upon term "potential" in there somewhere

Copy link
Member Author

Choose a reason for hiding this comment

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

Please just approve this PR as this is blocking another PR. I can improve docs for all 32 methods in a follow-up.

#[inline]
pub fn contains32(self, ch: u32) -> bool {
self.set.contains32(ch)
Expand Down
4 changes: 2 additions & 2 deletions components/properties/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
//! .get_set_for_value(GeneralCategory::LineSeparator);
//! let line_sep = line_sep_data.as_borrowed();
//!
//! assert!(line_sep.contains32(0x2028));
//! assert!(!line_sep.contains32(0x2029));
//! assert!(line_sep.contains('\u{2028}'));
//! assert!(!line_sep.contains('\u{2029}'));
//! ```
//!
//! ## Property data as `CodePointMapData`s
Expand Down
Loading