diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 1a179c41d..40533f211 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -46,9 +46,45 @@ func ConfigOptions() []ConfigOption { return configOptions } -var configValues = map[string][]string{ - "git_protocol": {"ssh", "https"}, - "prompt": {"enabled", "disabled"}, +func ValidateKey(key string) error { + for _, configKey := range configOptions { + if key == configKey.Key { + return nil + } + } + + return fmt.Errorf("invalid key") +} + +type InvalidValueError struct { + ValidValues []string +} + +func (e InvalidValueError) Error() string { + return "invalid value" +} + +func ValidateValue(key, value string) error { + var validValues []string + + for _, v := range configOptions { + if v.Key == key { + validValues = v.AllowedValues + break + } + } + + if validValues == nil { + return nil + } + + for _, v := range validValues { + if v == value { + return nil + } + } + + return &InvalidValueError{ValidValues: validValues} } // This interface describes interacting with some persistent configuration for gh. @@ -306,33 +342,7 @@ func (c *fileConfig) GetWithSource(hostname, key string) (string, string, error) return value, defaultSource, nil } -type InvalidValueError struct { - ValidValues []string -} - -func (e InvalidValueError) Error() string { - return "invalid value" -} - -func validateConfigEntry(key, value string) error { - validValues, found := configValues[key] - if !found { - return nil - } - - for _, v := range validValues { - if v == value { - return nil - } - } - - return &InvalidValueError{ValidValues: validValues} -} - func (c *fileConfig) Set(hostname, key, value string) error { - if err := validateConfigEntry(key, value); err != nil { - return err - } if hostname == "" { return c.SetStringValue(key, value) } else { diff --git a/internal/config/config_type_test.go b/internal/config/config_type_test.go index 46b9016f4..47295230f 100644 --- a/internal/config/config_type_test.go +++ b/internal/config/config_type_test.go @@ -28,7 +28,6 @@ func Test_fileConfig_Set(t *testing.T) { example.com: editor: vim `, hostsBuf.String()) - assert.EqualError(t, c.Set("github.com", "git_protocol", "sshpps"), "invalid value") } func Test_defaultConfig(t *testing.T) { @@ -70,16 +69,33 @@ func Test_defaultConfig(t *testing.T) { assert.Equal(t, expansion, "pr checkout") } -func Test_validateConfigEntry(t *testing.T) { - err := validateConfigEntry("git_protocol", "sshpps") +func Test_ValidateValue(t *testing.T) { + err := ValidateValue("git_protocol", "sshpps") assert.EqualError(t, err, "invalid value") - err = validateConfigEntry("git_protocol", "ssh") + err = ValidateValue("git_protocol", "ssh") assert.Nil(t, err) - err = validateConfigEntry("editor", "vim") + err = ValidateValue("editor", "vim") assert.Nil(t, err) - err = validateConfigEntry("got", "123") + err = ValidateValue("got", "123") assert.Nil(t, err) } + +func Test_ValidateKey(t *testing.T) { + err := ValidateKey("invalid") + assert.EqualError(t, err, "invalid key") + + err = ValidateKey("git_protocol") + assert.NoError(t, err) + + err = ValidateKey("editor") + assert.NoError(t, err) + + err = ValidateKey("prompt") + assert.NoError(t, err) + + err = ValidateKey("pager") + assert.NoError(t, err) +} diff --git a/internal/config/stub.go b/internal/config/stub.go new file mode 100644 index 000000000..57761dac5 --- /dev/null +++ b/internal/config/stub.go @@ -0,0 +1,51 @@ +package config + +import ( + "errors" +) + +type ConfigStub map[string]string + +func genKey(host, key string) string { + if host != "" { + return host + ":" + key + } + return key +} + +func (c ConfigStub) Get(host, key string) (string, error) { + val, _, err := c.GetWithSource(host, key) + return val, err +} + +func (c ConfigStub) GetWithSource(host, key string) (string, string, error) { + if v, found := c[genKey(host, key)]; found { + return v, "(memory)", nil + } + return "", "", errors.New("not found") +} + +func (c ConfigStub) Set(host, key, value string) error { + c[genKey(host, key)] = value + return nil +} + +func (c ConfigStub) Aliases() (*AliasConfig, error) { + return nil, nil +} + +func (c ConfigStub) Hosts() ([]string, error) { + return nil, nil +} + +func (c ConfigStub) UnsetHost(hostname string) { +} + +func (c ConfigStub) CheckWriteable(host, key string) error { + return nil +} + +func (c ConfigStub) Write() error { + c["_written"] = "true" + return nil +} diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go index 72dbb6955..7cecb66cb 100644 --- a/pkg/cmd/config/config.go +++ b/pkg/cmd/config/config.go @@ -1,12 +1,12 @@ package config import ( - "errors" "fmt" "strings" - "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/config" + cmdGet "github.com/cli/cli/pkg/cmd/config/get" + cmdSet "github.com/cli/cli/pkg/cmd/config/set" "github.com/cli/cli/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -31,100 +31,8 @@ func NewCmdConfig(f *cmdutil.Factory) *cobra.Command { cmdutil.DisableAuthCheck(cmd) - cmd.AddCommand(NewCmdConfigGet(f)) - cmd.AddCommand(NewCmdConfigSet(f)) - - return cmd -} - -func NewCmdConfigGet(f *cmdutil.Factory) *cobra.Command { - var hostname string - - cmd := &cobra.Command{ - Use: "get ", - Short: "Print the value of a given configuration key", - Example: heredoc.Doc(` - $ gh config get git_protocol - https - `), - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - cfg, err := f.Config() - if err != nil { - return err - } - - val, err := cfg.Get(hostname, args[0]) - if err != nil { - return err - } - - if val != "" { - fmt.Fprintf(f.IOStreams.Out, "%s\n", val) - } - return nil - }, - } - - cmd.Flags().StringVarP(&hostname, "host", "h", "", "Get per-host setting") - - return cmd -} - -func NewCmdConfigSet(f *cmdutil.Factory) *cobra.Command { - var hostname string - - cmd := &cobra.Command{ - Use: "set ", - Short: "Update configuration with a value for the given key", - Example: heredoc.Doc(` - $ gh config set editor vim - $ gh config set editor "code --wait" - $ gh config set git_protocol ssh --host github.com - $ gh config set prompt disabled - `), - Args: cobra.ExactArgs(2), - RunE: func(cmd *cobra.Command, args []string) error { - cfg, err := f.Config() - if err != nil { - return err - } - - key, value := args[0], args[1] - knownKey := false - for _, configKey := range config.ConfigOptions() { - if key == configKey.Key { - knownKey = true - break - } - } - if !knownKey { - iostreams := f.IOStreams - warningIcon := iostreams.ColorScheme().WarningIcon() - fmt.Fprintf(iostreams.ErrOut, "%s warning: '%s' is not a known configuration key\n", warningIcon, key) - } - err = cfg.Set(hostname, key, value) - if err != nil { - var invalidValue *config.InvalidValueError - if errors.As(err, &invalidValue) { - var values []string - for _, v := range invalidValue.ValidValues { - values = append(values, fmt.Sprintf("'%s'", v)) - } - return fmt.Errorf("failed to set %q to %q: valid values are %v", key, value, strings.Join(values, ", ")) - } - return fmt.Errorf("failed to set %q to %q: %w", key, value, err) - } - - err = cfg.Write() - if err != nil { - return fmt.Errorf("failed to write config to disk: %w", err) - } - return nil - }, - } - - cmd.Flags().StringVarP(&hostname, "host", "h", "", "Set per-host setting") + cmd.AddCommand(cmdGet.NewCmdConfigGet(f, nil)) + cmd.AddCommand(cmdSet.NewCmdConfigSet(f, nil)) return cmd } diff --git a/pkg/cmd/config/config_test.go b/pkg/cmd/config/config_test.go deleted file mode 100644 index 3a3d9c83a..000000000 --- a/pkg/cmd/config/config_test.go +++ /dev/null @@ -1,177 +0,0 @@ -package config - -import ( - "errors" - "testing" - - "github.com/cli/cli/internal/config" - "github.com/cli/cli/pkg/cmdutil" - "github.com/cli/cli/pkg/iostreams" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -type configStub map[string]string - -func genKey(host, key string) string { - if host != "" { - return host + ":" + key - } - return key -} - -func (c configStub) Get(host, key string) (string, error) { - val, _, err := c.GetWithSource(host, key) - return val, err -} - -func (c configStub) GetWithSource(host, key string) (string, string, error) { - if v, found := c[genKey(host, key)]; found { - return v, "(memory)", nil - } - return "", "", errors.New("not found") -} - -func (c configStub) Set(host, key, value string) error { - c[genKey(host, key)] = value - return nil -} - -func (c configStub) Aliases() (*config.AliasConfig, error) { - return nil, nil -} - -func (c configStub) Hosts() ([]string, error) { - return nil, nil -} - -func (c configStub) UnsetHost(hostname string) { -} - -func (c configStub) CheckWriteable(host, key string) error { - return nil -} - -func (c configStub) Write() error { - c["_written"] = "true" - return nil -} - -func TestConfigGet(t *testing.T) { - tests := []struct { - name string - config configStub - args []string - stdout string - stderr string - }{ - { - name: "get key", - config: configStub{ - "editor": "ed", - }, - args: []string{"editor"}, - stdout: "ed\n", - stderr: "", - }, - { - name: "get key scoped by host", - config: configStub{ - "editor": "ed", - "github.com:editor": "vim", - }, - args: []string{"editor", "-h", "github.com"}, - stdout: "vim\n", - stderr: "", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - io, _, stdout, stderr := iostreams.Test() - f := &cmdutil.Factory{ - IOStreams: io, - Config: func() (config.Config, error) { - return tt.config, nil - }, - } - - cmd := NewCmdConfigGet(f) - cmd.Flags().BoolP("help", "x", false, "") - cmd.SetArgs(tt.args) - cmd.SetOut(stdout) - cmd.SetErr(stderr) - - _, err := cmd.ExecuteC() - require.NoError(t, err) - - assert.Equal(t, tt.stdout, stdout.String()) - assert.Equal(t, tt.stderr, stderr.String()) - assert.Equal(t, "", tt.config["_written"]) - }) - } -} - -func TestConfigSet(t *testing.T) { - tests := []struct { - name string - config configStub - args []string - expectKey string - expectVal string - stdout string - stderr string - }{ - { - name: "set key", - config: configStub{}, - args: []string{"editor", "vim"}, - expectKey: "editor", - expectVal: "vim", - stdout: "", - stderr: "", - }, - { - name: "set key scoped by host", - config: configStub{}, - args: []string{"editor", "vim", "-h", "github.com"}, - expectKey: "github.com:editor", - expectVal: "vim", - stdout: "", - stderr: "", - }, - { - name: "set key", - config: configStub{}, - args: []string{"unknownKey", "someValue"}, - expectKey: "unknownKey", - expectVal: "someValue", - stdout: "", - stderr: "! warning: 'unknownKey' is not a known configuration key\n", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - io, _, stdout, stderr := iostreams.Test() - f := &cmdutil.Factory{ - IOStreams: io, - Config: func() (config.Config, error) { - return tt.config, nil - }, - } - - cmd := NewCmdConfigSet(f) - cmd.Flags().BoolP("help", "x", false, "") - cmd.SetArgs(tt.args) - cmd.SetOut(stdout) - cmd.SetErr(stderr) - - _, err := cmd.ExecuteC() - require.NoError(t, err) - - assert.Equal(t, tt.stdout, stdout.String()) - assert.Equal(t, tt.stderr, stderr.String()) - assert.Equal(t, tt.expectVal, tt.config[tt.expectKey]) - assert.Equal(t, "true", tt.config["_written"]) - }) - } -} diff --git a/pkg/cmd/config/get/get.go b/pkg/cmd/config/get/get.go new file mode 100644 index 000000000..a3ce55176 --- /dev/null +++ b/pkg/cmd/config/get/get.go @@ -0,0 +1,65 @@ +package get + +import ( + "fmt" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +type GetOptions struct { + IO *iostreams.IOStreams + Config config.Config + + Hostname string + Key string +} + +func NewCmdConfigGet(f *cmdutil.Factory, runF func(*GetOptions) error) *cobra.Command { + opts := &GetOptions{ + IO: f.IOStreams, + } + + cmd := &cobra.Command{ + Use: "get ", + Short: "Print the value of a given configuration key", + Example: heredoc.Doc(` + $ gh config get git_protocol + https + `), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + config, err := f.Config() + if err != nil { + return err + } + opts.Config = config + opts.Key = args[0] + + if runF != nil { + return runF(opts) + } + + return getRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Hostname, "host", "h", "", "Get per-host setting") + + return cmd +} + +func getRun(opts *GetOptions) error { + val, err := opts.Config.Get(opts.Hostname, opts.Key) + if err != nil { + return err + } + + if val != "" { + fmt.Fprintf(opts.IO.Out, "%s\n", val) + } + return nil +} diff --git a/pkg/cmd/config/get/get_test.go b/pkg/cmd/config/get/get_test.go new file mode 100644 index 000000000..1e2104dbd --- /dev/null +++ b/pkg/cmd/config/get/get_test.go @@ -0,0 +1,122 @@ +package get + +import ( + "bytes" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdConfigGet(t *testing.T) { + tests := []struct { + name string + input string + output GetOptions + wantsErr bool + }{ + { + name: "no arguments", + input: "", + output: GetOptions{}, + wantsErr: true, + }, + { + name: "get key", + input: "key", + output: GetOptions{Key: "key"}, + wantsErr: false, + }, + { + name: "get key with host", + input: "key --host test.com", + output: GetOptions{Hostname: "test.com", Key: "key"}, + wantsErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return config.ConfigStub{}, nil + }, + } + + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + + var gotOpts *GetOptions + cmd := NewCmdConfigGet(f, func(opts *GetOptions) error { + gotOpts = opts + return nil + }) + cmd.Flags().BoolP("help", "x", false, "") + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + if tt.wantsErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.output.Hostname, gotOpts.Hostname) + assert.Equal(t, tt.output.Key, gotOpts.Key) + }) + } +} + +func Test_getRun(t *testing.T) { + tests := []struct { + name string + input *GetOptions + stdout string + stderr string + wantErr bool + }{ + { + name: "get key", + input: &GetOptions{ + Key: "editor", + Config: config.ConfigStub{ + "editor": "ed", + }, + }, + stdout: "ed\n", + }, + { + name: "get key scoped by host", + input: &GetOptions{ + Hostname: "github.com", + Key: "editor", + Config: config.ConfigStub{ + "editor": "ed", + "github.com:editor": "vim", + }, + }, + stdout: "vim\n", + }, + } + + for _, tt := range tests { + io, _, stdout, stderr := iostreams.Test() + tt.input.IO = io + + t.Run(tt.name, func(t *testing.T) { + err := getRun(tt.input) + assert.NoError(t, err) + assert.Equal(t, tt.stdout, stdout.String()) + assert.Equal(t, tt.stderr, stderr.String()) + _, err = tt.input.Config.Get("", "_written") + assert.Error(t, err) + }) + } +} diff --git a/pkg/cmd/config/set/set.go b/pkg/cmd/config/set/set.go new file mode 100644 index 000000000..d1a2d5c59 --- /dev/null +++ b/pkg/cmd/config/set/set.go @@ -0,0 +1,90 @@ +package set + +import ( + "errors" + "fmt" + "strings" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +type SetOptions struct { + IO *iostreams.IOStreams + Config config.Config + + Key string + Value string + Hostname string +} + +func NewCmdConfigSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { + opts := &SetOptions{ + IO: f.IOStreams, + } + + cmd := &cobra.Command{ + Use: "set ", + Short: "Update configuration with a value for the given key", + Example: heredoc.Doc(` + $ gh config set editor vim + $ gh config set editor "code --wait" + $ gh config set git_protocol ssh --host github.com + $ gh config set prompt disabled + `), + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + config, err := f.Config() + if err != nil { + return err + } + opts.Config = config + opts.Key = args[0] + opts.Value = args[1] + + if runF != nil { + return runF(opts) + } + + return setRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Hostname, "host", "h", "", "Set per-host setting") + + return cmd +} + +func setRun(opts *SetOptions) error { + err := config.ValidateKey(opts.Key) + if err != nil { + warningIcon := opts.IO.ColorScheme().WarningIcon() + fmt.Fprintf(opts.IO.ErrOut, "%s warning: '%s' is not a known configuration key\n", warningIcon, opts.Key) + } + + err = config.ValidateValue(opts.Key, opts.Value) + if err != nil { + var invalidValue *config.InvalidValueError + if errors.As(err, &invalidValue) { + var values []string + for _, v := range invalidValue.ValidValues { + values = append(values, fmt.Sprintf("'%s'", v)) + } + return fmt.Errorf("failed to set %q to %q: valid values are %v", opts.Key, opts.Value, strings.Join(values, ", ")) + } + } + + err = opts.Config.Set(opts.Hostname, opts.Key, opts.Value) + if err != nil { + return fmt.Errorf("failed to set %q to %q: %w", opts.Key, opts.Value, err) + } + + err = opts.Config.Write() + if err != nil { + return fmt.Errorf("failed to write config to disk: %w", err) + } + return nil +} diff --git a/pkg/cmd/config/set/set_test.go b/pkg/cmd/config/set/set_test.go new file mode 100644 index 000000000..56efc2ebd --- /dev/null +++ b/pkg/cmd/config/set/set_test.go @@ -0,0 +1,157 @@ +package set + +import ( + "bytes" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdConfigSet(t *testing.T) { + tests := []struct { + name string + input string + output SetOptions + wantsErr bool + }{ + { + name: "no arguments", + input: "", + output: SetOptions{}, + wantsErr: true, + }, + { + name: "no value argument", + input: "key", + output: SetOptions{}, + wantsErr: true, + }, + { + name: "set key value", + input: "key value", + output: SetOptions{Key: "key", Value: "value"}, + wantsErr: false, + }, + { + name: "set key value with host", + input: "key value --host test.com", + output: SetOptions{Hostname: "test.com", Key: "key", Value: "value"}, + wantsErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return config.ConfigStub{}, nil + }, + } + + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + + var gotOpts *SetOptions + cmd := NewCmdConfigSet(f, func(opts *SetOptions) error { + gotOpts = opts + return nil + }) + cmd.Flags().BoolP("help", "x", false, "") + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + if tt.wantsErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.output.Hostname, gotOpts.Hostname) + assert.Equal(t, tt.output.Key, gotOpts.Key) + assert.Equal(t, tt.output.Value, gotOpts.Value) + }) + } +} + +func Test_setRun(t *testing.T) { + tests := []struct { + name string + input *SetOptions + expectedValue string + stdout string + stderr string + wantsErr bool + errMsg string + }{ + { + name: "set key value", + input: &SetOptions{ + Config: config.ConfigStub{}, + Key: "editor", + Value: "vim", + }, + expectedValue: "vim", + }, + { + name: "set key value scoped by host", + input: &SetOptions{ + Config: config.ConfigStub{}, + Hostname: "github.com", + Key: "editor", + Value: "vim", + }, + expectedValue: "vim", + }, + { + name: "set unknown key", + input: &SetOptions{ + Config: config.ConfigStub{}, + Key: "unknownKey", + Value: "someValue", + }, + expectedValue: "someValue", + stderr: "! warning: 'unknownKey' is not a known configuration key\n", + }, + { + name: "set invalid value", + input: &SetOptions{ + Config: config.ConfigStub{}, + Key: "git_protocol", + Value: "invalid", + }, + wantsErr: true, + errMsg: "failed to set \"git_protocol\" to \"invalid\": valid values are 'https', 'ssh'", + }, + } + for _, tt := range tests { + io, _, stdout, stderr := iostreams.Test() + tt.input.IO = io + + t.Run(tt.name, func(t *testing.T) { + err := setRun(tt.input) + if tt.wantsErr { + assert.EqualError(t, err, tt.errMsg) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.stdout, stdout.String()) + assert.Equal(t, tt.stderr, stderr.String()) + + val, err := tt.input.Config.Get(tt.input.Hostname, tt.input.Key) + assert.NoError(t, err) + assert.Equal(t, tt.expectedValue, val) + + val, err = tt.input.Config.Get("", "_written") + assert.NoError(t, err) + assert.Equal(t, "true", val) + }) + } +}