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

Internationalization (I18N) #1944

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Internationalization (I18N) #1944

wants to merge 12 commits into from

Conversation

Goutte
Copy link

@Goutte Goutte commented Apr 4, 2023

Work In Progress
Incomplete but good enough for my own use.
Overly big dependencies, needs some scrubbing.

How

  • Detects language from ENV (LANGUAGE > LC_ALL > LANG)
  • Loads embedded toml translation files
  • Translates

Breaking

Pluralization

Output in English of some flag errors has changed a little, since pluralization is dynamic.
Eg: accepts at most 2 args, received 3 and accepts at most 1 arg, received 3
If that's not something you think we should have, and should stick with arg(s) to ease parsing for example (but… regex), I can revert this.

go 1.16

go 1.16 is required for go-i18n v2. I enjoy goi18n extract and merge too much to bother downgrading to v1. Furthermore, go:embed requires go 1.16 as well.

Partial

Translations flow documentation

Not documented yet.
I'll start sketching it here…

The translation flow is as follows:

  1. Create a new Message in a wrapper function in localizer.go
  2. Call that function somewhere in cobra
  3. Run make i18n_extract
  4. Edit the newly generated translations/translate.*.toml files
  5. Run make i18n_merge, it will write to the translations/active.*.toml files
  6. Delete the now useless translations/translate.*.toml files

Not all strings translated

I focused on user-facing strings because those were the most useful to me.
I might do some more but probably won't do them all.

Bouerk

go.sum went bananas, and I'm not sure what happened. Help appreciated. Some info here.

@CLAassistant
Copy link

CLAassistant commented Apr 4, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

github-actions bot commented Apr 4, 2023

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.

@marckhouzam marckhouzam added kind/feature A feature request for cobra; new or enhanced behavior area/cobra-command Core `cobra.Command` implementations labels Apr 4, 2023
@marckhouzam
Copy link
Collaborator

Thanks @Goutte !
I just want to confirm that there is interest for this. It will take a little while to get this reviewed, but I think this is something valuable for cobra.

And to top it off, the translations are in French, which I am able to review 😀

@github-actions github-actions bot removed the area/cobra-command Core `cobra.Command` implementations label Apr 5, 2023
@github-actions
Copy link

github-actions bot commented Apr 5, 2023

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.

@marckhouzam marckhouzam added this to the 1.8.0 milestone Apr 5, 2023
Copy link
Collaborator

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this! Big feature add - I definitly want to be involved in the review of this since it's a rather sneaky breaking change (i.e., CLI apps that use their own internationalization, apps that expect output from errors of a certain kind, etc.)

But it's worth diving into!

@Goutte
Copy link
Author

Goutte commented Apr 18, 2023

I was quite a dive, but I was already in the lagoon and I wanted it for git spend (shameless plug), since having some english strings sprinkled in french doc was a tad uncanny.

I reviewed this today, and I find the translation-as-a-function approach to be verbose, but explicit enough (and this way we get automatic edition of toml files).

The dependencies are wrong, though, and desperately need some love.

Suspects:

  • TOML lib
  • Locale string parsing

I'd delve into it with a cooler head, but I'm changing jobs and wasting my time writing a CV (hire me while I'm free !) instead of writing the code that needs to be written and that I want to write.

Thanks for the kind words, I'm glad you're considering adding I18N to cobra one way or another.

@jpmcb
Copy link
Collaborator

jpmcb commented Oct 17, 2023

Hi @Goutte - what's the status of this? This PR needs a rebase but it'd be a cool feature add to get this into the next release

@Goutte
Copy link
Author

Goutte commented Oct 17, 2023

@jpmcb The go.mod is … not great. I have not investigated further as to why. (see my previous comment)

I can rebase, but I think this MR needs more work to reduce its dependencies. I'm not sure how, though. Perhaps by choosing another TOML lib ? I tried to hunt which lib was adding all these deps, to no avail.

@Goutte
Copy link
Author

Goutte commented Nov 4, 2023

Dependency Hell

I tried go mod graph, and found out that the dependency on golang.org/x/text/language is what's requiring golang.org/x/text and with it all the not useful things bloating the dependencies.

The thing is: github.com/nicksnyder/go-i18n/v2 is also using that golang.org/x/text dependency.

I'm not about to code a lightweight alternative of the language shenanigans, in order to bypass these libs.

Using init()

We're setting up the localizer in an init() function, and it's shunned upon, I believe.

func init() {
	setupLocalizer()
}

As an alternative, perhaps I could try to lazily initialize whenever we use a localize() function ?

@Goutte
Copy link
Author

Goutte commented Nov 4, 2023

Here's what I mean about the dependency bloat, see go.sum :

github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg=
golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

Naughty stuff : crypto, net, even goldmark.
We do not need any of this, and it's smelly, even.

Copy link

github-actions bot commented Nov 4, 2023

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.

@Goutte
Copy link
Author

Goutte commented Nov 4, 2023

Here's the go mod graph:

$ go mod graph
github.com/spf13/cobra github.com/BurntSushi/[email protected]
github.com/spf13/cobra github.com/cpuguy83/go-md2man/[email protected]
github.com/spf13/cobra github.com/inconshreveable/[email protected]
github.com/spf13/cobra github.com/nicksnyder/go-i18n/[email protected]
github.com/spf13/cobra github.com/spf13/[email protected]
github.com/spf13/cobra golang.org/x/[email protected]
github.com/spf13/cobra gopkg.in/[email protected]
github.com/cpuguy83/go-md2man/[email protected] github.com/russross/blackfriday/[email protected]
github.com/nicksnyder/go-i18n/[email protected] github.com/BurntSushi/[email protected]
github.com/nicksnyder/go-i18n/[email protected] golang.org/x/[email protected]
github.com/nicksnyder/go-i18n/[email protected] gopkg.in/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
gopkg.in/[email protected] gopkg.in/[email protected]
gopkg.in/[email protected] gopkg.in/[email protected]
golang.org/x/[email protected] github.com/yuin/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]
golang.org/x/[email protected] golang.org/x/[email protected]

I looked at the most recent version of golang.org/x/text (0.14.0, and we're requiring 0.4.0 to match the I18N lib), and it's still requiring tools, that will end up loading goldmark, crypto, and net.

I'm inspecting the source of golang.org/x/text using:
git clone https://go.googlesource.com/text golang-text

@Goutte
Copy link
Author

Goutte commented Nov 5, 2023

Looking at where the tools dep is used in x/text:

$ rg "x/tools"

cmd/gotext/main.go
30:     "golang.org/x/tools/go/buildutil"

cmd/gotext/common.go
12:     "golang.org/x/tools/go/loader"

go.sum
22:golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
23:golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
24:golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU=
25:golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=

go.mod
3:require golang.org/x/tools v0.1.12 // tagx:ignore

message/pipeline/rewrite.go
18:     "golang.org/x/tools/go/loader"

message/pipeline/extract.go
23:     "golang.org/x/tools/go/callgraph"
24:     "golang.org/x/tools/go/callgraph/cha"
25:     "golang.org/x/tools/go/loader"
26:     "golang.org/x/tools/go/ssa"
27:     "golang.org/x/tools/go/ssa/ssautil"

message/pipeline/generate.go
23:     "golang.org/x/tools/go/loader"

message/pipeline/pipeline.go
28:     "golang.org/x/tools/go/loader"

Nothing in language.

And is language using anything that requires tools ?

$ cd language
$ rg "golang.org/x/"

coverage.go
11:     "golang.org/x/text/internal/language"

gen.go
22:     "golang.org/x/text/internal/gen"
23:     "golang.org/x/text/internal/language"
24:     "golang.org/x/text/unicode/cldr"

display/lookup.go
15:     "golang.org/x/text/language"

display/maketables.go
21:     "golang.org/x/text/internal/gen"
22:     "golang.org/x/text/language"
23:     "golang.org/x/text/unicode/cldr"

display/display.go
15:package display // import "golang.org/x/text/language/display"
21:     "golang.org/x/text/internal/format"
22:     "golang.org/x/text/language"
44:// Format implements "golang.org/x/text/internal/format".Formatter.

display/examples_test.go
10:     "golang.org/x/text/language"
11:     "golang.org/x/text/language/display"
12:     "golang.org/x/text/message"

display/dict_test.go
11:     "golang.org/x/text/internal/testtext"
33:     "golang.org/x/text/language"
34:     "golang.org/x/text/language/display"

display/display_test.go
14:     "golang.org/x/text/internal/testtext"
15:     "golang.org/x/text/language"
16:     "golang.org/x/text/message"

examples_test.go
11:     "golang.org/x/text/language"

tags.go
7:import "golang.org/x/text/internal/language/compact"

httpexample_test.go
12:     "golang.org/x/text/language"

match.go
11:     "golang.org/x/text/internal/language"

parse_test.go
11:     "golang.org/x/text/internal/language"

tables.go
1:// Code generated by running "go generate" in golang.org/x/text. DO NOT EDIT.

parse.go
13:     "golang.org/x/text/internal/language"

language.go
15:     "golang.org/x/text/internal/language"
16:     "golang.org/x/text/internal/language/compact"

doc.go
95:package language // import "golang.org/x/text/language"

match_test.go
17:     "golang.org/x/text/internal/testtext"
18:     "golang.org/x/text/internal/ucd"

coverage_test.go
12:     "golang.org/x/text/internal/language"

Yes, once, but in a test file.


Now, is go-i18n using anything else than golang.org/x/text/language ?

Short answer : NO.

Conclusion

We definitely DO NOT NEED THAT BLOAT.

Perhaps I'm missing some magical tree shaking or something that would mitigate this issue ?

After all, I'm still quite the noob in Go. 😝

Plan

  1. Make a leaner language lib, going against the core
  2. Convince go-i18n to use that leaner lib
  3. ???
  4. Profit!

I'm not about to do step 1 without some serious confirmation that I'm not horribly mistaken about all this. Even so, going against the core libs is … frightening, and not something I'm likely to do.

@Goutte
Copy link
Author

Goutte commented Nov 7, 2023

The gotext command in x/text does use x/tools, but our command in go-i18n, i18n_extract, appears not to ; they go straight for go/ast and go/parser.

Again, I might be totally wrong.
But I can't shake the feeling that including net and crypto libs where they're not actually needed is a security vulnerability.

@marckhouzam
Copy link
Collaborator

Might be replaced with #2090

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature A feature request for cobra; new or enhanced behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants