Create wrapper structs and interfaces for properties data structures and payloads #1333

echeran · 2021-11-19T21:29:37Z

sffc

Also, please remove the 4th commit from this PR.

sffc · 2021-11-19T21:39:31Z

CODEOWNERS

@@ -14,7 +14,7 @@ components/icu4x/                 @unicode-org/icu4x-owners
 components/locale_canonicalizer/  @dminor @zbraniecki
 components/locid/                 @zbraniecki @nciric
 components/plurals/               @zbraniecki @sffc
-components/uniset/                @echeran @iainireland
+components/codepointset/          echeran @iainireland


Please restore yourself

sffc · 2021-11-19T21:40:46Z

.github/workflows/build-test.yml

@@ -468,7 +468,7 @@ jobs:
          - components/locid
          - components/plurals
          - components/datetime
-          - utils/uniset
+          - utils/codepointset


Reminder to update the gh_pages branch

sffc · 2021-11-19T21:41:34Z

docs/process/prd.md

@@ -57,7 +57,7 @@ ICU4X 0.1’s selection of components aimed to:

 -   Validate low-level models around [data management](https://github.com/unicode-org/icu4x/blob/master/docs/design/data_pipeline.md)
 -   Establish project culture using simple building blocks such as [Locale](https://docs.rs/icu_locid/0.1.0/icu_locid/) and [Plural Rules](https://docs.rs/icu_plurals/0.1.0/icu_plurals/)
-   Introduce a low-level foundation for string operations with [UnicodeSet](https://docs.rs/icu_datetime/0.1.0/icu_datetime/)
+-   Introduce a low-level foundation for string operations with [UnicodeSet](https://docs.rs/icu_codepointset/0.1.0/icu_codepointset/)


Suggestion: Change this to go to icu_uniset/0.1.0

sffc · 2021-11-19T21:44:08Z

docs/process/prd.md

@@ -117,7 +117,7 @@ Mozilla currently maintains [its own segmentation engine](https://searchfox.org/

 Replacing lwbrk with ICU4C for layout needs would require a substantial effort to customize data management to handle Mozilla’s custom data.

-ICU4X offers an environment in which we are developing a new Unicode UAX#14/UAX#29 compatible segmentation API which will use the foundational [UnicodeSet API](https://docs.rs/icu_uniset/0.1.0/icu_uniset/), and fit the needs of both Layout and ECMA-402.
+ICU4X offers an environment in which we are developing a new Unicode UAX#14/UAX#29 compatible segmentation API which will use the foundational [CodePointSet API](https://docs.rs/icu_codepointset/0.1.0/icu_codepointset/), and fit the needs of both Layout and ECMA-402.


Either revert or link to the staging docs

sffc · 2021-11-19T21:45:46Z

docs/tutorials/writing_a_new_data_struct.md

@@ -33,7 +33,7 @@ The data struct definitions should live in the crate that uses them. By conventi

 - `icu::decimal::provider::DecimalSymbolsV1`
 - `icu::locale_canonicalizer::provider::LikelySubtagsV1`
- `icu::uniset::provider::UnicodePropertyV1`
+- `icu::codepointset::provider::UnicodePropertyV1`


Nit: This should be icu::properties anyway

sffc · 2021-11-19T21:49:29Z

utils/codepointset/src/lib.rs

-//! It is an implementation of the existing [ICU4C UnicodeSet API](https://unicode-org.github.io/icu-docs/apidoc/released/icu4c/classicu_1_1UnicodeSet.html).
+//! It is an implementation of the existing [ICU4C CodePointSet API](https://unicode-org.github.io/icu-docs/apidoc/released/icu4c/classicu_1_1CodePointSet.html).


Nit: This one should not change

sffc · 2021-11-19T21:55:16Z

components/properties/src/lib.rs

@@ -70,6 +66,9 @@

 #![cfg_attr(not(any(test, feature = "std")), no_std)]

+extern crate alloc;
+
+pub mod api_util;


Nit: Make the mod non-pub

sffc · 2021-11-19T21:58:17Z

components/properties/src/api_util.rs

+
+    fn contains_u32(&self, code_point: u32) -> bool;
+
+    // return type cannot be `impl ExactSizeIterator<Item = RangeInclusive<u32>>;`. 


The type needs to go the trait as an associated type. However, we should discuss whether we want this all in one trait or whether to split this into smaller traits.

pub trait CodePointSetLike { type Iter: ExactSizeIterator<Item = RangeInclusive<u32>> }

sffc · 2021-11-19T22:04:24Z

components/properties/src/api_util.rs

+        self.payload.get().inv_list.contains_u32(code_point)
+    }
+
+    fn iter_ranges(&self) -> Vec<u32> {


This becomes -> Self::Iter

sffc · 2021-11-19T22:04:37Z

components/properties/src/api_util.rs

+    fn iter_ranges(&self) -> Vec<u32>;
+}
+
+impl CodePointSetLike for CodePointSetData {


Add the fully qualified iterator type here

sffc · 2021-11-19T22:05:03Z

components/properties/src/lib.rs

@@ -70,6 +66,9 @@

 #![cfg_attr(not(any(test, feature = "std")), no_std)]

+extern crate alloc;


Don't forget to delete this after removing the Vec

sffc · 2021-11-19T22:07:21Z

components/properties/src/api_util.rs

+    pub payload: DataPayload<UnicodePropertyV1Marker>,
+}
+
+pub trait CodePointSetLike {


Issue: This trait should be in icu_codepointset

sffc · 2021-11-19T22:20:21Z

.github/workflows/build-test.yml

@@ -468,7 +468,7 @@ jobs:
          - components/locid
          - components/plurals
          - components/datetime
-          - utils/uniset
+          - utils/codepointset


Actually, let's NOT change the crate name. Only change the type name

sffc · 2021-11-19T22:23:32Z

components/icu/src/lib.rs

    //! properties. See the [`sets`] module for more details.
    //!
    //! APIs that return a [`CodePointTrie`] exist for certain enumerated properties. See the
    //! [`maps`] module for more details.
    //!
    //! # Examples
    //!
-    //! ## Property data as `UnicodeSet`s
+    //! ## Property data as `CodePointSet`s


As discussed: CodePointInversionList

echeran · 2022-08-09T23:06:03Z

Closing this old draft PR because it has been superseded by the subsequent proper design work and PRs #2112 and #2140 and other related PRs.

echeran added 4 commits November 18, 2021 15:22

Rename UnicodeSet to CodePointSet

b6bcac8

Regenerate data after renaming resource key category to properties

de0aeb3

More renaming unicodeset -> codepointset

2cabde9

Use CodePointSetData to simplify FFI and properties as sets API examples

1e5f3e1

echeran requested a review from sffc November 19, 2021 21:29

sffc requested changes Nov 19, 2021

View reviewed changes

sffc reviewed Nov 19, 2021

View reviewed changes

sffc requested changes Nov 19, 2021

View reviewed changes

sffc reviewed Nov 19, 2021

View reviewed changes

echeran mentioned this pull request Apr 14, 2022

Revisit return values of unicode property functions #1239

Closed

Manishearth mentioned this pull request Apr 20, 2022

Macroify properties #1810

Merged

echeran closed this Aug 9, 2022

echeran deleted the props-return-types branch June 8, 2023 01:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create wrapper structs and interfaces for properties data structures and payloads #1333

Create wrapper structs and interfaces for properties data structures and payloads #1333

echeran commented Nov 19, 2021

sffc left a comment

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

sffc Nov 19, 2021

echeran commented Aug 9, 2022

		//! It is an implementation of the existing [ICU4C UnicodeSet API](https://unicode-org.github.io/icu-docs/apidoc/released/icu4c/classicu_1_1UnicodeSet.html).
		//! It is an implementation of the existing [ICU4C CodePointSet API](https://unicode-org.github.io/icu-docs/apidoc/released/icu4c/classicu_1_1CodePointSet.html).


		fn contains_u32(&self, code_point: u32) -> bool;

		// return type cannot be `impl ExactSizeIterator<Item = RangeInclusive<u32>>;`.

		@@ -70,6 +66,9 @@

		#![cfg_attr(not(any(test, feature = "std")), no_std)]

		extern crate alloc;

Create wrapper structs and interfaces for properties data structures and payloads #1333

Create wrapper structs and interfaces for properties data structures and payloads #1333

Conversation

echeran commented Nov 19, 2021

sffc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

echeran commented Aug 9, 2022