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

Suggestion: Use PluralRules instead of Count #5369

Open
younies opened this issue Aug 14, 2024 · 4 comments
Open

Suggestion: Use PluralRules instead of Count #5369

younies opened this issue Aug 14, 2024 · 4 comments
Assignees

Comments

@younies
Copy link
Member

younies commented Aug 14, 2024

Summary

Currently, the enum Count is being used in multiple places around icu4x, such as units, currency, decimal ... etc.

  • I suggest implement in PluralRules place an enum called l PluralRuleCount, which would contain all the CLDR counts:
pub enum PluralRulesCount {

  Zero,
  One,
  Two,
... etc.
}
  • All data providers should adopt this standardized PluralRuleCount.
  • If users need to use extra counts or variations, they should extend the existing PluralRuleCount.
    • Example:
     pub enum CompactDecimalCount {
          PluralRules(PluralRulesCount)
           Explicit1,
           Explicit0,
     }
@younies younies self-assigned this Aug 14, 2024
@younies younies added the discuss-priority Discuss at the next ICU4X meeting label Aug 14, 2024
@sffc
Copy link
Member

sffc commented Aug 15, 2024

The standard enum already exists: icu_plurals::PluralCategory

I like the idea of component-specific enums extending the standard one. Alternatively, I would be okay with icu_plurals exporting an enum with the explicit rules.

@younies
Copy link
Member Author

younies commented Aug 15, 2024

In this case, we can use icu_plurals::PluralCategory.

But we need to address two things:

  1. When the user extends the enum, there should be a function that handles, let's say, the first 4 bits for verifying and converting from and to the unaligned version of the PluralCategory. Then, the user can manipulate the remaining 4 bits as needed.
  2. Ideally, it would be good if make_ule could handle this without a need of intervetion.

@younies
Copy link
Member Author

younies commented Aug 15, 2024

Example of an extended enume: #5375

and a problem that need to be solved:
#5375 (comment)

@sffc
Copy link
Member

sffc commented Aug 15, 2024

  • @Manishearth It's okay and expected for a manual ULE impl. make_ule won't work well here because it would need to share data with the child enum. Also, please remove the repr(u8).
  • @echeran I noticed that this issue is talking about an enum to represent the explicit number matching, but there's an active discussion Match numbers numerically message-format-wg#842 with a proposal that the matching is not specific to integers, let alone to an enumerated list of integers.
  • @sffc Currentlly, CLDR defines currency patterns for =0 and =1. When CLDR supports currency matching something like =73, then we can update our enum accordingly. But we are only supporting what CLDR specifies, and we don't need to design a general solution prematurely.
  • @younies We could extend in the plurals crate for what is needed.
  • @sffc One advantage of redefining the explicit enum in each crate is that each crate might need a different set of explicit values. An advantage of defining the enums in a common plae is that I'm not sure if it ever happens that different crates need different enums. It's always =0 and =1.
  • @younies This plural implementation needs to have all categories that .. provides.
  • @sffc Overall though I think it probably makes sense to have a single icu_plurals::PluralCategoryExtended.
  • @Manishearth generally in favor of per-crate enums until we need something else. provider types are unstable so we can make such changes later. though it is nice to reduce unsafe
  • @robertbastian The PluralRules type could return PluralCategoryExtended as well.
  • @sffc It's good to return PluralRules becuase PluralCategoryExtended is a very ICU4X-specific type. I'm in favor of having both types and another function that returns the new PluralCategoryExtended type.
  • @robertbastian If we had MessageFormat 2 (MF2) in the future, what would we do?
  • @sffc This is not for MF2, this is a data provider concern for ICU4X.

Proposal:

  • Add icu_plurals::PluralCategoryExtended which is PluralCategory but with Explicit0, Explicit1, and is non-exhaustive so more explicits can be added
  • The type uses 4 bits.
  • Add a function PluralRules::extended_category_for which returns PluralCategoryExtended

LGTM: @sffc @Manishearth @younies @echeran

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants