cobra icon indicating copy to clipboard operation
cobra copied to clipboard

Implement localization using gettext files — I18N — L10N

Open Goutte opened this issue 1 year ago • 23 comments

See #1134

Features

  • Recipe to extract new translations from the Go code: make i18n_extract
  • Using widely supported gettext format
  • Embedded lightweight MO files
  • Detect language from environment variables
  • Some strings were pluralized

Issues

No regional fallback for now, waiting for leonelquinteros/gotext#85 to be merged.

This will also unlock fallback to english when a translation is not found.


Replaces #1944

Goutte avatar Dec 14 '23 17:12 Goutte

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Dec 14 '23 17:12 github-actions[bot]

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Dec 15 '23 04:12 github-actions[bot]

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Dec 16 '23 09:12 github-actions[bot]

I know it's a lot of added lines, but more than half are from the POT and PO files. I added the french translation as an example, for good measure.

Goutte avatar Dec 16 '23 10:12 Goutte

Thanks @Goutte!

A couple of things to help get this reviewed: 1- can resolve the conflicts with main 2- can you add in the description how to test this (including the best way to install xgotext)

marckhouzam avatar Dec 22 '23 19:12 marckhouzam

@Goutte could you explain what the process would be, if we adopted this PR, for a cobra contributor to add a new string printout? It is not clear to me when make i18n_extract should be run and which *.po/default.pot/*.mo files need to be modified by hand and when, etc.

At this point, clarifying these steps in the PR description is sufficient, but if the review goes well, we'll need those details in some cobra documentation.

Also, I'm also interested in understanding how a project using Cobra would proceed to add translations, or is that completely up to the project and this PR does not help?

marckhouzam avatar Dec 22 '23 20:12 marckhouzam

@Goutte could you explain what the process would be, if we adopted this PR, for a cobra contributor to add a new string printout? It is not clear to me when make i18n_extract should be run and which *.po/default.pot/*.mo files need to be modified by hand and when, etc.

I see this already in the docs. Nice work. Can you clarify what you mean by "Make sure your software has also updated the MO files"? Do we absolutely need to use a special editor like poedit or can this be done from the command-line?

marckhouzam avatar Dec 22 '23 21:12 marckhouzam

Something to discuss later, I just don’t want to forget.

We will need a way for a program using cobra to disable these translations. For example, if a mycli project does not translate its own strings, it might not want the cobra strings to get translated as it would give an inconsistent user experience.

So we probably want the project to explicitly have to turn on translations through a global setting.

marckhouzam avatar Dec 23 '23 01:12 marckhouzam

Thanks for the comprehensive review, @marckhouzam !

Also, I'm also interested in understanding how a project using Cobra would proceed to add translations, or is that completely up to the project and this PR does not help?

This PR does not help devs to implement their I18N. They are free to choose whatever solution they like.

We could perhaps provide some doc/sugar on how to use the same translation toolkit as cobra, since the relevant libraries are already included ?

Can you clarify what you mean by "Make sure your software has also updated the MO files"? Do we absolutely need to use a special editor like poedit or can this be done from the command-line?

Good question. Since gettext is mature, I'm pretty confident there are CLI tools out there handling this. (or we'll have to make one, preferably in Golang, using cobra of course)

I'd like the MO to be generated by CI as well, if only to prevent contributors to inject nasty things in the binary files.

I'll look into this.

So we probably want the project to explicitly have turn on translations through a global setting.

Very good point. Partially translated apps are awkward, which is precisely why I wanted cobra to support i18n. I'm not sure right now how to proceed to add a global setting to cobra, but I'll look into it and come back to ask for help if I don't find how. Do give tips if you'd prefer it to be done in some specific way :)

Goutte avatar Dec 23 '23 13:12 Goutte

To allow a project to enable translations I’m thinking of two approaches you can take:

  1. A global setting like cobra has for other features: https://github.com/spf13/cobra/blob/41227856cd731aeeeb5a2f7203b3a4c16b81fc67/cobra.go#L52-L66
  2. A field in the root command like we have for tuning shell completions https://github.com/spf13/cobra/blob/41227856cd731aeeeb5a2f7203b3a4c16b81fc67/command.go#L203

if you think different settings might be useful eventually, I would recommend using a struct so we could grow it overtime

marckhouzam avatar Dec 23 '23 14:12 marckhouzam

This PR does not help devs to implement their I18N. They are free to choose whatever solution they like.

We could perhaps provide some doc/sugar on how to use the same translation toolkit as cobra, since the relevant libraries are already included ?

Thanks for clarifying, it makes sense. Let’s leave this for a potential separate PR and focus on cobra’s strings .

marckhouzam avatar Dec 23 '23 14:12 marckhouzam

I think cobra should have a built tag to disable/enable translations. Right now it looks like all translations are embedded in the final binary. That will lead to unnecessary bloat for users who do not want to use it so a build tag would be the better option IMO.

And these translations should be opt in not opt out IMO.

Luap99 avatar Dec 28 '23 14:12 Luap99

I think cobra should have a built tag to disable/enable translations IMO.

Good idea +1

And these translations should be opt in not opt out IMO.

Agreed 👍

marckhouzam avatar Dec 28 '23 20:12 marckhouzam

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Jan 10 '24 15:01 github-actions[bot]

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Jan 10 '24 15:01 github-actions[bot]

I think cobra should have a built tag to disable/enable translations. Right now it looks like all translations are embedded in the final binary. That will lead to unnecessary bloat for users who do not want to use it so a build tag would be the better option IMO.

And these translations should be opt in not opt out IMO.

That's a good idea.

I think I managed to do this using the "locales" build tag.

Now I need to tweak the test suite to use that build tag, or perhaps even make two runs, with and without.

Anyhow, for now the tests are broken until I figure this out.

Goutte avatar Jan 10 '24 15:01 Goutte

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Jan 10 '24 15:01 github-actions[bot]