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

Move icu_plurals::rules::reference to datagen? #5181

Open
robertbastian opened this issue Jul 4, 2024 · 4 comments
Open

Move icu_plurals::rules::reference to datagen? #5181

robertbastian opened this issue Jul 4, 2024 · 4 comments
Labels
discuss Discuss at a future ICU4X-SC meeting needs-approval One or more stakeholders need to approve proposal

Comments

@robertbastian
Copy link
Member

robertbastian commented Jul 4, 2024

Seems to be datagen-only code?

Both icu_plurals::rules and icu_datetime::pattern have parallel reference and runtime modules, presumably reference is not used at runtime?

@robertbastian robertbastian added this to the ICU4X 2.0 ⟨P1⟩ milestone Jul 4, 2024
@sffc sffc added the needs-approval One or more stakeholders need to approve proposal label Jul 8, 2024
@sffc
Copy link
Member

sffc commented Jul 8, 2024

@zbraniecki

@sffc
Copy link
Member

sffc commented Jul 23, 2024

In 2.0 I'm adding https://unicode-org.github.io/icu4x/rustdoc/icu_datetime/neo_pattern/struct.DateTimePattern.html which is a polished API around datetime::pattern::reference. I don't know if we need/want a public API around plurals::pattern::reference. I'm not super happy with it being doc(hidden).

@zbraniecki
Copy link
Member

The reason I added reference is that it allows for canonical parse/edit/serialize loopback on the rules/patterns.

This is used by datagen but the target audience is wider - customers who want to manipulate a Plural Rule should be able to acquire this parser/serializer pair from icu_plurals. I'm ok for this to be behind a flag, but I'd be against moving it out of the respective crate.

@sffc
Copy link
Member

sffc commented Sep 6, 2024

@zbraniecki, do you want the whole module exported with all its (fairly deep) AST APIs and things, or would you be happy with a high-level API with just a few functions like parse and serialize?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Discuss at a future ICU4X-SC meeting needs-approval One or more stakeholders need to approve proposal
Projects
Status: Needs discussion to unblock
Development

No branches or pull requests

3 participants