error on fallback to non-default less specific language

le-yams commented 4 days ago

Description

go-i18n version: v2.4.1

When asking for a given language, if not defined it correctly fallback to a less specific language. But if the specific language is define but has not a given key it fails. Thus, all non specific keys must be duplicated and redefined for the specific language.

Here is a test to reproduce the error:

bundle := i18n.NewBundle(language.English)

fr := language.MustParse("fr")
frBE := language.MustParse("fr-BE")

bundle.MustAddMessages(fr, &i18n.Message{ID: "key1", Other: "fr key 1"})
bundle.MustAddMessages(fr, &i18n.Message{ID: "key2", Other: "fr key 2"})
bundle.MustAddMessages(frBE, &i18n.Message{ID: "key2", Other: "fr-BE specific key 2"})

localizerFrBE := i18n.NewLocalizer(bundle, "fr-BE")

key1 := localizerFrBE.MustLocalize(&i18n.LocalizeConfig{MessageID: "key1"})
key2 := localizerFrBE.MustLocalize(&i18n.LocalizeConfig{MessageID: "key2"})

if key1 != "fr key 1" {
  t.Error("key1 should 'be fr key 1'")
}
if key2 != "fr-BE specific key 2" {
  t.Error("key2 should 'be fr-BE specific key 2'")
}

Expected behavior

The test should pass and the key2 variable should have the value fr-BE specific key 2

Actual behavior

I get the error message

message "key1" not found in language "fr-BE"

Thanks in advance for your help.

Regards, Yann

nicksnyder commented 4 days ago

But if the specific language is define but has not a given key it fails. Thus, all non specific keys must be duplicated and redefined for the specific language.

You are correct that this is how the library behaves, and this was an intentional design decision. If you register a language, you need to provide all translations for that language. The reason for this design is that golang.org/x/text/language.Matcher is a relatively expensive object to create, so we really don't want to have to create it for every single call to Localize or for every single message id (which I think is what would need to happen to be able to support the behavior you want). Instead, the library expects users to provide complete translations files.

In particular for this case, localizerFrBE := i18n.NewLocalizer(bundle, "fr-BE") doesn't even indicate that fr is a valid fallback language, and in general it is not safe to assume that xx is a safe fallback for xx-YY.

That said, even in the case of localizerFrBE := i18n.NewLocalizer(bundle, "fr-BE fr"), the localizer will match on fr-BE since it has translations and will never fallback to fr for a single translation (unless there are no translations for fr-BE at all).

I understand why it would be nice to have this handled within the library, but in terms of overall complexity and performance I think it is best to just copy translations.

le-yams commented 4 days ago

in general it is not safe to assume that xx is a safe fallback for xx-YY

I would argue that it is a very common usage (probably the main one ^^). Moreover this library already makes that assumption when the specific language doesn't exist at all. So it kind of fall between two stools, in one case you make that assumption but not in another case.

The reason for this design is that golang.org/x/text/language.Matcher is a relatively expensive object to create, so we really don't want to have to create it for every single call to Localize or for every single message id (which I think is what would need to happen to be able to support the behavior you want). Instead, the library expects users to provide complete translations files. I understand why it would be nice to have this handled within the library, but in terms of overall complexity and performance I think it is best to just copy translations.

That makes perfect sense :). I'll try to dive in the code if I have some time and perhaps make a PR if I find a solution that might be interesting. Thanks for your time.

Regards, Yann

nicksnyder commented 3 days ago

I would argue that it is a very common usage (probably the main one ^^).

It may be common, but it is not correct in general across languages, so this library can't make this assumption in general.

You are basically asking to make an individualized match per message based on what translations are available for that message. Here are all the reasons not to do this:

V2 of this library was originally designed to do this behavior and I changed it back exactly due to the perf regression. Here is the context: https://github.com/nicksnyder/go-i18n/issues/182#issuecomment-501898610
If this library implements fallback calculations per translation, then a single rendered page could end up with an arbitrary number of languages on it, which is not a great user experience.
There is no reason this kind of fallback computation needs be done at runtime. It can (and should) be effectively precomputed by explicitly providing a full translation file to go-i18n for each language.

Moreover this library already makes that assumption when the specific language doesn't exist at all. So it kind of fall between two stools, in one case you make that assumption but not in another case.

These two assumptions are not the same.

The current library picks the best language it has translations for given a set of preferences, and then serves all translations for that language. If an individual message in the preferred language is missing, it does a (constant time) fallback to the default language in the bundle (because as the source language that is guaranteed to exist and showing something is better than showing nothing). A missing translation for a language that has been provided to go-i18n is effectively an error. The fix is to provide the translation, not to implement complex/slow fallback mechanism.

nicksnyder / go-i18n