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

langs/i18n: Fix warning regression in i18n #8497

Merged
merged 1 commit into from
May 2, 2021
Merged

Conversation

bep
Copy link
Member

@bep bep commented May 2, 2021

Fixes #8492

Fix this by

1. Making sure that only numerical values are treated as plural counts
2. Making sure that `i18n.pluralFormNotFoundError` is not logged as a warning if `other` resolved.

Note that 2. isn't a new problem, but became visible because of the plural improvements in Hugo `0.83.0`.

Fixes gohugoio#8492
@bep bep force-pushed the fix/i18n-/8492 branch from 82d0770 to 14b77a1 Compare May 2, 2021 14:03
@bep bep merged commit ececd1b into gohugoio:master May 2, 2021
@jmooring
Copy link
Member

jmooring commented May 2, 2021

This should generate a warning because the result will be wrong:

{{ T "day" 1 }}
[day]
other = "days"

@jmooring
Copy link
Member

jmooring commented May 2, 2021

I feel like we are perpetuating a problem with i18n files in the wild where the other category has been historically misused due to insufficient documentation. If someone needs to translate something without pluralization rules, they should do it this way:

i18n/fr.toml

# Items without pluralization rules.

about = "about in French"
help = "help in French"
contact_us = "contact us in French"

# Items with pluralization rules.

[day]
one = "jour
other = "jours"

@bep
Copy link
Member Author

bep commented May 2, 2021

I feel like we are perpetuating a problem with i18n files in the wild where the other category has been historically misused due to insufficient documentation.

I don't agree.

  • As you have proven yourself with your test cases, languages have different plural rules -- for a given message/language combo a given language may need/want only one (which currently fits nicely into other) other needs one, few, many ...
  • The most common use case is to pass nothing as data/ctx to i18n (nil), which is treated as the "other" case in go-i18n -- and the real regression that this fixes.
  • We obviously needed to patch this as this broke a very common use in the wild.

If you still feel that this is a problem needing a fix, open up another issue.

@jmooring
Copy link
Member

jmooring commented May 2, 2021

Will do. Thanks.

@salim-b
Copy link
Contributor

salim-b commented May 3, 2021

Ok, this is now really an unsatisfying situation!

  • First Hugo 0.83 brought these has no plural form "one" i18n warning messages because it correctly applied Language Plural Rules.

  • I've fixed those warnings by defining one instead of other for my French translations.

  • Then Hugo 0.83.1 flipped this upside down and now I get has no plural form "other" i18n warning messages for French with the same config. When I revert to defining other instead of one for the French translations, they disappear.

  • They also disappear when I define the translations without an explicit cardinal form ("without pluralization rules") as @jmooring suggested above.

I guess the underlying problem is a lack of formal specification how "simple" translations without pluralization forms are to be defined. Just falling back to cardinal form other obviously feels wrong. Or as @jmooring puts it:

the other category has been historically misused due to insufficient documentation


Now my question: Will the mentioned simple "without pluralization rules" definition form stay the official/recommended way to define translations without pluralization rules in Hugo? (This would be my preference I guess 🙂 )

salim-b added a commit to salim-b/airspace-hugo that referenced this pull request May 3, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Hugo [v0.83.1](https://github.com/gohugoio/hugo/releases/tag/v0.83.1) reverted things changed in v0.83, so themefisher#147 is a non-solution now.

I've changed i18n files to use the [simple/old translation formalism](gohugoio/hugo#8497 (comment)) which [hopefully stays the recommended to define translations without pluralization rules way in Hugo](gohugoio/hugo#8497 (comment))
@github-actions
Copy link

github-actions bot commented May 4, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In v0.83.0 : i18n|MISSING_TRANSLATION WARN messages for keywords not used in i18n
3 participants