Fixed locales (translations) not being detected with default config (#2724)

* Fixed locales (translations) not being detected with default config

* Fixed integration test

* Apply i18n settings only if specified

* Fixed settings.GetLocale()

* Do not print directly to stdout if locale-detection fails (windows)

* Give LANG env var priority (windows)

Author: cmaglie

Diff for: commands/instances.go

@@ -419,7 +419,9 @@ func (s *arduinoCoreServerImpl) Init(req *rpc.InitRequest, stream rpc.ArduinoCor
 	// Refreshes the locale used, this will change the
 	// language of the CLI if the locale is different
 	// after started.
-	i18n.Init(s.settings.GetString("locale"))
+	if locale, ok, _ := s.settings.GetStringOk("locale"); ok {
+		i18n.Init(locale)
+	}
 
 	return nil
 }

Diff for: commands/service.go

@@ -19,20 +19,14 @@ import (
 	"context"
 
 	"github.com/arduino/arduino-cli/internal/cli/configuration"
-	"github.com/arduino/arduino-cli/internal/i18n"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/arduino-cli/version"
 )
 
 // NewArduinoCoreServer returns an implementation of the ArduinoCoreService gRPC service
 // that uses the provided version string.
 func NewArduinoCoreServer() rpc.ArduinoCoreServiceServer {
-	settings := configuration.NewSettings()
-
-	// Setup i18n
-	i18n.Init(settings.Locale())
-
-	return &arduinoCoreServerImpl{settings: settings}
+	return &arduinoCoreServerImpl{settings: configuration.NewSettings()}
 }
 
 type arduinoCoreServerImpl struct {

Diff for: commands/service_settings_test.go

@@ -62,7 +62,6 @@ func TestGetAll(t *testing.T) {
 			"user": `+defaultUserDir.GetEncodedValue()+`
 		},
 		"library": {},
-		"locale": "en",
 		"logging": {
 			"format": "text",
 			"level": "info"

Diff for: internal/cli/configuration/defaults.go

@@ -73,7 +73,7 @@ func SetDefaults(settings *Settings) {
 	setKeyTypeSchema("network.user_agent_ext", "")
 
 	// locale
-	setDefaultValueAndKeyTypeSchema("locale", "en")
+	setKeyTypeSchema("locale", "")
 }
 
 // InjectEnvVars change settings based on the environment variables values

Diff for: internal/cli/configuration/locale.go

@@ -16,5 +16,8 @@
 package configuration
 
 func (s *Settings) Locale() string {
+	if locale, ok, err := s.GetStringOk("locale"); ok && err == nil {
+		return locale
+	}
 	return s.Defaults.GetString("locale")
 }

Diff for: internal/i18n/detect_windows.go

@@ -16,19 +16,24 @@
 package i18n
 
 import (
-	"fmt"
 	"strings"
 	"syscall"
 	"unsafe"
+
+	"github.com/sirupsen/logrus"
 )
 
 func getLocaleIdentifier() string {
 	defer func() {
 		if r := recover(); r != nil {
-			fmt.Println("failed to get windows user locale", r)
+			logrus.WithField("error", r).Errorf("Failed to get windows user locale")
 		}
 	}()
 
+	if loc := getLocaleIdentifierFromEnv(); loc != "" {
+		return loc
+	}
+
 	dll := syscall.MustLoadDLL("kernel32")
 	defer dll.Release()
 	proc := dll.MustFindProc("GetUserDefaultLocaleName")

Diff for: internal/integrationtest/config/config_test.go

@@ -882,25 +882,25 @@ build.unk: 123
 	t.Cleanup(func() { unkwnownConfig.Remove() })
 
 	// Run "config get" with a configuration containing an unknown key
-	out, _, err := cli.Run("config", "get", "locale", "--config-file", unkwnownConfig.String())
+	out, _, err := cli.Run("config", "get", "daemon.port", "--config-file", unkwnownConfig.String())
 	require.NoError(t, err)
-	require.Equal(t, "en", strings.TrimSpace(string(out)))
+	require.Equal(t, `"50051"`, strings.TrimSpace(string(out)))
 
-	// Run "config get" with a configuration containing an invalid key
+	// Run "config get" with a configuration containing an invalid value
 	invalidConfig := paths.New(filepath.Join(tmp, "invalid.yaml"))
-	invalidConfig.WriteFile([]byte(`locale: 123`))
+	invalidConfig.WriteFile([]byte(`daemon.port: 123`))
 	t.Cleanup(func() { invalidConfig.Remove() })
-	out, _, err = cli.Run("config", "get", "locale", "--config-file", invalidConfig.String())
+	out, _, err = cli.Run("config", "get", "daemon.port", "--config-file", invalidConfig.String())
 	require.NoError(t, err)
-	require.Equal(t, "en", strings.TrimSpace(string(out)))
+	require.Equal(t, `"50051"`, strings.TrimSpace(string(out)))
 
 	// Run "config get" with a configuration containing a null array
 	nullArrayConfig := paths.New(filepath.Join(tmp, "null_array.yaml"))
 	nullArrayConfig.WriteFile([]byte(`board_manager.additional_urls:`))
 	t.Cleanup(func() { nullArrayConfig.Remove() })
-	out, _, err = cli.Run("config", "get", "locale", "--config-file", invalidConfig.String())
+	out, _, err = cli.Run("config", "get", "daemon.port", "--config-file", nullArrayConfig.String())
 	require.NoError(t, err)
-	require.Equal(t, "en", strings.TrimSpace(string(out)))
+	require.Equal(t, `"50051"`, strings.TrimSpace(string(out)))
 }
 
 func TestConfigViaEnvVars(t *testing.T) {
@@ -931,3 +931,16 @@ func TestConfigViaEnvVars(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, "20\n\n", string(out))
 }
+
+func TestI18N(t *testing.T) {
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	out, _, err := cli.RunWithCustomEnv(map[string]string{"LC_ALL": "it"})
+	require.NoError(t, err)
+	require.Contains(t, string(out), "Comandi disponibili")
+
+	out, _, err = cli.RunWithCustomEnv(map[string]string{"LC_ALL": "en"})
+	require.NoError(t, err)
+	require.Contains(t, string(out), "Available Commands")
+}