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

Get closer to full CLDR pluralization support #634

Conversation

movermeyer
Copy link
Contributor

@movermeyer movermeyer commented Aug 12, 2022

What are you trying to accomplish?

A step towards CLDR pluralization. Ref: #629

Previously, I put forth a PR, that was reverted in #633 since dropping [incorrect] support for zero key is a breaking change.

This PR is the same as #630 (adding support for explicit 0/1 keys and basic lateral inheritance), but maintains the existing [incorrect] zero key behaviour.

What approach did you choose and why?

I brought back the changes from my first attempt, but also brought back the [incorrect] zero key behaviour:

Some progress towards CLDR compatibility is better than none. Removal of the zero key behaviour will be done in a separate PR.

Q&A

Q: Are 0 & 1 the only special cases, or can one introduce others? Perhaps 2: I have a pair of cars
A: It might be allowed, depending on how you read the spec... but the spec only mentions 0 and 1, the LDML DTD only considers 0 and 1 to be valid, and CLDR has only ever used 0 and 1. I left support for arbitrary numbers out of this PR, as I figure that it would be better to be restrictive with what we accept right now. It's an easy change to accept other values if it turns out to be desirable.

The impact of these changes

Pluralization lookups will will support CLDR's data.
Ref: #629 for details of why these changes are important.

Adds explicit 0/1 keys and basic lateral inheritance.
@movermeyer movermeyer force-pushed the movermeyer/explicit_0_1_and_lateral_inheritance branch from 6290cbb to c78ca61 Compare August 12, 2022 17:16
@movermeyer
Copy link
Contributor Author

cc @radar this is ready for review

@radar
Copy link
Collaborator

radar commented Feb 3, 2023

This looks good to me. Thanks @movermeyer!

@radar radar merged commit d94228c into ruby-i18n:master Feb 3, 2023
@movermeyer
Copy link
Contributor Author

Hey @radar, whenever you get some time, could we get a new release of ruby-i18n/i18n? 🙏

I'm eagerly awaiting the time when I can start using these features.

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

Successfully merging this pull request may close these issues.

2 participants