unicode-org / icu4x

Solving i18n for client-side and resource-constrained environments.
https://icu4x.unicode.org
Other
1.39k stars 179 forks source link

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

Open robertbastian opened 4 months ago

robertbastian commented 4 months ago

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?

sffc commented 4 months ago

@zbraniecki

sffc commented 4 months ago

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 commented 2 months ago

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 commented 2 months ago

@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?

robertbastian commented 1 month ago

Conclusion for 2.0:

LGTM: @sffc @robertbastian @zbraniecki