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

Missing support for fallback translations #70

Closed
m-horky opened this issue Apr 12, 2023 · 3 comments
Closed

Missing support for fallback translations #70

m-horky opened this issue Apr 12, 2023 · 3 comments

Comments

@m-horky
Copy link
Contributor

m-horky commented Apr 12, 2023

GNU gettext contains a support for priority lists. The LANGUAGE environment variable can contain multiple languages (e.g. sv:de or sk:cs:pl) to provide alternative translation if the primary is not available, instead of falling back to the msgid.

The current implementation of gotext does not support this; when multiple languages are specified, only the first one is loaded, the rest of the locale string is discarded.

Proposal description

Update config struct and methods in gotext.go (loadStorage, Configure, Get*) to handle multiple Locale objects instead of just one. Get* methods, instead of reaching into the only domain, would iterate over loaded Locales and return the string in the first language in which it is translated, keeping the msgid as the last option.

Further considerations

  • gotext itself does not load environment variables. Even if GNU gettext only supports this via the LANGUAGE variable, not LC_* or LANG, this should be the new only implementation; the change is internal and would not alter the behavior for those who are using just one language.
  • On load, all translations should be loaded into memory and .IsTranslated* from PR Runtime translation detection #69 could be used to load the first available one. An alternative to this could be to pick a translations on load, and serve this string when requested -- effectively a tradeoff between startup time and memory efficiency during runtime. I lean towards the former, as it should be easier to implement and maintain, with the current structure of the code.

Comments

I can create a patch containing these changes.

@leonelquinteros
Copy link
Owner

Hey @m-horky, this sounds like a good improvement.

So, how would the implementation look like? Would you extend the support for this syntax (en:es) in the Config.language field? So current users don't have to modify anything, but after this change, if anyone wants to add the : option to it, it would work? Or maybe a different field?
I prefer the former, but gettext docs doesn't mention anything about the en_EN syntax support for that env var, so it seems you can't do something like en_EN:en_US:es_ES:es_AR:br, or at least it's not clear how would it work?

I'm up to support this kind of definitions, they seem appropriate, unless there are specific docs in gettext suggesting otherwise.

What do you think?

@m-horky
Copy link
Contributor Author

m-horky commented Apr 17, 2023

I started writing my ideas here, but then I went to check whether they were possible to do and I managed to create a proof of concept. I'm not totally happy with it, especially the functionality of Locale.GetActualLanguage() may be better of somewhere else, but I'd call it a start.

I opened draft PR #73

@m-horky
Copy link
Contributor Author

m-horky commented May 11, 2023

Hi @leonelquinteros, when you have the time, could you look at the mentioned PR and do some fast review? It is still draft as there surely are things to improve, but I'd like to know if its ideas were something that would fit into the library.

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

No branches or pull requests

2 participants