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.

Sign up for GitHub

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

Jump to bottom

Implement localization using gettext files — I18N — L10N #2090

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

Implement localization using gettext files — I18N — L10N #2090

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ab14efa
feat(i18n): implement localization using gettext files
Goutte Apr 4, 2023
c922911
feat(i18n): only embed locale files when using the build tag 'locales'
Goutte Jan 10, 2024
20e0d82
feat(i18n): add a Makefile recipe to install i18n extraction dependen…
Goutte Jan 10, 2024
f7f98c4
chore(i18n): lint
Goutte Jan 10, 2024
d362950
feat(i18n): test the locales using the appropriate build flag
Goutte Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion Makefile
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@ lint:

test: install_deps
$(info ******************** running tests ********************)
go test -v ./...
LANGUAGE="en" go test -v ./...

richtest: install_deps
$(info ******************** running tests with kyoh86/richgo ********************)
Expand All @@ -33,3 +33,7 @@ install_deps:

clean:
rm -rf $(BIN)

i18n_extract:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do I understand correctly that whenever a file changes, this make target must be re-run to update the line numbers in the default.pot file?

If that is the case, then it is essential that we run this make target in .github/test.yml and check if default.pot (or anything else) gets changed. I don't expect many contributors to know they have to run this, so CI should fail if they don't

$(info ******************** extracting translation files ********************)
xgotext -v -in . -out locales
Goutte marked this conversation as resolved.
Show resolved Hide resolved
15 changes: 8 additions & 7 deletions args.go
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@

import (
"fmt"
"github.com/leonelquinteros/gotext"

Check failure on line 19 in args.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

File is not `goimports`-ed (goimports)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could import this as i18n, so that instead of calling gotext.Get() we would call i18n.Get().
This may be clearer when browsing the code?

"strings"
)

Expand All @@ -33,15 +34,15 @@

// root command with subcommands, do subcommand checking.
if !cmd.HasParent() && len(args) > 0 {
return fmt.Errorf("unknown command %q for %q%s", args[0], cmd.CommandPath(), cmd.findSuggestions(args[0]))
return fmt.Errorf(gotext.Get("LegacyArgsValidationError"), args[0], cmd.CommandPath(), cmd.findSuggestions(args[0]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about leaving the original english string as the index for all the gotext.Get() calls?
It would make reading the code much easier and it would make working with the *.po files easier as the original text would be right there as the index.

If the % formatting gives a problem, maybe we can replace it with %% in the index? It is not ideal, but it would be manageable. So, for this line here we would instead use (unless there is a better way?)

return fmt.Errorf(gotext.Get("unknown command %%q for %%q%%s"), args[0], cmd.CommandPath(), cmd.findSuggestions(args[0]))

Copy link
Collaborator

Choose a reason for hiding this comment

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

I’ve looked at gotext in more detail, and the % form will work just fine as long as the parameters are in the gotext.Get() call instead of outside.

Copy link
Author

Choose a reason for hiding this comment

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

This is the crux of the matter.

I've done a whole bunch of implementations of translations in the past, and every time I try to use plain english instead of keys I end up either regretting it or refactoring heavily to keys.

Here's some food for thought when using raw english as keys:

  • cosmetically changing the english string (typo, whitespace, etc.) invalidates all the existing translations (possibly hundreds of files need to be updated, for nothing)
  • compels usage of context shenanigans for strings that are the same in english in different contexts but different in other languages
  • too many gettext parsers out there choke on nasty translation ids like the "quoted q'tara" <tag>
  • sometimes it creates a bias towards english in the structure of strings, that need to be fixed by translators, straight in the code, once again invalidating all existing translations

I'm very aware that using keys makes the code harder to read and understand. This is mitigated a little by a careful choice of the wording of the key.

I had to pick one way or the other ; it was not an easy choice, but it's one I made many times and I decided to go with hindsight from past experiences.

I'm not adamant on this, quite the contrary.
I listed some of the key points of my decision above (probably forgot some) ; I'll let y'all be the final judges. Good luck !

}
return nil
}

// NoArgs returns an error if any args are included.
func NoArgs(cmd *Command, args []string) error {
if len(args) > 0 {
return fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath())
return fmt.Errorf(gotext.Get("NoArgsValidationError"), args[0], cmd.CommandPath())
}
return nil
}
Expand All @@ -58,7 +59,7 @@
}
for _, v := range args {
if !stringInSlice(v, validArgs) {
return fmt.Errorf("invalid argument %q for %q%s", v, cmd.CommandPath(), cmd.findSuggestions(args[0]))
return fmt.Errorf(gotext.Get("OnlyValidArgsValidationError"), v, cmd.CommandPath(), cmd.findSuggestions(args[0]))
}
}
}
Expand All @@ -74,7 +75,7 @@
func MinimumNArgs(n int) PositionalArgs {
return func(cmd *Command, args []string) error {
if len(args) < n {
return fmt.Errorf("requires at least %d arg(s), only received %d", n, len(args))
return fmt.Errorf(gotext.GetN("MinimumNArgsValidationError", "MinimumNArgsValidationErrorPlural", n), n, len(args))
}
return nil
}
Expand All @@ -84,7 +85,7 @@
func MaximumNArgs(n int) PositionalArgs {
return func(cmd *Command, args []string) error {
if len(args) > n {
return fmt.Errorf("accepts at most %d arg(s), received %d", n, len(args))
return fmt.Errorf(gotext.GetN("MaximumNArgsValidationError", "MaximumNArgsValidationErrorPlural", n), n, len(args))
}
return nil
}
Expand All @@ -94,7 +95,7 @@
func ExactArgs(n int) PositionalArgs {
return func(cmd *Command, args []string) error {
if len(args) != n {
return fmt.Errorf("accepts %d arg(s), received %d", n, len(args))
return fmt.Errorf(gotext.GetN("ExactArgsValidationError", "ExactArgsValidationErrorPlural", n), n, len(args))
}
return nil
}
Expand All @@ -104,7 +105,7 @@
func RangeArgs(min int, max int) PositionalArgs {
return func(cmd *Command, args []string) error {
if len(args) < min || len(args) > max {
return fmt.Errorf("accepts between %d and %d arg(s), received %d", min, max, len(args))
return fmt.Errorf(gotext.GetN("RangeArgsValidationError", "RangeArgsValidationErrorPlural", max), min, max, len(args))
}
return nil
}
Expand Down
8 changes: 4 additions & 4 deletions args_test.go
View file
Open in desktop
Expand Up @@ -68,7 +68,7 @@ func minimumNArgsWithLessArgs(err error, t *testing.T) {
t.Fatal("Expected an error")
}
got := err.Error()
expected := "requires at least 2 arg(s), only received 1"
expected := "requires at least 2 args, only received 1"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
Expand All @@ -79,7 +79,7 @@ func maximumNArgsWithMoreArgs(err error, t *testing.T) {
t.Fatal("Expected an error")
}
got := err.Error()
expected := "accepts at most 2 arg(s), received 3"
expected := "accepts at most 2 args, received 3"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
Expand All @@ -90,7 +90,7 @@ func exactArgsWithInvalidCount(err error, t *testing.T) {
t.Fatal("Expected an error")
}
got := err.Error()
expected := "accepts 2 arg(s), received 3"
expected := "accepts 2 args, received 3"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
Expand All @@ -101,7 +101,7 @@ func rangeArgsWithInvalidCount(err error, t *testing.T) {
t.Fatal("Expected an error")
}
got := err.Error()
expected := "accepts between 2 and 4 arg(s), received 1"
expected := "accepts between 2 and 4 args, received 1"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
Expand Down
3 changes: 2 additions & 1 deletion cobra.go
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@

import (
"fmt"
"github.com/leonelquinteros/gotext"

Check failure on line 22 in cobra.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

File is not `goimports`-ed (goimports)
"io"
"os"
"reflect"
Expand Down Expand Up @@ -230,7 +231,7 @@
// CheckErr prints the msg with the prefix 'Error:' and exits with error code 1. If the msg is nil, it does nothing.
func CheckErr(msg interface{}) {
if msg != nil {
fmt.Fprintln(os.Stderr, "Error:", msg)
fmt.Fprintln(os.Stderr, gotext.Get("Error")+":", msg)
os.Exit(1)
}
}
Expand Down
56 changes: 33 additions & 23 deletions command.go
View file
Open in desktop
Expand Up @@ -21,6 +21,7 @@
"context"
"errors"
"fmt"
"github.com/leonelquinteros/gotext"

Check failure on line 24 in command.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

File is not `goimports`-ed (goimports)
"io"
"os"
"path/filepath"
Expand All @@ -44,6 +45,12 @@
Title string
}

// CommandUsageTemplateData is the data passed to the template of command usage
type CommandUsageTemplateData struct {
*Command
I18n *i18nCommandGlossary
}

// Command is just that, a command for your application.
// E.g. 'go run ...' - 'run' is the command. Cobra requires
// you to define the usage and description as part of your command
Expand Down Expand Up @@ -432,7 +439,11 @@
}
return func(c *Command) error {
c.mergePersistentFlags()
err := tmpl(c.OutOrStderr(), c.UsageTemplate(), c)
data := CommandUsageTemplateData{
Command: c,
I18n: getCommandGlossary(),
}
err := tmpl(c.OutOrStderr(), c.UsageTemplate(), data)
if err != nil {
c.PrintErrln(err)
}
Expand Down Expand Up @@ -549,35 +560,35 @@
if c.HasParent() {
return c.parent.UsageTemplate()
}
return `Usage:{{if .Runnable}}
return `{{.I18n.SectionUsage}}:{{if .Runnable}}
{{.UseLine}}{{end}}{{if .HasAvailableSubCommands}}
{{.CommandPath}} [command]{{end}}{{if gt (len .Aliases) 0}}

Aliases:
{{.I18n.SectionAliases}}:
{{.NameAndAliases}}{{end}}{{if .HasExample}}

Examples:
{{.I18n.SectionExamples}}:
{{.Example}}{{end}}{{if .HasAvailableSubCommands}}{{$cmds := .Commands}}{{if eq (len .Groups) 0}}

Available Commands:{{range $cmds}}{{if (or .IsAvailableCommand (eq .Name "help"))}}
{{.I18n.SectionAvailableCommands}}:{{range $cmds}}{{if (or .IsAvailableCommand (eq .Name "help"))}}
{{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{else}}{{range $group := .Groups}}

{{.Title}}{{range $cmds}}{{if (and (eq .GroupID $group.ID) (or .IsAvailableCommand (eq .Name "help")))}}
{{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{if not .AllChildCommandsHaveGroup}}

Additional Commands:{{range $cmds}}{{if (and (eq .GroupID "") (or .IsAvailableCommand (eq .Name "help")))}}
{{.I18n.SectionAdditionalCommands}}:{{range $cmds}}{{if (and (eq .GroupID "") (or .IsAvailableCommand (eq .Name "help")))}}
{{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{end}}{{if .HasAvailableLocalFlags}}

Flags:
{{.I18n.SectionFlags}}:
{{.LocalFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{if .HasAvailableInheritedFlags}}

Global Flags:
{{.I18n.SectionGlobalFlags}}:
{{.InheritedFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{if .HasHelpSubCommands}}

Additional help topics:{{range .Commands}}{{if .IsAdditionalHelpTopicCommand}}
{{.I18n.SectionAdditionalHelpTopics}}:{{range .Commands}}{{if .IsAdditionalHelpTopicCommand}}
{{rpad .CommandPath .CommandPathPadding}} {{.Short}}{{end}}{{end}}{{end}}{{if .HasAvailableSubCommands}}

Use "{{.CommandPath}} [command] --help" for more information about a command.{{end}}
{{.I18n.Use}} "{{.CommandPath}} [command] --help" {{.I18n.ForInfoAboutCommand}}.{{end}}
`
}

Expand Down Expand Up @@ -756,7 +767,7 @@
}
var sb strings.Builder
if suggestions := c.SuggestionsFor(arg); len(suggestions) > 0 {
sb.WriteString("\n\nDid you mean this?\n")
sb.WriteString("\n\n" + gotext.Get("DidYouMeanThis") + "\n")
for _, s := range suggestions {
_, _ = fmt.Fprintf(&sb, "\t%v\n", s)
}
Expand Down Expand Up @@ -877,7 +888,7 @@
}

if len(c.Deprecated) > 0 {
c.Printf("Command %q is deprecated, %s\n", c.Name(), c.Deprecated)
c.Printf(gotext.Get("CommandDeprecatedWarning")+"\n", c.Name(), c.Deprecated)
}

// initialize help and version flag at the last point possible to allow for user
Expand Down Expand Up @@ -1096,7 +1107,7 @@
}
if !c.SilenceErrors {
c.PrintErrln(c.ErrPrefix(), err.Error())
c.PrintErrf("Run '%v --help' for usage.\n", c.CommandPath())
c.PrintErrf(gotext.Get("RunHelpTip")+"\n", c.CommandPath())
}
return c, err
}
Expand Down Expand Up @@ -1162,7 +1173,7 @@
})

if len(missingFlagNames) > 0 {
return fmt.Errorf(`required flag(s) "%s" not set`, strings.Join(missingFlagNames, `", "`))
return fmt.Errorf(gotext.GetN("FlagNotSetError", "FlagNotSetErrorPlural", len(missingFlagNames)), strings.Join(missingFlagNames, `", "`))
}
return nil
}
Expand All @@ -1186,9 +1197,9 @@
func (c *Command) InitDefaultHelpFlag() {
c.mergePersistentFlags()
if c.Flags().Lookup("help") == nil {
usage := "help for "
usage := gotext.Get("HelpFor") + " "
if c.Name() == "" {
usage += "this command"
usage += gotext.Get("ThisCommand")
} else {
usage += c.Name()
}
Expand All @@ -1208,9 +1219,9 @@

c.mergePersistentFlags()
if c.Flags().Lookup("version") == nil {
usage := "version for "
usage := gotext.Get("VersionFor") + " "
if c.Name() == "" {
usage += "this command"
usage += gotext.Get("ThisCommand")
} else {
usage += c.Name()
}
Expand All @@ -1233,10 +1244,9 @@

if c.helpCommand == nil {
c.helpCommand = &Command{
Use: "help [command]",
Short: "Help about any command",
Long: `Help provides help for any command in the application.
Simply type ` + c.Name() + ` help [path to command] for full details.`,
Use: fmt.Sprintf("help [%s]", gotext.Get("command")),
Short: gotext.Get("CommandHelpShort"),
Long: fmt.Sprintf(gotext.Get("CommandHelpLong"), c.Name()+fmt.Sprintf(" help [%s]", gotext.Get("command"))),
ValidArgsFunction: func(c *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
var completions []string
cmd, _, e := c.Root().Find(args)
Expand All @@ -1259,7 +1269,7 @@
Run: func(c *Command, args []string) {
cmd, _, e := c.Root().Find(args)
if cmd == nil || e != nil {
c.Printf("Unknown help topic %#q\n", args)
c.Printf(gotext.Get("CommandHelpUnknownTopicError")+"\n", args)
CheckErr(c.Root().Usage())
} else {
cmd.InitDefaultHelpFlag() // make possible 'help' flag to be shown
Expand Down
19 changes: 17 additions & 2 deletions command_test.go
View file
Open in desktop
Expand Up @@ -815,6 +815,21 @@ func TestPersistentFlagsOnChild(t *testing.T) {
}
}

func TestRequiredFlag(t *testing.T) {
c := &Command{Use: "c", Run: emptyRun}
c.Flags().String("foo1", "", "")
assertNoErr(t, c.MarkFlagRequired("foo1"))

expected := fmt.Sprintf("required flag %q is not set", "foo1")

_, err := executeCommand(c)
got := err.Error()

if got != expected {
t.Errorf("Expected error: %q, got: %q", expected, got)
}
}

func TestRequiredFlags(t *testing.T) {
c := &Command{Use: "c", Run: emptyRun}
c.Flags().String("foo1", "", "")
Expand All @@ -823,7 +838,7 @@ func TestRequiredFlags(t *testing.T) {
assertNoErr(t, c.MarkFlagRequired("foo2"))
c.Flags().String("bar", "", "")

expected := fmt.Sprintf("required flag(s) %q, %q not set", "foo1", "foo2")
expected := fmt.Sprintf("required flags %q, %q are not set", "foo1", "foo2")

_, err := executeCommand(c)
got := err.Error()
Expand All @@ -850,7 +865,7 @@ func TestPersistentRequiredFlags(t *testing.T) {

parent.AddCommand(child)

expected := fmt.Sprintf("required flag(s) %q, %q, %q, %q not set", "bar1", "bar2", "foo1", "foo2")
expected := fmt.Sprintf("required flags %q, %q, %q, %q are not set", "bar1", "bar2", "foo1", "foo2")

_, err := executeCommand(parent, "child")
if err.Error() != expected {
Expand Down