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

Implement localization using gettext files — I18N — L10N #2090

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,22 @@ lint:
test: install_deps
$(info ******************** running tests ********************)
go test -v ./...
LANGUAGE="en" go test -tags locales -v ./...

richtest: install_deps
$(info ******************** running tests with kyoh86/richgo ********************)
richgo test -v ./...

i18n_extract: install_i18n_deps
$(info ******************** extracting translation files ********************)
xgotext -v -in . -out locales

install_deps:
$(info ******************** downloading dependencies ********************)
go get -v ./...

install_i18n_deps:
go install github.com/leonelquinteros/gotext/cli/xgotext

clean:
rm -rf $(BIN)
16 changes: 9 additions & 7 deletions args.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package cobra
import (
"fmt"
"strings"

"github.com/leonelquinteros/gotext"
)

type PositionalArgs func(cmd *Command, args []string) error
Expand All @@ -33,15 +35,15 @@ func legacyArgs(cmd *Command, args []string) error {

// 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 +60,7 @@ func OnlyValidArgs(cmd *Command, args []string) error {
}
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 +76,7 @@ func ArbitraryArgs(cmd *Command, args []string) error {
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 +86,7 @@ func MinimumNArgs(n int) PositionalArgs {
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 +96,7 @@ func MaximumNArgs(n int) PositionalArgs {
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 +106,7 @@ func ExactArgs(n int) PositionalArgs {
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
Original file line number Diff line number Diff line change
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
4 changes: 3 additions & 1 deletion cobra.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"text/template"
"time"
"unicode"

"github.com/leonelquinteros/gotext"
)

var templateFuncs = template.FuncMap{
Expand Down Expand Up @@ -230,7 +232,7 @@ func stringInSlice(a string, list []string) bool {
// 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
57 changes: 34 additions & 23 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"sort"
"strings"

"github.com/leonelquinteros/gotext"

flag "github.com/spf13/pflag"
)

Expand All @@ -44,6 +46,12 @@ type Group struct {
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 +440,11 @@ func (c *Command) UsageFunc() (f func(*Command) error) {
}
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 +561,35 @@ func (c *Command) UsageTemplate() string {
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 +768,7 @@ func (c *Command) findSuggestions(arg string) string {
}
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 +889,7 @@ func (c *Command) execute(a []string) (err error) {
}

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 +1108,7 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
}
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 +1174,7 @@ func (c *Command) ValidateRequiredFlags() error {
})

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 +1198,9 @@ func (c *Command) checkCommandGroups() {
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 +1220,9 @@ func (c *Command) InitDefaultVersionFlag() {

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 +1245,9 @@ func (c *Command) InitDefaultHelpCmd() {

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 +1270,7 @@ Simply type ` + c.Name() + ` help [path to command] for full details.`,
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
Original file line number Diff line number Diff line change
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