From 8487a18169b322a125de6a1180867f4ab688c449 Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Tue, 13 Oct 2020 15:56:54 +0530 Subject: [PATCH 1/3] Warn when an unknown config key is set --- pkg/cmd/config/config.go | 42 +++++++++++++++++++++++++++-------- pkg/cmd/config/config_test.go | 14 +++++++++++- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go index 408975629..43c608f5a 100644 --- a/pkg/cmd/config/config.go +++ b/pkg/cmd/config/config.go @@ -8,19 +8,31 @@ import ( "github.com/spf13/cobra" ) +type ConfigKey struct { + name string + description string +} + +var respectedConfigKeys []ConfigKey = []ConfigKey{ + {"git_protocol", `"https" or "ssh". Default is "https".`}, + {"editor", `if unset, defaults to environment variables.`}, + {"prompt", `"enabled" or "disabled". Toggles interactive prompting.`}, + {"pager", `terminal pager program to send standard output to.`}, +} + func NewCmdConfig(f *cmdutil.Factory) *cobra.Command { + longDoc := ` +Display or change configuration settings for gh. + +Current respected settings: +` + for _, configKey := range respectedConfigKeys { + longDoc += fmt.Sprintf("- %s: %s\n", configKey.name, configKey.description) + } cmd := &cobra.Command{ Use: "config", Short: "Manage configuration for gh", - Long: heredoc.Doc(` - Display or change configuration settings for gh. - - Current respected settings: - - git_protocol: "https" or "ssh". Default is "https". - - editor: if unset, defaults to environment variables. - - prompt: "enabled" or "disabled". Toggles interactive prompting. - - pager: terminal pager program to send standard output to. - `), + Long: heredoc.Doc(longDoc), } cmdutil.DisableAuthCheck(cmd) @@ -85,6 +97,18 @@ func NewCmdConfigSet(f *cmdutil.Factory) *cobra.Command { } key, value := args[0], args[1] + knownKey := false + for _, configKey := range respectedConfigKeys { + if key == configKey.name { + 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 { return fmt.Errorf("failed to set %q to %q: %w", key, value, err) diff --git a/pkg/cmd/config/config_test.go b/pkg/cmd/config/config_test.go index 20c7dd543..3a3d9c83a 100644 --- a/pkg/cmd/config/config_test.go +++ b/pkg/cmd/config/config_test.go @@ -117,6 +117,7 @@ func TestConfigSet(t *testing.T) { config configStub args []string expectKey string + expectVal string stdout string stderr string }{ @@ -125,6 +126,7 @@ func TestConfigSet(t *testing.T) { config: configStub{}, args: []string{"editor", "vim"}, expectKey: "editor", + expectVal: "vim", stdout: "", stderr: "", }, @@ -133,9 +135,19 @@ func TestConfigSet(t *testing.T) { 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) { @@ -158,7 +170,7 @@ func TestConfigSet(t *testing.T) { assert.Equal(t, tt.stdout, stdout.String()) assert.Equal(t, tt.stderr, stderr.String()) - assert.Equal(t, "vim", tt.config[tt.expectKey]) + assert.Equal(t, tt.expectVal, tt.config[tt.expectKey]) assert.Equal(t, "true", tt.config["_written"]) }) } From ae7d42a876226f204e213f121031c4aa4abf5814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 26 Oct 2020 14:42:04 +0100 Subject: [PATCH 2/3] Centralize all known configuration options --- cmd/gh/main.go | 2 +- internal/config/config_type.go | 55 +++++++++++++++++++++++++--------- pkg/cmd/config/config.go | 35 ++++++++-------------- 3 files changed, 55 insertions(+), 37 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index c393a627e..38ca1db84 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -71,7 +71,7 @@ func main() { os.Exit(2) } - if prompt, _ := cfg.Get("", "prompt"); prompt == config.PromptsDisabled { + if prompt, _ := cfg.Get("", "prompt"); prompt == "disabled" { cmdFactory.IOStreams.SetNeverPrompt(true) } diff --git a/internal/config/config_type.go b/internal/config/config_type.go index a28521f0e..1a179c41d 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -10,11 +10,41 @@ import ( "gopkg.in/yaml.v3" ) -const ( - defaultGitProtocol = "https" - PromptsDisabled = "disabled" - PromptsEnabled = "enabled" -) +type ConfigOption struct { + Key string + Description string + DefaultValue string + AllowedValues []string +} + +var configOptions = []ConfigOption{ + { + Key: "git_protocol", + Description: "the protocol to use for git clone and push operations", + DefaultValue: "https", + AllowedValues: []string{"https", "ssh"}, + }, + { + Key: "editor", + Description: "the text editor program to use for authoring text", + DefaultValue: "", + }, + { + Key: "prompt", + Description: "toggle interactive prompting in the terminal", + DefaultValue: "enabled", + AllowedValues: []string{"enabled", "disabled"}, + }, + { + Key: "pager", + Description: "the terminal pager program to send standard output to", + DefaultValue: "", + }, +} + +func ConfigOptions() []ConfigOption { + return configOptions +} var configValues = map[string][]string{ "git_protocol": {"ssh", "https"}, @@ -183,7 +213,7 @@ func NewBlankRoot() *yaml.Node { }, { Kind: yaml.ScalarNode, - Value: PromptsEnabled, + Value: "enabled", }, { HeadComment: "A pager program to send command output to, e.g. \"less\". Set the value to \"cat\" to disable the pager.", @@ -525,13 +555,10 @@ func (c *fileConfig) parseHosts(hostsEntry *yaml.Node) ([]*HostConfig, error) { } func defaultFor(key string) string { - // we only have a set default for one setting right now - switch key { - case "git_protocol": - return defaultGitProtocol - case "prompt": - return PromptsEnabled - default: - return "" + for _, co := range configOptions { + if co.Key == key { + return co.DefaultValue + } } + return "" } diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go index f9cbbc9d1..5c8b51507 100644 --- a/pkg/cmd/config/config.go +++ b/pkg/cmd/config/config.go @@ -11,31 +11,22 @@ import ( "github.com/spf13/cobra" ) -type ConfigKey struct { - name string - description string -} - -var respectedConfigKeys []ConfigKey = []ConfigKey{ - {"git_protocol", `"https" or "ssh". Default is "https".`}, - {"editor", `if unset, defaults to environment variables.`}, - {"prompt", `"enabled" or "disabled". Toggles interactive prompting.`}, - {"pager", `terminal pager program to send standard output to.`}, -} - func NewCmdConfig(f *cmdutil.Factory) *cobra.Command { - longDoc := ` -Display or change configuration settings for gh. - -Current respected settings: -` - for _, configKey := range respectedConfigKeys { - longDoc += fmt.Sprintf("- %s: %s\n", configKey.name, configKey.description) + longDoc := strings.Builder{} + longDoc.WriteString("Display or change configuration settings for gh.\n\n") + longDoc.WriteString("Current respected settings:\n") + for _, co := range config.ConfigOptions() { + longDoc.WriteString(fmt.Sprintf("- %s: %s", co.Key, co.Description)) + if co.DefaultValue != "" { + longDoc.WriteString(fmt.Sprintf(" (default: %q)", co.DefaultValue)) + } + longDoc.WriteRune('\n') } + cmd := &cobra.Command{ Use: "config", Short: "Manage configuration for gh", - Long: heredoc.Doc(longDoc), + Long: longDoc.String(), } cmdutil.DisableAuthCheck(cmd) @@ -101,8 +92,8 @@ func NewCmdConfigSet(f *cmdutil.Factory) *cobra.Command { key, value := args[0], args[1] knownKey := false - for _, configKey := range respectedConfigKeys { - if key == configKey.name { + for _, configKey := range config.ConfigOptions() { + if key == configKey.Key { knownKey = true break } From 2d04292d446321ba7fe3d235fc16042f4748f948 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 26 Oct 2020 14:42:24 +0100 Subject: [PATCH 3/3] Improve `git config set git_protocol` example It's better to set this per-host in general, since setting the top-level `git_protocol` key will not have any effect if the per-host setting exists, and it does by default for anyone who ran `auth login`. --- pkg/cmd/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go index 5c8b51507..72dbb6955 100644 --- a/pkg/cmd/config/config.go +++ b/pkg/cmd/config/config.go @@ -80,7 +80,7 @@ func NewCmdConfigSet(f *cmdutil.Factory) *cobra.Command { Example: heredoc.Doc(` $ gh config set editor vim $ gh config set editor "code --wait" - $ gh config set git_protocol ssh + $ gh config set git_protocol ssh --host github.com $ gh config set prompt disabled `), Args: cobra.ExactArgs(2),