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

Add ordinalization for locales with simple pattern #1109

Merged
merged 8 commits into from Feb 28, 2024

Conversation

c960657
Copy link
Contributor

@c960657 c960657 commented Nov 3, 2023

According to CLDR, a lot of locales use the same ordinalisation pattern for all numbers:
https://github.com/unicode-org/cldr/blob/12380e8/common/supplemental/ordinals.xml#L16-L18

Of these locales, the majority inherit from the simple rules defined in root.xml:
https://github.com/unicode-org/cldr/blob/12380e8/common/rbnf/root.xml#L698-L703

The rule just says that a number is ordinalised by adding a trailing period (aka ordinal dot), just like we currently do for German:
https://github.com/svenfuchs/rails-i18n/blob/master/rails/ordinals/de.rb

In fact, the rules for German are so simple, so we don't have to use Ruby files but can just use YAML files like in rails/locales/. Also, there was a bug in the existing definitions for de-AT, de-CH and de-DE – they all define rules for de, not for each region-specific locale.

Based on the tool from #1107 I used Cldr::Export.export(components: [Rbnf], …) to generate rbnf.yml for all rails-i18n locales whose CLDR counterpart includes an rbnf.xml. In order to get proper inheritance from root, I applied this patch: ruby-i18n/ruby-cldr#257.

I then found all rails-i18n locales whose ordinal rules were identical to the rules defined in root.xml. For each of these locales I generated an identical file in rails/ordinals.

Copy link
Contributor

@sunny sunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find! 👏🏻

spec/unit/ordinals_spec.rb Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
Comment on lines 3 to 4
- Add ordinalization for many locales (be, bs, cs, da, eo, et, fa, fi, hr, hu, is, ka,
lb, lt, lv, mk, nb, ne, nn, pl, sk, sl, sq, sr, sw, tr) #1109

This comment was marked as resolved.

@pouyaemami
Copy link

Can you please add en-CA to this list as well?

@c960657
Copy link
Contributor Author

c960657 commented Feb 28, 2024

@pouyaemami But en-CA doesn't have a “simple” pattern (as defined in this PR), does it?

@pouyaemami
Copy link

Ah, I think I missed the point of this PR. You're right, this isn't what I was hoping for. My issue is that I get this error: Translation missing: en-CA.number.nth.ordinalized but en-CA and en have the same ordinalized values and I assumed that this PR takes a known ordinalized value from a base locale and applies it to the region specific one.

@pama pama merged commit fb81390 into svenfuchs:master Feb 28, 2024
3 checks passed
@c960657 c960657 deleted the ordinals branch February 28, 2024 19:59
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.

None yet

4 participants