From f3bf1684d94da20ad732338e3f073fbf8557d7e9 Mon Sep 17 00:00:00 2001 From: ardnew Date: Sat, 9 Sep 2023 23:11:34 -0500 Subject: [PATCH 01/10] add "config get" command to print settings values --- internal/cli/config/config.go | 1 + internal/cli/config/get.go | 88 +++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 internal/cli/config/get.go diff --git a/internal/cli/config/config.go b/internal/cli/config/config.go index 38666248f91..c59714478d3 100644 --- a/internal/cli/config/config.go +++ b/internal/cli/config/config.go @@ -37,6 +37,7 @@ func NewCommand() *cobra.Command { configCommand.AddCommand(initAddCommand()) configCommand.AddCommand(initDeleteCommand()) configCommand.AddCommand(initDumpCommand()) + configCommand.AddCommand(initGetCommand()) configCommand.AddCommand(initInitCommand()) configCommand.AddCommand(initRemoveCommand()) configCommand.AddCommand(initSetCommand()) diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go new file mode 100644 index 00000000000..e9cc9ba497d --- /dev/null +++ b/internal/cli/config/get.go @@ -0,0 +1,88 @@ +// This file is part of arduino-cli. +// +// Copyright 2020 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package config + +import ( + "os" + "reflect" + + "github.com/arduino/arduino-cli/configuration" + "github.com/arduino/arduino-cli/internal/cli/feedback" + "github.com/sirupsen/logrus" + "github.com/spf13/cobra" + "gopkg.in/yaml.v2" +) + +func initGetCommand() *cobra.Command { + getCommand := &cobra.Command{ + Use: "get", + Short: tr("Gets a setting value."), + Long: tr("Gets a setting value."), + Example: "" + + " " + os.Args[0] + " config get logging.level\n" + + " " + os.Args[0] + " config get logging.file\n" + + " " + os.Args[0] + " config get sketch.always_export_binaries\n" + + " " + os.Args[0] + " config get board_manager.additional_urls", + Args: cobra.MinimumNArgs(1), + Run: runGetCommand, + ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return configuration.Settings.AllKeys(), cobra.ShellCompDirectiveDefault + }, + } + return getCommand +} + +func runGetCommand(cmd *cobra.Command, args []string) { + logrus.Info("Executing `arduino-cli config get`") + key := args[0] + kind := validateKey(key) + + if kind != reflect.Slice && len(args) > 1 { + feedback.Fatal(tr("Can't get multiple key values"), feedback.ErrGeneric) + } + + var value interface{} + switch kind { + case reflect.Slice: + value = configuration.Settings.GetStringSlice(key) + case reflect.String: + value = configuration.Settings.GetString(key) + case reflect.Bool: + value = configuration.Settings.GetBool(key) + } + + feedback.PrintResult(getResult{value}) +} + +// output from this command may require special formatting. +// create a dedicated feedback.Result implementation to safely handle +// any changes to the configuration.Settings struct. +type getResult struct { + data interface{} +} + +func (gr getResult) Data() interface{} { + return gr.data +} + +func (gr getResult) String() string { + gs, err := yaml.Marshal(gr.data) + if err != nil { + // Should never happen + panic(tr("unable to marshal config to YAML: %v", err)) + } + return string(gs) +} From 76eca4dd762507514f63b9a9dec79491617af376 Mon Sep 17 00:00:00 2001 From: ardnew Date: Sat, 9 Sep 2023 23:52:53 -0500 Subject: [PATCH 02/10] use RPC for "config get" and add test cases --- internal/cli/config/get.go | 32 +++++--------- .../integrationtest/config/config_test.go | 43 +++++++++++++++++++ 2 files changed, 54 insertions(+), 21 deletions(-) diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go index e9cc9ba497d..eb337e2062e 100644 --- a/internal/cli/config/get.go +++ b/internal/cli/config/get.go @@ -29,12 +29,11 @@ import ( func initGetCommand() *cobra.Command { getCommand := &cobra.Command{ Use: "get", - Short: tr("Gets a setting value."), - Long: tr("Gets a setting value."), + Short: tr("Gets settings key values."), + Long: tr("Gets settings key values."), Example: "" + - " " + os.Args[0] + " config get logging.level\n" + - " " + os.Args[0] + " config get logging.file\n" + - " " + os.Args[0] + " config get sketch.always_export_binaries\n" + + " " + os.Args[0] + " config get logging\n" + + " " + os.Args[0] + " config get logging.level logging.file\n" + " " + os.Args[0] + " config get board_manager.additional_urls", Args: cobra.MinimumNArgs(1), Run: runGetCommand, @@ -47,24 +46,15 @@ func initGetCommand() *cobra.Command { func runGetCommand(cmd *cobra.Command, args []string) { logrus.Info("Executing `arduino-cli config get`") - key := args[0] - kind := validateKey(key) - if kind != reflect.Slice && len(args) > 1 { - feedback.Fatal(tr("Can't get multiple key values"), feedback.ErrGeneric) + svc := daemon.ArduinoCoreServerImpl{} + for _, toGet := range args { + resp, err := svc.SettingsGetValue(cmd.Context(), &rpc.SettingsGetValueRequest{Key: toGet}) + if err != nil { + feedback.Fatal(tr("Cannot get the key %[1]s: %[2]v", toGet, err), feedback.ErrGeneric) + } + feedback.PrintResult(resp.GetJsonData()) } - - var value interface{} - switch kind { - case reflect.Slice: - value = configuration.Settings.GetStringSlice(key) - case reflect.String: - value = configuration.Settings.GetString(key) - case reflect.Bool: - value = configuration.Settings.GetBool(key) - } - - feedback.PrintResult(getResult{value}) } // output from this command may require special formatting. diff --git a/internal/integrationtest/config/config_test.go b/internal/integrationtest/config/config_test.go index 2d830978c98..9f3cdf53a4c 100644 --- a/internal/integrationtest/config/config_test.go +++ b/internal/integrationtest/config/config_test.go @@ -818,6 +818,49 @@ func TestDelete(t *testing.T) { require.NotContains(t, configLines, "board_manager") } +func TestGet(t *testing.T) { + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + defer env.CleanUp() + + // Create a config file + _, _, err := cli.Run("config", "init", "--dest-dir", ".") + require.NoError(t, err) + + // Verifies default state + stdout, _, err := cli.Run("config", "dump", "--format", "json", "--config-file", "arduino-cli.yaml") + require.NoError(t, err) + requirejson.Query(t, stdout, ".config | .daemon | .port", `"50051"`) + + // Get simple key value + stdout, _, err = cli.Run("config", "get", "daemon.port", "--format", "json", "--config-file", "arduino-cli.yaml") + require.NoError(t, err) + require.Equal(t, `"50051"`, stdout) + + // Get structured key value + stdout, _, err = cli.Run("config", "get", "daemon", "--format", "json", "--config-file", "arduino-cli.yaml") + require.NoError(t, err) + require.Equal(t, `{"port":"50051"}`, stdout) + + // Get multiple key values + stdout, _, err = cli.Run("config", "get", "logging.format", "logging.level", "--format", "json", "--config-file", "arduino-cli.yaml") + require.NoError(t, err) + require.Equal(t,`"text"` + "\n" + `"info"`, stdout) + + // Get undefined key + stdout, _, err = cli.Run("config", "get", "foo", "--format", "json", "--config-file", "arduino-cli.yaml") + require.Empty(t, stdout) + require.Contains(t, err, "Cannot get key foo") + + // Set undefined key + _, _, err = cli.Run("config", "set", "foo", "bar", "--config-file", "arduino-cli.yaml") + require.NoError(t, err) + + // Get previously-undefined key + stdout, _, err = cli.Run("config", "get", "foo", "--format", "json", "--config-file", "arduino-cli.yaml") + require.NoError(t, err) + require.Equal(t, `"bar"`, stdout) +} + func TestInitializationOrderOfConfigThroughFlagAndEnv(t *testing.T) { env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) defer env.CleanUp() From e5f0881ac8616eb7ecec8814db0b88cb1e92a093 Mon Sep 17 00:00:00 2001 From: ardnew Date: Thu, 25 Jan 2024 15:07:59 -0600 Subject: [PATCH 03/10] update imports for changed internal layout --- internal/cli/config/get.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go index eb337e2062e..ebdd61875db 100644 --- a/internal/cli/config/get.go +++ b/internal/cli/config/get.go @@ -19,11 +19,11 @@ import ( "os" "reflect" - "github.com/arduino/arduino-cli/configuration" + "github.com/arduino/arduino-cli/internal/cli/configuration" "github.com/arduino/arduino-cli/internal/cli/feedback" "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" ) func initGetCommand() *cobra.Command { From 926d850ea02ab304ffaaa92a24bf101cc9f62656 Mon Sep 17 00:00:00 2001 From: ardnew Date: Thu, 25 Jan 2024 15:27:27 -0600 Subject: [PATCH 04/10] config/get: do not wrap JSON output in YAML --- internal/cli/config/get.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go index ebdd61875db..3d6706498c5 100644 --- a/internal/cli/config/get.go +++ b/internal/cli/config/get.go @@ -17,13 +17,13 @@ package config import ( "os" - "reflect" + "github.com/arduino/arduino-cli/commands/daemon" "github.com/arduino/arduino-cli/internal/cli/configuration" "github.com/arduino/arduino-cli/internal/cli/feedback" + rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "gopkg.in/yaml.v3" ) func initGetCommand() *cobra.Command { @@ -53,7 +53,7 @@ func runGetCommand(cmd *cobra.Command, args []string) { if err != nil { feedback.Fatal(tr("Cannot get the key %[1]s: %[2]v", toGet, err), feedback.ErrGeneric) } - feedback.PrintResult(resp.GetJsonData()) + feedback.PrintResult(getResult{key: toGet, data: resp.GetJsonData()}) } } @@ -61,6 +61,7 @@ func runGetCommand(cmd *cobra.Command, args []string) { // create a dedicated feedback.Result implementation to safely handle // any changes to the configuration.Settings struct. type getResult struct { + key string data interface{} } @@ -69,10 +70,10 @@ func (gr getResult) Data() interface{} { } func (gr getResult) String() string { - gs, err := yaml.Marshal(gr.data) - if err != nil { + gs, ok := gr.data.(string) + if !ok { // Should never happen - panic(tr("unable to marshal config to YAML: %v", err)) + panic(tr("Cannot get key %s value as string: %v", gr.key, gr.data)) } - return string(gs) + return gs } From 6efcca6a8e2af9d0fa07964bf9c2615727338fca Mon Sep 17 00:00:00 2001 From: ardnew Date: Thu, 25 Jan 2024 15:31:11 -0600 Subject: [PATCH 05/10] fix formatting with gofmt --- internal/cli/config/get.go | 4 ++-- internal/integrationtest/config/config_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go index 3d6706498c5..a0583fd0455 100644 --- a/internal/cli/config/get.go +++ b/internal/cli/config/get.go @@ -47,7 +47,7 @@ func initGetCommand() *cobra.Command { func runGetCommand(cmd *cobra.Command, args []string) { logrus.Info("Executing `arduino-cli config get`") - svc := daemon.ArduinoCoreServerImpl{} + svc := daemon.ArduinoCoreServerImpl{} for _, toGet := range args { resp, err := svc.SettingsGetValue(cmd.Context(), &rpc.SettingsGetValueRequest{Key: toGet}) if err != nil { @@ -61,7 +61,7 @@ func runGetCommand(cmd *cobra.Command, args []string) { // create a dedicated feedback.Result implementation to safely handle // any changes to the configuration.Settings struct. type getResult struct { - key string + key string data interface{} } diff --git a/internal/integrationtest/config/config_test.go b/internal/integrationtest/config/config_test.go index 9f3cdf53a4c..1d109e09f5a 100644 --- a/internal/integrationtest/config/config_test.go +++ b/internal/integrationtest/config/config_test.go @@ -844,7 +844,7 @@ func TestGet(t *testing.T) { // Get multiple key values stdout, _, err = cli.Run("config", "get", "logging.format", "logging.level", "--format", "json", "--config-file", "arduino-cli.yaml") require.NoError(t, err) - require.Equal(t,`"text"` + "\n" + `"info"`, stdout) + require.Equal(t, `"text"`+"\n"+`"info"`, stdout) // Get undefined key stdout, _, err = cli.Run("config", "get", "foo", "--format", "json", "--config-file", "arduino-cli.yaml") From d67f3d7689737dd10dc184ebdc8e3e90d5893108 Mon Sep 17 00:00:00 2001 From: ardnew Date: Thu, 25 Jan 2024 16:39:11 -0600 Subject: [PATCH 06/10] config/get: unmarshal JSON RPC response --- internal/cli/config/get.go | 27 ++++++++++--------- .../integrationtest/config/config_test.go | 24 ++++------------- 2 files changed, 19 insertions(+), 32 deletions(-) diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go index a0583fd0455..e09c73d1f84 100644 --- a/internal/cli/config/get.go +++ b/internal/cli/config/get.go @@ -16,6 +16,8 @@ package config import ( + "encoding/json" + "fmt" "os" "github.com/arduino/arduino-cli/commands/daemon" @@ -29,11 +31,11 @@ import ( func initGetCommand() *cobra.Command { getCommand := &cobra.Command{ Use: "get", - Short: tr("Gets settings key values."), - Long: tr("Gets settings key values."), + Short: tr("Gets a settings key value."), + Long: tr("Gets a settings key value."), Example: "" + " " + os.Args[0] + " config get logging\n" + - " " + os.Args[0] + " config get logging.level logging.file\n" + + " " + os.Args[0] + " config get daemon.port\n" + " " + os.Args[0] + " config get board_manager.additional_urls", Args: cobra.MinimumNArgs(1), Run: runGetCommand, @@ -53,7 +55,12 @@ func runGetCommand(cmd *cobra.Command, args []string) { if err != nil { feedback.Fatal(tr("Cannot get the key %[1]s: %[2]v", toGet, err), feedback.ErrGeneric) } - feedback.PrintResult(getResult{key: toGet, data: resp.GetJsonData()}) + var result getResult + err = json.Unmarshal([]byte(resp.GetJsonData()), &result.resp) + if err != nil { + feedback.Fatal(tr("Cannot parse JSON for key %[1]s: %[2]v", toGet, err), feedback.ErrGeneric) + } + feedback.PrintResult(result) } } @@ -61,19 +68,13 @@ func runGetCommand(cmd *cobra.Command, args []string) { // create a dedicated feedback.Result implementation to safely handle // any changes to the configuration.Settings struct. type getResult struct { - key string - data interface{} + resp interface{} } func (gr getResult) Data() interface{} { - return gr.data + return gr.resp } func (gr getResult) String() string { - gs, ok := gr.data.(string) - if !ok { - // Should never happen - panic(tr("Cannot get key %s value as string: %v", gr.key, gr.data)) - } - return gs + return fmt.Sprintf("%v", gr.resp) } diff --git a/internal/integrationtest/config/config_test.go b/internal/integrationtest/config/config_test.go index 1d109e09f5a..b8c5036e0da 100644 --- a/internal/integrationtest/config/config_test.go +++ b/internal/integrationtest/config/config_test.go @@ -834,31 +834,17 @@ func TestGet(t *testing.T) { // Get simple key value stdout, _, err = cli.Run("config", "get", "daemon.port", "--format", "json", "--config-file", "arduino-cli.yaml") require.NoError(t, err) - require.Equal(t, `"50051"`, stdout) + requirejson.Contains(t, stdout, `"50051"`) // Get structured key value stdout, _, err = cli.Run("config", "get", "daemon", "--format", "json", "--config-file", "arduino-cli.yaml") require.NoError(t, err) - require.Equal(t, `{"port":"50051"}`, stdout) - - // Get multiple key values - stdout, _, err = cli.Run("config", "get", "logging.format", "logging.level", "--format", "json", "--config-file", "arduino-cli.yaml") - require.NoError(t, err) - require.Equal(t, `"text"`+"\n"+`"info"`, stdout) + requirejson.Contains(t, stdout, `{"port":"50051"}`) // Get undefined key - stdout, _, err = cli.Run("config", "get", "foo", "--format", "json", "--config-file", "arduino-cli.yaml") - require.Empty(t, stdout) - require.Contains(t, err, "Cannot get key foo") - - // Set undefined key - _, _, err = cli.Run("config", "set", "foo", "bar", "--config-file", "arduino-cli.yaml") - require.NoError(t, err) - - // Get previously-undefined key - stdout, _, err = cli.Run("config", "get", "foo", "--format", "json", "--config-file", "arduino-cli.yaml") - require.NoError(t, err) - require.Equal(t, `"bar"`, stdout) + _, stderr, err := cli.Run("config", "get", "foo", "--format", "json", "--config-file", "arduino-cli.yaml") + require.Error(t, err) + requirejson.Contains(t, stderr, `{"error":"Cannot get the key foo: key not found in settings"}`) } func TestInitializationOrderOfConfigThroughFlagAndEnv(t *testing.T) { From ba04e6101c9bde93845695da990694b8eef170b4 Mon Sep 17 00:00:00 2001 From: ardnew Date: Thu, 8 Feb 2024 17:30:31 -0600 Subject: [PATCH 07/10] Apply suggestions from code review Co-authored-by: Cristian Maglie --- internal/cli/config/get.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go index e09c73d1f84..3459742cc74 100644 --- a/internal/cli/config/get.go +++ b/internal/cli/config/get.go @@ -53,12 +53,13 @@ func runGetCommand(cmd *cobra.Command, args []string) { for _, toGet := range args { resp, err := svc.SettingsGetValue(cmd.Context(), &rpc.SettingsGetValueRequest{Key: toGet}) if err != nil { - feedback.Fatal(tr("Cannot get the key %[1]s: %[2]v", toGet, err), feedback.ErrGeneric) + feedback.Fatal(tr("Cannot get the configuration key %[1]s: %[2]v", toGet, err), feedback.ErrGeneric) } var result getResult err = json.Unmarshal([]byte(resp.GetJsonData()), &result.resp) if err != nil { - feedback.Fatal(tr("Cannot parse JSON for key %[1]s: %[2]v", toGet, err), feedback.ErrGeneric) + // Should never happen... + panic(fmt.Sprintf("Cannot parse JSON for key %[1]s: %[2]v", toGet, err)) } feedback.PrintResult(result) } From c069ecea005cfa6a54bdc5165c0104d837d4c94a Mon Sep 17 00:00:00 2001 From: ardnew Date: Thu, 8 Feb 2024 17:41:32 -0600 Subject: [PATCH 08/10] use same default format as "config dump" --- internal/cli/config/get.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go index 3459742cc74..6fe8f59e9bc 100644 --- a/internal/cli/config/get.go +++ b/internal/cli/config/get.go @@ -17,7 +17,6 @@ package config import ( "encoding/json" - "fmt" "os" "github.com/arduino/arduino-cli/commands/daemon" @@ -26,6 +25,7 @@ import ( rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "gopkg.in/yaml.v3" ) func initGetCommand() *cobra.Command { @@ -77,5 +77,10 @@ func (gr getResult) Data() interface{} { } func (gr getResult) String() string { - return fmt.Sprintf("%v", gr.resp) + gs, err := yaml.Marshal(gr.resp) + if err != nil { + // Should never happen + panic(tr("unable to marshal config to YAML: %v", err)) + } + return string(gs) } From 6e5be88705a73127a9433364b3a4b6327fde5b58 Mon Sep 17 00:00:00 2001 From: ardnew Date: Thu, 8 Feb 2024 17:50:58 -0600 Subject: [PATCH 09/10] fix import missing after merging review changes remotely --- internal/cli/config/get.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go index 6fe8f59e9bc..cb2a5cb0072 100644 --- a/internal/cli/config/get.go +++ b/internal/cli/config/get.go @@ -17,6 +17,7 @@ package config import ( "encoding/json" + "fmt" "os" "github.com/arduino/arduino-cli/commands/daemon" From 294203132e1974c04413b3e8547072b595c8a469 Mon Sep 17 00:00:00 2001 From: ardnew Date: Tue, 13 Feb 2024 10:43:12 -0600 Subject: [PATCH 10/10] test (config): fix expected error message with "get " --- internal/integrationtest/config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/integrationtest/config/config_test.go b/internal/integrationtest/config/config_test.go index b8c5036e0da..91bb73af9f6 100644 --- a/internal/integrationtest/config/config_test.go +++ b/internal/integrationtest/config/config_test.go @@ -844,7 +844,7 @@ func TestGet(t *testing.T) { // Get undefined key _, stderr, err := cli.Run("config", "get", "foo", "--format", "json", "--config-file", "arduino-cli.yaml") require.Error(t, err) - requirejson.Contains(t, stderr, `{"error":"Cannot get the key foo: key not found in settings"}`) + requirejson.Contains(t, stderr, `{"error":"Cannot get the configuration key foo: key not found in settings"}`) } func TestInitializationOrderOfConfigThroughFlagAndEnv(t *testing.T) {