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.

Sign up for GitHub

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

Jump to bottom

PluralRule for DualFromZeroToTwo: Does not cover value of 2 #369

Closed
axunonb opened this issue Jan 8, 2024 Discussed in #363 · 3 comments
Closed

PluralRule for DualFromZeroToTwo: Does not cover value of 2 #369

axunonb opened this issue Jan 8, 2024 Discussed in #363 · 3 comments

Comments

@axunonb
Copy link
Member

axunonb commented Jan 8, 2024

Discussed in #363

Originally posted by Fuitad December 4, 2023
Heya,

I'm trying to understand why French is set to DualFromZeroToTwo instead of DualOneOther like English. I'm French Canadian and I'm having a hard time understanding why

{0:plural:No object|{} object|{} objects} cached on file system is working in English but {0:plural:Aucun objet|{} objet|{} objets} en cache sur le système in French is giving me SmartFormat.Core.Formatting.FormattingException: Error parsing format string: Invalid number of plural parameters in PluralLocalizationFormatter

Maybe there's something in FR-FR that explains this? But on a French Canadian point of view, this doesn't make much sense.
axunonb added a commit that referenced this issue Jan 8, 2024
@axunonb
Closes #369
69916e3 
PluralRule for DualFromZeroToTwo: Does not cover value of 2

The `Dictionary<string, PluralRuleDelegate> IsoLangToDelegate` is backed by a default dictionary. It can be restored with `PluralRules.RestoreDefault()` if one of the delegates was changed. Both has global effect.

Extract class `CustomPluralRuleProvider` to its own file

`PluralRuleDelegate DualFromZeroToTwo`:
* with 3 words, the index is for counts of 0, > 0 and < 2, more than 2
* with 4 words, the index is for counts of 0, > 0 and < 2,  >= 2 and < 3, more than 3

Add unit tests
@axunonb
Copy link
Member Author

axunonb commented Jan 8, 2024 •
edited
Loading

@karljj1 @Fuitad
PluralRules is very old code that had rarely been touched. So we should be careful with changes.
Please have a critical look at the draft PR https://github.com/axuno/SmartFormat/tree/pr/plural-rules
Besides the fix for pluralWordsCount == 2 I tried to make DualFromZeroToTwo more consistent for different pluralWordsCount.
Your feedback is very welcome.

@karljj1
Copy link
Collaborator

karljj1 commented Jan 8, 2024

@karljj1 @Fuitad PluralRules is very old code that had rarely been touched. So we should be careful with changes. Please have a critical look at the draft PR https://github.com/axuno/SmartFormat/tree/pr/plural-rules Besides the fix for pluralWordsCount == 2 I tried to make DualFromZeroToTwo more consistent for different pluralWordsCount. Your feedback is very welcome.

The link doesnt seem to work, it just takes me to the main repo page. I cant see any PRs, maybe I dont have permission to see it?

@axunonb
Copy link
Member Author

axunonb commented Jan 8, 2024 •
edited
Loading

PR submitted right now: #370
The link above opens the repo with branch pr/plural-rules

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@axunonb @karljj1