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

Translation improvements: CLDR Plurals + Translatables #19916

Closed
wants to merge 27 commits into from

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Jun 7, 2022

This PR will provide the framework for a few translation improvements.

TranslatableStringer and TranslatableFormatted

The first thing is the ability to pass in translatable elements - this would allow us (for example) to create an error:

type TranslatableError struct {
Key string
Args []interface{}
}

func (err *TranslatableError) Error() string {
   return i18n.Tr("en-US", err.Key, err.Args...)
}

func (err *TranslatableError) TranslatedString(l i18n.Locale) {
   return l.Tr(err.Key, err.Args...)
}

TrPlural and TrOrdinal

The second part is the creation of TrPlural and TrOrdinal which use the CLDR data to correctly determine plurals for the locales. This is used to replace TrN.

Plural and ordinal keys can be written as go templates if the key has the suffix _plural or _ordinal:

example_plural=common text {{if .One}}Singular{{else}}Other{{end}}

These are interpreted at the time of locale reading and split into the appropriate forms for the language. Alternatively the individual plural forms can be provided separately as example_plural_one etc.

As the templates are parsed once at locale load time the magic suffix is required - I don't think this is too clunky. It would be possible to make it so we parse the template at time of TrPlural call but that would require the locale storing a text.Template or other locking mechanism, and I think the magic suffix isn't too inconvenient.

(TrN has been kept in place to help handle merges and old data.)

Fixes #23797

Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added type/feature Completely new functionality. Can only be merged if feature freeze is not active. pr/wip This PR is not ready for review labels Jun 7, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 10, 2022

Codecov Report

❗ No coverage uploaded for pull request base (main@8430f73). Click here to learn what that means.
The diff coverage is 87.03%.

@@           Coverage Diff           @@
##             main   #19916   +/-   ##
=======================================
  Coverage        ?   48.39%           
=======================================
  Files           ?     1038           
  Lines           ?   141321           
  Branches        ?        0           
=======================================
  Hits            ?    68394           
  Misses          ?    64826           
  Partials        ?     8101           
Impacted Files Coverage Δ
modules/test/context_tests.go 62.96% <0.00%> (ø)
modules/translation/i18n/i18n.go 77.77% <0.00%> (ø)
modules/translation/i18n/translatable.go 0.00% <0.00%> (ø)
modules/translation/translation.go 36.29% <ø> (ø)
routers/web/repo/migrate.go 38.80% <0.00%> (ø)
routers/web/repo/repo.go 20.00% <0.00%> (ø)
routers/web/repo/setting.go 16.71% <0.00%> (ø)
modules/translation/i18n/plurals/form.go 16.66% <16.66%> (ø)
modules/translation/i18n/format.go 40.62% <42.85%> (ø)
modules/translation/i18n/locale.go 44.04% <44.04%> (ø)
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 10, 2022
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor Author

zeripath commented Sep 1, 2022

Conflicts resolved.

@zeripath

This comment was marked as outdated.

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath removed the pr/wip This PR is not ready for review label Sep 1, 2022
@zeripath zeripath changed the title WIP: Translation improvements Translation improvements: CLDR Plurals + Translatables Sep 1, 2022
@lafriks
Copy link
Member

lafriks commented Sep 5, 2022

How that would work with crowdin? When English have two forms but other languages could have 3 or 4?

@zeripath
Copy link
Contributor Author

zeripath commented Sep 5, 2022

example_plural = {{if .Zero}}zero{{else if .One}}one{{else}}other{{end}}form

Or add each form individually as example_plural_zero etc. (But I don't think crowdin will let you do that at present.)

Locale keys ending in _plural or _ordinal are parsed as go templates once at time of locale reading and the plural forms are extracted as per the forms within that language.

@zeripath
Copy link
Contributor Author

zeripath commented Sep 6, 2022

@lafriks I've updated the description.

@wxiaoguang
Copy link
Contributor

Generally L-G-T-M.

Could it be better if the plurals and the common/dtd are put in a new public go package? They seem quite complex and independent. Making them a new package would keep Gitea repo more focused. (and maybe other projects could re-use them)

@zeripath
Copy link
Contributor Author

Generally L-G-T-M.

Could it be better if the plurals and the common/dtd are put in a new public go package? They seem quite complex and independent. Making them a new package would keep Gitea repo more focused. (and maybe other projects could re-use them)

I think we should try to figure out what works before export things. for example we need to include date and time formatting at some point.

Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@delvh delvh added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 29, 2023
@wxiaoguang
Copy link
Contributor

We need ICU message system #23863, this homemade syntax is impossible to maintain.

@wxiaoguang wxiaoguang closed this May 2, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect handling of plural forms for translation
6 participants