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.

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

Remaining properties naming polishes #2141

Closed
Manishearth opened this issue Jun 29, 2022 · 4 comments
Closed

Remaining properties naming polishes #2141

Manishearth opened this issue Jun 29, 2022 · 4 comments
Assignees
Labels
C-unicode Component: Props, sets, tries help wanted Issue needs an assignee

Comments

@Manishearth
Copy link
Member

Carried over from #1239

With #2140 and #2112, the main icu_properties return values are all consistently named. We have CodePoint[Set|Map]Data(Borrowed)? as a naming scheme for the main APIs for interacting with this data. Stringy maps and sets can be supported in the future with [Unicode|CodePoint]{Set|Map]Data(Borrowed)?

So far, the data structs have not been renamed. They're currently called UnicodePropertyV1 and UnicodePropertyMapV1, which isn't great. Note that these types are not super important since users shouldn't be interfacing with them, but consistent naming woudl be nice.

@echeran suggested Property[Unicode|CodePoint][Set|Map]V1. I like having Property in the name there.

Also we have UnicodeSet. I don't think we need as much consistency there: that's a specific backend and doesn't need to be named analogously to other backends, but we probably should name it CodePointSet or something (and rename the builder type and associated methods).

@Manishearth Manishearth added help wanted Issue needs an assignee C-unicode Component: Props, sets, tries labels Jun 29, 2022
@Manishearth Manishearth added this to the ICU4X 1.0 (Polish) milestone Jun 29, 2022
@sffc
Copy link
Member

sffc commented Jun 29, 2022

Renaming UnicodeSet to CodePointSet should be done at some point, either as a standalone PR or as part of the migration to icu_collections.

@andrewpollack andrewpollack self-assigned this Jul 17, 2022
@andrewpollack
Copy link
Contributor

For this, I'll create two PRs:

  1. Renaming UnicodePropertyV1 -> PropertyUnicodeSetV1, and UnicodePropertyMapV1 -> PropertyUnicodeMapV1
  2. Renaming UnicodeSet -> CodePointSet, and all related libraries. This will be done before the migration to icu_collections

@sffc
Copy link
Member

sffc commented Jul 28, 2022

@andrewpollack Is this complete?

@andrewpollack
Copy link
Contributor

Completed by #2198

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-unicode Component: Props, sets, tries help wanted Issue needs an assignee
Projects
None yet
Development

No branches or pull requests

3 participants