diff --git a/internal/config/config.go b/internal/config/config.go index ad451ee97..fbdcdef95 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/keyring" + o "github.com/cli/cli/v2/pkg/option" ghAuth "github.com/cli/go-gh/v2/pkg/auth" ghConfig "github.com/cli/go-gh/v2/pkg/config" ) @@ -41,28 +42,44 @@ type cfg struct { cfg *ghConfig.Config } -func (c *cfg) Get(hostname, key string) (string, error) { +func (c *cfg) get(hostname, key string) o.Option[string] { if hostname != "" { val, err := c.cfg.Get([]string{hostsKey, hostname, key}) if err == nil { - return val, err + return o.Some(val) } } - return c.cfg.Get([]string{key}) + val, err := c.cfg.Get([]string{key}) + if err == nil { + return o.Some(val) + } + + return o.None[string]() } -func (c *cfg) GetOrDefault(hostname, key string) (string, error) { - val, err := c.Get(hostname, key) - if err == nil { - return val, err +func (c *cfg) GetOrDefault(hostname, key string) o.Option[gh.ConfigEntry] { + if val := c.get(hostname, key); val.IsSome() { + // Map the Option[string] to Option[gh.ConfigEntry] with a source of ConfigUserProvided + return o.Map(val, toConfigEntry(gh.ConfigUserProvided)) } - if val, ok := defaultFor(key); ok { - return val, nil + if defaultVal := defaultFor(key); defaultVal.IsSome() { + // Map the Option[string] to Option[gh.ConfigEntry] with a source of ConfigDefaultProvided + return o.Map(defaultVal, toConfigEntry(gh.ConfigDefaultProvided)) } - return val, err + return o.None[gh.ConfigEntry]() +} + +// toConfigEntry is a helper function to convert a string value to a ConfigEntry with a given source. +// +// It's a bit of FP style but it allows us to map an Option[string] to Option[gh.ConfigEntry] without +// unwrapping the it and rewrapping it. +func toConfigEntry(source gh.ConfigSource) func(val string) gh.ConfigEntry { + return func(val string) gh.ConfigEntry { + return gh.ConfigEntry{Value: val, Source: source} + } } func (c *cfg) Set(hostname, key, value string) { @@ -90,43 +107,44 @@ func (c *cfg) Authentication() gh.AuthConfig { return &AuthConfig{cfg: c.cfg} } -func (c *cfg) Browser(hostname string) string { - val, _ := c.GetOrDefault(hostname, browserKey) - return val +func (c *cfg) Browser(hostname string) gh.ConfigEntry { + // Intentionally panic if there is no user provided value or default value (which would be a programmer error) + return c.GetOrDefault(hostname, browserKey).Unwrap() } -func (c *cfg) Editor(hostname string) string { - val, _ := c.GetOrDefault(hostname, editorKey) - return val +func (c *cfg) Editor(hostname string) gh.ConfigEntry { + // Intentionally panic if there is no user provided value or default value (which would be a programmer error) + return c.GetOrDefault(hostname, editorKey).Unwrap() } -func (c *cfg) GitProtocol(hostname string) string { - val, _ := c.GetOrDefault(hostname, gitProtocolKey) - return val +func (c *cfg) GitProtocol(hostname string) gh.ConfigEntry { + // Intentionally panic if there is no user provided value or default value (which would be a programmer error) + return c.GetOrDefault(hostname, gitProtocolKey).Unwrap() } -func (c *cfg) HTTPUnixSocket(hostname string) string { - val, _ := c.GetOrDefault(hostname, httpUnixSocketKey) - return val +func (c *cfg) HTTPUnixSocket(hostname string) gh.ConfigEntry { + // Intentionally panic if there is no user provided value or default value (which would be a programmer error) + return c.GetOrDefault(hostname, httpUnixSocketKey).Unwrap() } -func (c *cfg) Pager(hostname string) string { - val, _ := c.GetOrDefault(hostname, pagerKey) - return val +func (c *cfg) Pager(hostname string) gh.ConfigEntry { + // Intentionally panic if there is no user provided value or default value (which would be a programmer error) + return c.GetOrDefault(hostname, pagerKey).Unwrap() } -func (c *cfg) Prompt(hostname string) string { - val, _ := c.GetOrDefault(hostname, promptKey) - return val +func (c *cfg) Prompt(hostname string) gh.ConfigEntry { + // Intentionally panic if there is no user provided value or default value (which would be a programmer error) + return c.GetOrDefault(hostname, promptKey).Unwrap() } -func (c *cfg) Version() string { - val, _ := c.GetOrDefault("", versionKey) - return val +func (c *cfg) Version() o.Option[string] { + return c.get("", versionKey) } func (c *cfg) Migrate(m gh.Migration) error { - version := c.Version() + // If there is no version entry we must never have applied a migration, and the following conditional logic + // handles the version as an empty string correctly. + version := c.Version().UnwrapOrZero() // If migration has already occurred then do not attempt to migrate again. if m.PostVersion() == version { @@ -156,13 +174,13 @@ func (c *cfg) CacheDir() string { return ghConfig.CacheDir() } -func defaultFor(key string) (string, bool) { - for _, co := range ConfigOptions() { +func defaultFor(key string) o.Option[string] { + for _, co := range Options { if co.Key == key { - return co.DefaultValue, true + return o.Some(co.DefaultValue) } } - return "", false + return o.None[string]() } // AuthConfig is used for interacting with some persistent configuration for gh, @@ -507,43 +525,60 @@ type ConfigOption struct { Description string DefaultValue string AllowedValues []string + CurrentValue func(c gh.Config, hostname string) string } -func ConfigOptions() []ConfigOption { - return []ConfigOption{ - { - Key: gitProtocolKey, - Description: "the protocol to use for git clone and push operations", - DefaultValue: "https", - AllowedValues: []string{"https", "ssh"}, +var Options = []ConfigOption{ + { + Key: gitProtocolKey, + Description: "the protocol to use for git clone and push operations", + DefaultValue: "https", + AllowedValues: []string{"https", "ssh"}, + CurrentValue: func(c gh.Config, hostname string) string { + return c.GitProtocol(hostname).Value }, - { - Key: editorKey, - Description: "the text editor program to use for authoring text", - DefaultValue: "", + }, + { + Key: editorKey, + Description: "the text editor program to use for authoring text", + DefaultValue: "", + CurrentValue: func(c gh.Config, hostname string) string { + return c.Editor(hostname).Value }, - { - Key: promptKey, - Description: "toggle interactive prompting in the terminal", - DefaultValue: "enabled", - AllowedValues: []string{"enabled", "disabled"}, + }, + { + Key: promptKey, + Description: "toggle interactive prompting in the terminal", + DefaultValue: "enabled", + AllowedValues: []string{"enabled", "disabled"}, + CurrentValue: func(c gh.Config, hostname string) string { + return c.Prompt(hostname).Value }, - { - Key: pagerKey, - Description: "the terminal pager program to send standard output to", - DefaultValue: "", + }, + { + Key: pagerKey, + Description: "the terminal pager program to send standard output to", + DefaultValue: "", + CurrentValue: func(c gh.Config, hostname string) string { + return c.Pager(hostname).Value }, - { - Key: httpUnixSocketKey, - Description: "the path to a Unix socket through which to make an HTTP connection", - DefaultValue: "", + }, + { + Key: httpUnixSocketKey, + Description: "the path to a Unix socket through which to make an HTTP connection", + DefaultValue: "", + CurrentValue: func(c gh.Config, hostname string) string { + return c.HTTPUnixSocket(hostname).Value }, - { - Key: browserKey, - Description: "the web browser to use for opening URLs", - DefaultValue: "", + }, + { + Key: browserKey, + Description: "the web browser to use for opening URLs", + DefaultValue: "", + CurrentValue: func(c gh.Config, hostname string) string { + return c.Browser(hostname).Value }, - } + }, } func HomeDirPath(subdir string) (string, error) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f6832f4a4..fef87ddc6 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,10 +1,12 @@ package config import ( + "fmt" "testing" "github.com/stretchr/testify/require" + "github.com/cli/cli/v2/internal/gh" ghConfig "github.com/cli/go-gh/v2/pkg/config" ) @@ -32,71 +34,6 @@ func TestNewConfigProvidesFallback(t *testing.T) { requireKeyWithValue(t, spiedCfg, []string{browserKey}, "") } -func TestGetNonExistentKey(t *testing.T) { - // Given we have no top level configuration - cfg := newTestConfig() - - // When we get a key that has no value - val, err := cfg.Get("", "non-existent-key") - - // Then it returns an error and the value is empty - var keyNotFoundError *ghConfig.KeyNotFoundError - require.ErrorAs(t, err, &keyNotFoundError) - require.Empty(t, val) -} - -func TestGetNonExistentHostSpecificKey(t *testing.T) { - // Given have no top level configuration - cfg := newTestConfig() - - // When we get a key for a host that has no value - val, err := cfg.Get("non-existent-host", "non-existent-key") - - // Then it returns an error and the value is empty - var keyNotFoundError *ghConfig.KeyNotFoundError - require.ErrorAs(t, err, &keyNotFoundError) - require.Empty(t, val) -} - -func TestGetExistingTopLevelKey(t *testing.T) { - // Given have a top level config entry - cfg := newTestConfig() - cfg.Set("", "top-level-key", "top-level-value") - - // When we get that key - val, err := cfg.Get("non-existent-host", "top-level-key") - - // Then it returns successfully with the correct value - require.NoError(t, err) - require.Equal(t, "top-level-value", val) -} - -func TestGetExistingHostSpecificKey(t *testing.T) { - // Given have a host specific config entry - cfg := newTestConfig() - cfg.Set("github.com", "host-specific-key", "host-specific-value") - - // When we get that key - val, err := cfg.Get("github.com", "host-specific-key") - - // Then it returns successfully with the correct value - require.NoError(t, err) - require.Equal(t, "host-specific-value", val) -} - -func TestGetHostnameSpecificKeyFallsBackToTopLevel(t *testing.T) { - // Given have a top level config entry - cfg := newTestConfig() - cfg.Set("", "key", "value") - - // When we get that key on a specific host - val, err := cfg.Get("github.com", "key") - - // Then it returns successfully, falling back to the top level config - require.NoError(t, err) - require.Equal(t, "value", val) -} - func TestGetOrDefaultApplicationDefaults(t *testing.T) { tests := []struct { key string @@ -116,40 +53,79 @@ func TestGetOrDefaultApplicationDefaults(t *testing.T) { cfg := newTestConfig() // When we get a key that has no value, but has a default - val, err := cfg.GetOrDefault("", tt.key) + optionalEntry := cfg.GetOrDefault("", tt.key) - // Then it returns the default value - require.NoError(t, err) - require.Equal(t, tt.expectedDefault, val) + // Then there is an entry with the default value, and source set as default + entry := optionalEntry.Expect(fmt.Sprintf("expected there to be a value for %s", tt.key)) + require.Equal(t, tt.expectedDefault, entry.Value) + require.Equal(t, gh.ConfigDefaultProvided, entry.Source) }) } } -func TestGetOrDefaultExistingKey(t *testing.T) { - // Given have a top level config entry +func TestGetOrDefaultNonExistentKey(t *testing.T) { + // Given we have no top level configuration cfg := newTestConfig() - cfg.Set("", gitProtocolKey, "ssh") - // When we get that key - val, err := cfg.GetOrDefault("", gitProtocolKey) + // When we get a key that has no value + optionalEntry := cfg.GetOrDefault("", "non-existent-key") - // Then it returns successfully with the correct value, and doesn't fall back - // to the default - require.NoError(t, err) - require.Equal(t, "ssh", val) + // Then it returns a None variant + require.True(t, optionalEntry.IsNone(), "expected there to be no value") } -func TestGetOrDefaultNotFoundAndNoDefault(t *testing.T) { - // Given have no configuration +func TestGetOrDefaultNonExistentHostSpecificKey(t *testing.T) { + // Given have no top level configuration cfg := newTestConfig() - // When we get a non-existent-key that has no default - val, err := cfg.GetOrDefault("", "non-existent-key") + // When we get a key for a host that has no value + optionalEntry := cfg.GetOrDefault("non-existent-host", "non-existent-key") - // Then it returns an error and the value is empty - var keyNotFoundError *ghConfig.KeyNotFoundError - require.ErrorAs(t, err, &keyNotFoundError) - require.Empty(t, val) + // Then it returns a None variant + require.True(t, optionalEntry.IsNone(), "expected there to be no value") +} + +func TestGetOrDefaultExistingTopLevelKey(t *testing.T) { + // Given have a top level config entry + cfg := newTestConfig() + cfg.Set("", "top-level-key", "top-level-value") + + // When we get that key + optionalEntry := cfg.GetOrDefault("non-existent-host", "top-level-key") + + // Then it returns a Some variant containing the correct value and a source of user + entry := optionalEntry.Expect("expected there to be a value") + require.Equal(t, "top-level-value", entry.Value) + require.Equal(t, gh.ConfigUserProvided, entry.Source) +} + +func TestGetOrDefaultExistingHostSpecificKey(t *testing.T) { + // Given have a host specific config entry + cfg := newTestConfig() + cfg.Set("github.com", "host-specific-key", "host-specific-value") + + // When we get that key + optionalEntry := cfg.GetOrDefault("github.com", "host-specific-key") + + // Then it returns a Some variant containing the correct value and a source of user + entry := optionalEntry.Expect("expected there to be a value") + require.Equal(t, "host-specific-value", entry.Value) + require.Equal(t, gh.ConfigUserProvided, entry.Source) +} + +func TestGetOrDefaultHostnameSpecificKeyFallsBackToTopLevel(t *testing.T) { + // Given have a top level config entry + cfg := newTestConfig() + cfg.Set("", "key", "value") + + // When we get that key on a specific host + optionalEntry := cfg.GetOrDefault("github.com", "key") + + // Then it returns a Some variant containing the correct value by falling back + // to the top level config, with a source of user + entry := optionalEntry.Expect("expected there to be a value") + require.Equal(t, "value", entry.Value) + require.Equal(t, gh.ConfigUserProvided, entry.Source) } func TestFallbackConfig(t *testing.T) { diff --git a/internal/config/stub.go b/internal/config/stub.go index d0500cfe4..c89868721 100644 --- a/internal/config/stub.go +++ b/internal/config/stub.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/v2/internal/gh" ghmock "github.com/cli/cli/v2/internal/gh/mock" "github.com/cli/cli/v2/internal/keyring" + o "github.com/cli/cli/v2/pkg/option" ghConfig "github.com/cli/go-gh/v2/pkg/config" ) @@ -20,7 +21,7 @@ func NewFromString(cfgStr string) *ghmock.ConfigMock { c := ghConfig.ReadFromString(cfgStr) cfg := cfg{c} mock := &ghmock.ConfigMock{} - mock.GetOrDefaultFunc = func(host, key string) (string, error) { + mock.GetOrDefaultFunc = func(host, key string) o.Option[gh.ConfigEntry] { return cfg.GetOrDefault(host, key) } mock.SetFunc = func(host, key, value string) { @@ -51,33 +52,26 @@ func NewFromString(cfgStr string) *ghmock.ConfigMock { }, } } - mock.BrowserFunc = func(hostname string) string { - val, _ := cfg.GetOrDefault(hostname, browserKey) - return val + mock.BrowserFunc = func(hostname string) gh.ConfigEntry { + return cfg.Browser(hostname) } - mock.EditorFunc = func(hostname string) string { - val, _ := cfg.GetOrDefault(hostname, editorKey) - return val + mock.EditorFunc = func(hostname string) gh.ConfigEntry { + return cfg.Editor(hostname) } - mock.GitProtocolFunc = func(hostname string) string { - val, _ := cfg.GetOrDefault(hostname, gitProtocolKey) - return val + mock.GitProtocolFunc = func(hostname string) gh.ConfigEntry { + return cfg.GitProtocol(hostname) } - mock.HTTPUnixSocketFunc = func(hostname string) string { - val, _ := cfg.GetOrDefault(hostname, httpUnixSocketKey) - return val + mock.HTTPUnixSocketFunc = func(hostname string) gh.ConfigEntry { + return cfg.HTTPUnixSocket(hostname) } - mock.PagerFunc = func(hostname string) string { - val, _ := cfg.GetOrDefault(hostname, pagerKey) - return val + mock.PagerFunc = func(hostname string) gh.ConfigEntry { + return cfg.Pager(hostname) } - mock.PromptFunc = func(hostname string) string { - val, _ := cfg.GetOrDefault(hostname, promptKey) - return val + mock.PromptFunc = func(hostname string) gh.ConfigEntry { + return cfg.Prompt(hostname) } - mock.VersionFunc = func() string { - val, _ := cfg.GetOrDefault("", versionKey) - return val + mock.VersionFunc = func() o.Option[string] { + return cfg.Version() } mock.CacheDirFunc = func() string { return cfg.CacheDir() diff --git a/internal/gh/gh.go b/internal/gh/gh.go index 6e3094ea1..a6db43d66 100644 --- a/internal/gh/gh.go +++ b/internal/gh/gh.go @@ -10,30 +10,43 @@ package gh import ( + o "github.com/cli/cli/v2/pkg/option" ghConfig "github.com/cli/go-gh/v2/pkg/config" ) +type ConfigSource string + +const ( + ConfigDefaultProvided ConfigSource = "default" + ConfigUserProvided ConfigSource = "user" +) + +type ConfigEntry struct { + Value string + Source ConfigSource +} + // A Config implements persistent storage and modification of application configuration. // //go:generate moq -rm -pkg ghmock -out mock/config.go . Config type Config interface { // GetOrDefault provides primitive access for fetching configuration values, optionally scoped by host. - GetOrDefault(hostname string, key string) (string, error) + GetOrDefault(hostname string, key string) o.Option[ConfigEntry] // Set provides primitive access for setting configuration values, optionally scoped by host. Set(hostname string, key string, value string) // Browser returns the configured browser, optionally scoped by host. - Browser(hostname string) string + Browser(hostname string) ConfigEntry // Editor returns the configured editor, optionally scoped by host. - Editor(hostname string) string + Editor(hostname string) ConfigEntry // GitProtocol returns the configured git protocol, optionally scoped by host. - GitProtocol(hostname string) string + GitProtocol(hostname string) ConfigEntry // HTTPUnixSocket returns the configured HTTP unix socket, optionally scoped by host. - HTTPUnixSocket(hostname string) string + HTTPUnixSocket(hostname string) ConfigEntry // Pager returns the configured Pager, optionally scoped by host. - Pager(hostname string) string + Pager(hostname string) ConfigEntry // Prompt returns the configured prompt, optionally scoped by host. - Prompt(hostname string) string + Prompt(hostname string) ConfigEntry // Aliases provides persistent storage and modification of command aliases. Aliases() AliasConfig @@ -48,7 +61,7 @@ type Config interface { Migrate(Migration) error // Version returns the current schema version of the configuration. - Version() string + Version() o.Option[string] // Write persists modifications to the configuration. Write() error diff --git a/internal/gh/mock/config.go b/internal/gh/mock/config.go index 736c8c262..502047240 100644 --- a/internal/gh/mock/config.go +++ b/internal/gh/mock/config.go @@ -5,6 +5,7 @@ package ghmock import ( "github.com/cli/cli/v2/internal/gh" + o "github.com/cli/cli/v2/pkg/option" "sync" ) @@ -24,37 +25,37 @@ var _ gh.Config = &ConfigMock{} // AuthenticationFunc: func() gh.AuthConfig { // panic("mock out the Authentication method") // }, -// BrowserFunc: func(s string) string { +// BrowserFunc: func(hostname string) gh.ConfigEntry { // panic("mock out the Browser method") // }, // CacheDirFunc: func() string { // panic("mock out the CacheDir method") // }, -// EditorFunc: func(s string) string { +// EditorFunc: func(hostname string) gh.ConfigEntry { // panic("mock out the Editor method") // }, -// GetOrDefaultFunc: func(s1 string, s2 string) (string, error) { +// GetOrDefaultFunc: func(hostname string, key string) o.Option[gh.ConfigEntry] { // panic("mock out the GetOrDefault method") // }, -// GitProtocolFunc: func(s string) string { +// GitProtocolFunc: func(hostname string) gh.ConfigEntry { // panic("mock out the GitProtocol method") // }, -// HTTPUnixSocketFunc: func(s string) string { +// HTTPUnixSocketFunc: func(hostname string) gh.ConfigEntry { // panic("mock out the HTTPUnixSocket method") // }, // MigrateFunc: func(migration gh.Migration) error { // panic("mock out the Migrate method") // }, -// PagerFunc: func(s string) string { +// PagerFunc: func(hostname string) gh.ConfigEntry { // panic("mock out the Pager method") // }, -// PromptFunc: func(s string) string { +// PromptFunc: func(hostname string) gh.ConfigEntry { // panic("mock out the Prompt method") // }, -// SetFunc: func(s1 string, s2 string, s3 string) { +// SetFunc: func(hostname string, key string, value string) { // panic("mock out the Set method") // }, -// VersionFunc: func() string { +// VersionFunc: func() o.Option[string] { // panic("mock out the Version method") // }, // WriteFunc: func() error { @@ -74,37 +75,37 @@ type ConfigMock struct { AuthenticationFunc func() gh.AuthConfig // BrowserFunc mocks the Browser method. - BrowserFunc func(s string) string + BrowserFunc func(hostname string) gh.ConfigEntry // CacheDirFunc mocks the CacheDir method. CacheDirFunc func() string // EditorFunc mocks the Editor method. - EditorFunc func(s string) string + EditorFunc func(hostname string) gh.ConfigEntry // GetOrDefaultFunc mocks the GetOrDefault method. - GetOrDefaultFunc func(s1 string, s2 string) (string, error) + GetOrDefaultFunc func(hostname string, key string) o.Option[gh.ConfigEntry] // GitProtocolFunc mocks the GitProtocol method. - GitProtocolFunc func(s string) string + GitProtocolFunc func(hostname string) gh.ConfigEntry // HTTPUnixSocketFunc mocks the HTTPUnixSocket method. - HTTPUnixSocketFunc func(s string) string + HTTPUnixSocketFunc func(hostname string) gh.ConfigEntry // MigrateFunc mocks the Migrate method. MigrateFunc func(migration gh.Migration) error // PagerFunc mocks the Pager method. - PagerFunc func(s string) string + PagerFunc func(hostname string) gh.ConfigEntry // PromptFunc mocks the Prompt method. - PromptFunc func(s string) string + PromptFunc func(hostname string) gh.ConfigEntry // SetFunc mocks the Set method. - SetFunc func(s1 string, s2 string, s3 string) + SetFunc func(hostname string, key string, value string) // VersionFunc mocks the Version method. - VersionFunc func() string + VersionFunc func() o.Option[string] // WriteFunc mocks the Write method. WriteFunc func() error @@ -119,33 +120,33 @@ type ConfigMock struct { } // Browser holds details about calls to the Browser method. Browser []struct { - // S is the s argument value. - S string + // Hostname is the hostname argument value. + Hostname string } // CacheDir holds details about calls to the CacheDir method. CacheDir []struct { } // Editor holds details about calls to the Editor method. Editor []struct { - // S is the s argument value. - S string + // Hostname is the hostname argument value. + Hostname string } // GetOrDefault holds details about calls to the GetOrDefault method. GetOrDefault []struct { - // S1 is the s1 argument value. - S1 string - // S2 is the s2 argument value. - S2 string + // Hostname is the hostname argument value. + Hostname string + // Key is the key argument value. + Key string } // GitProtocol holds details about calls to the GitProtocol method. GitProtocol []struct { - // S is the s argument value. - S string + // Hostname is the hostname argument value. + Hostname string } // HTTPUnixSocket holds details about calls to the HTTPUnixSocket method. HTTPUnixSocket []struct { - // S is the s argument value. - S string + // Hostname is the hostname argument value. + Hostname string } // Migrate holds details about calls to the Migrate method. Migrate []struct { @@ -154,22 +155,22 @@ type ConfigMock struct { } // Pager holds details about calls to the Pager method. Pager []struct { - // S is the s argument value. - S string + // Hostname is the hostname argument value. + Hostname string } // Prompt holds details about calls to the Prompt method. Prompt []struct { - // S is the s argument value. - S string + // Hostname is the hostname argument value. + Hostname string } // Set holds details about calls to the Set method. Set []struct { - // S1 is the s1 argument value. - S1 string - // S2 is the s2 argument value. - S2 string - // S3 is the s3 argument value. - S3 string + // Hostname is the hostname argument value. + Hostname string + // Key is the key argument value. + Key string + // Value is the value argument value. + Value string } // Version holds details about calls to the Version method. Version []struct { @@ -249,19 +250,19 @@ func (mock *ConfigMock) AuthenticationCalls() []struct { } // Browser calls BrowserFunc. -func (mock *ConfigMock) Browser(s string) string { +func (mock *ConfigMock) Browser(hostname string) gh.ConfigEntry { if mock.BrowserFunc == nil { panic("ConfigMock.BrowserFunc: method is nil but Config.Browser was just called") } callInfo := struct { - S string + Hostname string }{ - S: s, + Hostname: hostname, } mock.lockBrowser.Lock() mock.calls.Browser = append(mock.calls.Browser, callInfo) mock.lockBrowser.Unlock() - return mock.BrowserFunc(s) + return mock.BrowserFunc(hostname) } // BrowserCalls gets all the calls that were made to Browser. @@ -269,10 +270,10 @@ func (mock *ConfigMock) Browser(s string) string { // // len(mockedConfig.BrowserCalls()) func (mock *ConfigMock) BrowserCalls() []struct { - S string + Hostname string } { var calls []struct { - S string + Hostname string } mock.lockBrowser.RLock() calls = mock.calls.Browser @@ -308,19 +309,19 @@ func (mock *ConfigMock) CacheDirCalls() []struct { } // Editor calls EditorFunc. -func (mock *ConfigMock) Editor(s string) string { +func (mock *ConfigMock) Editor(hostname string) gh.ConfigEntry { if mock.EditorFunc == nil { panic("ConfigMock.EditorFunc: method is nil but Config.Editor was just called") } callInfo := struct { - S string + Hostname string }{ - S: s, + Hostname: hostname, } mock.lockEditor.Lock() mock.calls.Editor = append(mock.calls.Editor, callInfo) mock.lockEditor.Unlock() - return mock.EditorFunc(s) + return mock.EditorFunc(hostname) } // EditorCalls gets all the calls that were made to Editor. @@ -328,10 +329,10 @@ func (mock *ConfigMock) Editor(s string) string { // // len(mockedConfig.EditorCalls()) func (mock *ConfigMock) EditorCalls() []struct { - S string + Hostname string } { var calls []struct { - S string + Hostname string } mock.lockEditor.RLock() calls = mock.calls.Editor @@ -340,21 +341,21 @@ func (mock *ConfigMock) EditorCalls() []struct { } // GetOrDefault calls GetOrDefaultFunc. -func (mock *ConfigMock) GetOrDefault(s1 string, s2 string) (string, error) { +func (mock *ConfigMock) GetOrDefault(hostname string, key string) o.Option[gh.ConfigEntry] { if mock.GetOrDefaultFunc == nil { panic("ConfigMock.GetOrDefaultFunc: method is nil but Config.GetOrDefault was just called") } callInfo := struct { - S1 string - S2 string + Hostname string + Key string }{ - S1: s1, - S2: s2, + Hostname: hostname, + Key: key, } mock.lockGetOrDefault.Lock() mock.calls.GetOrDefault = append(mock.calls.GetOrDefault, callInfo) mock.lockGetOrDefault.Unlock() - return mock.GetOrDefaultFunc(s1, s2) + return mock.GetOrDefaultFunc(hostname, key) } // GetOrDefaultCalls gets all the calls that were made to GetOrDefault. @@ -362,12 +363,12 @@ func (mock *ConfigMock) GetOrDefault(s1 string, s2 string) (string, error) { // // len(mockedConfig.GetOrDefaultCalls()) func (mock *ConfigMock) GetOrDefaultCalls() []struct { - S1 string - S2 string + Hostname string + Key string } { var calls []struct { - S1 string - S2 string + Hostname string + Key string } mock.lockGetOrDefault.RLock() calls = mock.calls.GetOrDefault @@ -376,19 +377,19 @@ func (mock *ConfigMock) GetOrDefaultCalls() []struct { } // GitProtocol calls GitProtocolFunc. -func (mock *ConfigMock) GitProtocol(s string) string { +func (mock *ConfigMock) GitProtocol(hostname string) gh.ConfigEntry { if mock.GitProtocolFunc == nil { panic("ConfigMock.GitProtocolFunc: method is nil but Config.GitProtocol was just called") } callInfo := struct { - S string + Hostname string }{ - S: s, + Hostname: hostname, } mock.lockGitProtocol.Lock() mock.calls.GitProtocol = append(mock.calls.GitProtocol, callInfo) mock.lockGitProtocol.Unlock() - return mock.GitProtocolFunc(s) + return mock.GitProtocolFunc(hostname) } // GitProtocolCalls gets all the calls that were made to GitProtocol. @@ -396,10 +397,10 @@ func (mock *ConfigMock) GitProtocol(s string) string { // // len(mockedConfig.GitProtocolCalls()) func (mock *ConfigMock) GitProtocolCalls() []struct { - S string + Hostname string } { var calls []struct { - S string + Hostname string } mock.lockGitProtocol.RLock() calls = mock.calls.GitProtocol @@ -408,19 +409,19 @@ func (mock *ConfigMock) GitProtocolCalls() []struct { } // HTTPUnixSocket calls HTTPUnixSocketFunc. -func (mock *ConfigMock) HTTPUnixSocket(s string) string { +func (mock *ConfigMock) HTTPUnixSocket(hostname string) gh.ConfigEntry { if mock.HTTPUnixSocketFunc == nil { panic("ConfigMock.HTTPUnixSocketFunc: method is nil but Config.HTTPUnixSocket was just called") } callInfo := struct { - S string + Hostname string }{ - S: s, + Hostname: hostname, } mock.lockHTTPUnixSocket.Lock() mock.calls.HTTPUnixSocket = append(mock.calls.HTTPUnixSocket, callInfo) mock.lockHTTPUnixSocket.Unlock() - return mock.HTTPUnixSocketFunc(s) + return mock.HTTPUnixSocketFunc(hostname) } // HTTPUnixSocketCalls gets all the calls that were made to HTTPUnixSocket. @@ -428,10 +429,10 @@ func (mock *ConfigMock) HTTPUnixSocket(s string) string { // // len(mockedConfig.HTTPUnixSocketCalls()) func (mock *ConfigMock) HTTPUnixSocketCalls() []struct { - S string + Hostname string } { var calls []struct { - S string + Hostname string } mock.lockHTTPUnixSocket.RLock() calls = mock.calls.HTTPUnixSocket @@ -472,19 +473,19 @@ func (mock *ConfigMock) MigrateCalls() []struct { } // Pager calls PagerFunc. -func (mock *ConfigMock) Pager(s string) string { +func (mock *ConfigMock) Pager(hostname string) gh.ConfigEntry { if mock.PagerFunc == nil { panic("ConfigMock.PagerFunc: method is nil but Config.Pager was just called") } callInfo := struct { - S string + Hostname string }{ - S: s, + Hostname: hostname, } mock.lockPager.Lock() mock.calls.Pager = append(mock.calls.Pager, callInfo) mock.lockPager.Unlock() - return mock.PagerFunc(s) + return mock.PagerFunc(hostname) } // PagerCalls gets all the calls that were made to Pager. @@ -492,10 +493,10 @@ func (mock *ConfigMock) Pager(s string) string { // // len(mockedConfig.PagerCalls()) func (mock *ConfigMock) PagerCalls() []struct { - S string + Hostname string } { var calls []struct { - S string + Hostname string } mock.lockPager.RLock() calls = mock.calls.Pager @@ -504,19 +505,19 @@ func (mock *ConfigMock) PagerCalls() []struct { } // Prompt calls PromptFunc. -func (mock *ConfigMock) Prompt(s string) string { +func (mock *ConfigMock) Prompt(hostname string) gh.ConfigEntry { if mock.PromptFunc == nil { panic("ConfigMock.PromptFunc: method is nil but Config.Prompt was just called") } callInfo := struct { - S string + Hostname string }{ - S: s, + Hostname: hostname, } mock.lockPrompt.Lock() mock.calls.Prompt = append(mock.calls.Prompt, callInfo) mock.lockPrompt.Unlock() - return mock.PromptFunc(s) + return mock.PromptFunc(hostname) } // PromptCalls gets all the calls that were made to Prompt. @@ -524,10 +525,10 @@ func (mock *ConfigMock) Prompt(s string) string { // // len(mockedConfig.PromptCalls()) func (mock *ConfigMock) PromptCalls() []struct { - S string + Hostname string } { var calls []struct { - S string + Hostname string } mock.lockPrompt.RLock() calls = mock.calls.Prompt @@ -536,23 +537,23 @@ func (mock *ConfigMock) PromptCalls() []struct { } // Set calls SetFunc. -func (mock *ConfigMock) Set(s1 string, s2 string, s3 string) { +func (mock *ConfigMock) Set(hostname string, key string, value string) { if mock.SetFunc == nil { panic("ConfigMock.SetFunc: method is nil but Config.Set was just called") } callInfo := struct { - S1 string - S2 string - S3 string + Hostname string + Key string + Value string }{ - S1: s1, - S2: s2, - S3: s3, + Hostname: hostname, + Key: key, + Value: value, } mock.lockSet.Lock() mock.calls.Set = append(mock.calls.Set, callInfo) mock.lockSet.Unlock() - mock.SetFunc(s1, s2, s3) + mock.SetFunc(hostname, key, value) } // SetCalls gets all the calls that were made to Set. @@ -560,14 +561,14 @@ func (mock *ConfigMock) Set(s1 string, s2 string, s3 string) { // // len(mockedConfig.SetCalls()) func (mock *ConfigMock) SetCalls() []struct { - S1 string - S2 string - S3 string + Hostname string + Key string + Value string } { var calls []struct { - S1 string - S2 string - S3 string + Hostname string + Key string + Value string } mock.lockSet.RLock() calls = mock.calls.Set @@ -576,7 +577,7 @@ func (mock *ConfigMock) SetCalls() []struct { } // Version calls VersionFunc. -func (mock *ConfigMock) Version() string { +func (mock *ConfigMock) Version() o.Option[string] { if mock.VersionFunc == nil { panic("ConfigMock.VersionFunc: method is nil but Config.Version was just called") } diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index 4e675c532..262b8fb31 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -177,7 +177,7 @@ func refreshRun(opts *RefreshOptions) error { Prompter: opts.Prompter, GitClient: opts.GitClient, } - gitProtocol := cfg.GitProtocol(hostname) + gitProtocol := cfg.GitProtocol(hostname).Value if opts.Interactive && gitProtocol == "https" { if err := credentialFlow.Prompt(hostname); err != nil { return err diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index deb5ca6a1..153c03c04 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -201,7 +201,7 @@ func statusRun(opts *StatusOptions) error { } var activeUser string - gitProtocol := cfg.GitProtocol(hostname) + gitProtocol := cfg.GitProtocol(hostname).Value activeUserToken, activeUserTokenSource := authCfg.ActiveToken(hostname) if authTokenWriteable(activeUserTokenSource) { activeUser, _ = authCfg.ActiveUser(hostname) diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go index 20a660279..2661c3369 100644 --- a/pkg/cmd/config/config.go +++ b/pkg/cmd/config/config.go @@ -17,7 +17,7 @@ func NewCmdConfig(f *cmdutil.Factory) *cobra.Command { 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() { + for _, co := range config.Options { longDoc.WriteString(fmt.Sprintf("- `%s`: %s", co.Key, co.Description)) if len(co.AllowedValues) > 0 { longDoc.WriteString(fmt.Sprintf(" {%s}", strings.Join(co.AllowedValues, "|"))) diff --git a/pkg/cmd/config/get/get.go b/pkg/cmd/config/get/get.go index e714c6a64..bec22a979 100644 --- a/pkg/cmd/config/get/get.go +++ b/pkg/cmd/config/get/get.go @@ -64,13 +64,22 @@ func getRun(opts *GetOptions) error { return nil } - val, err := opts.Config.GetOrDefault(opts.Hostname, opts.Key) - if err != nil { - return err + optionalEntry := opts.Config.GetOrDefault(opts.Hostname, opts.Key) + if optionalEntry.IsNone() { + return nonExistentKeyError{key: opts.Key} } + val := optionalEntry.Unwrap().Value if val != "" { fmt.Fprintf(opts.IO.Out, "%s\n", val) } return nil } + +type nonExistentKeyError struct { + key string +} + +func (e nonExistentKeyError) Error() string { + return fmt.Sprintf("could not find key \"%s\"", e.key) +} diff --git a/pkg/cmd/config/get/get_test.go b/pkg/cmd/config/get/get_test.go index 40eca49e5..6320ffa16 100644 --- a/pkg/cmd/config/get/get_test.go +++ b/pkg/cmd/config/get/get_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewCmdConfigGet(t *testing.T) { @@ -77,11 +78,10 @@ func TestNewCmdConfigGet(t *testing.T) { func Test_getRun(t *testing.T) { tests := []struct { - name string - input *GetOptions - stdout string - stderr string - wantErr bool + name string + input *GetOptions + stdout string + err error }{ { name: "get key", @@ -109,17 +109,24 @@ func Test_getRun(t *testing.T) { }, stdout: "vim\n", }, + { + name: "non-existent key", + input: &GetOptions{ + Key: "non-existent", + Config: config.NewBlankConfig(), + }, + err: nonExistentKeyError{key: "non-existent"}, + }, } for _, tt := range tests { - ios, _, stdout, stderr := iostreams.Test() + ios, _, stdout, _ := iostreams.Test() tt.input.IO = ios 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()) + require.Equal(t, err, tt.err) + require.Equal(t, tt.stdout, stdout.String()) }) } } diff --git a/pkg/cmd/config/list/list.go b/pkg/cmd/config/list/list.go index da2707134..1cfb92c87 100644 --- a/pkg/cmd/config/list/list.go +++ b/pkg/cmd/config/list/list.go @@ -55,14 +55,8 @@ func listRun(opts *ListOptions) error { host, _ = cfg.Authentication().DefaultHost() } - configOptions := config.ConfigOptions() - - for _, key := range configOptions { - val, err := cfg.GetOrDefault(host, key.Key) - if err != nil { - return err - } - fmt.Fprintf(opts.IO.Out, "%s=%s\n", key.Key, val) + for _, option := range config.Options { + fmt.Fprintf(opts.IO.Out, "%s=%s\n", option.Key, option.CurrentValue(cfg, host)) } return nil diff --git a/pkg/cmd/config/list/list_test.go b/pkg/cmd/config/list/list_test.go index 32088b324..b6a9d0fca 100644 --- a/pkg/cmd/config/list/list_test.go +++ b/pkg/cmd/config/list/list_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewCmdConfigList(t *testing.T) { @@ -108,9 +109,8 @@ browser=brave t.Run(tt.name, func(t *testing.T) { err := listRun(tt.input) - assert.NoError(t, err) - assert.Equal(t, tt.stdout, stdout.String()) - //assert.Equal(t, tt.stderr, stderr.String()) + require.NoError(t, err) + require.Equal(t, tt.stdout, stdout.String()) }) } } diff --git a/pkg/cmd/config/set/set.go b/pkg/cmd/config/set/set.go index dedbd0044..a1fb2bbe9 100644 --- a/pkg/cmd/config/set/set.go +++ b/pkg/cmd/config/set/set.go @@ -88,7 +88,7 @@ func setRun(opts *SetOptions) error { } func ValidateKey(key string) error { - for _, configKey := range config.ConfigOptions() { + for _, configKey := range config.Options { if key == configKey.Key { return nil } @@ -108,7 +108,7 @@ func (e InvalidValueError) Error() string { func ValidateValue(key, value string) error { var validValues []string - for _, v := range config.ConfigOptions() { + for _, v := range config.Options { if v.Key == key { validValues = v.AllowedValues break diff --git a/pkg/cmd/config/set/set_test.go b/pkg/cmd/config/set/set_test.go index 532d78f62..adfb7ba74 100644 --- a/pkg/cmd/config/set/set_test.go +++ b/pkg/cmd/config/set/set_test.go @@ -150,9 +150,10 @@ func Test_setRun(t *testing.T) { assert.Equal(t, tt.stdout, stdout.String()) assert.Equal(t, tt.stderr, stderr.String()) - val, err := tt.input.Config.GetOrDefault(tt.input.Hostname, tt.input.Key) - assert.NoError(t, err) - assert.Equal(t, tt.expectedValue, val) + optionalEntry := tt.input.Config.GetOrDefault(tt.input.Hostname, tt.input.Key) + entry := optionalEntry.Expect("expected a value to be set") + assert.Equal(t, tt.expectedValue, entry.Value) + assert.Equal(t, gh.ConfigUserProvided, entry.Source) }) } } diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 37d80573a..0ab429c2e 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -347,7 +347,7 @@ func writeManifest(dir, name string, data []byte) (writeErr error) { } func (m *Manager) installGit(repo ghrepo.Interface, target string) error { - protocol := m.config.GitProtocol(repo.RepoHost()) + protocol := m.config.GitProtocol(repo.RepoHost()).Value cloneURL := ghrepo.FormatRemoteURL(repo, protocol) var commitSHA string diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index cb38c10fe..7abf3ca5f 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -185,7 +185,7 @@ func ioStreams(f *cmdutil.Factory) *iostreams.IOStreams { if _, ghPromptDisabled := os.LookupEnv("GH_PROMPT_DISABLED"); ghPromptDisabled { io.SetNeverPrompt(true) - } else if prompt := cfg.Prompt(""); prompt == "disabled" { + } else if prompt := cfg.Prompt(""); prompt.Value == "disabled" { io.SetNeverPrompt(true) } @@ -195,8 +195,8 @@ func ioStreams(f *cmdutil.Factory) *iostreams.IOStreams { // 3. PAGER if ghPager, ghPagerExists := os.LookupEnv("GH_PAGER"); ghPagerExists { io.SetPager(ghPager) - } else if pager := cfg.Pager(""); pager != "" { - io.SetPager(pager) + } else if pager := cfg.Pager(""); pager.Value != "" { + io.SetPager(pager.Value) } return io diff --git a/pkg/cmd/gist/clone/clone.go b/pkg/cmd/gist/clone/clone.go index 0f50bce31..6fa6f226a 100644 --- a/pkg/cmd/gist/clone/clone.go +++ b/pkg/cmd/gist/clone/clone.go @@ -80,7 +80,7 @@ func cloneRun(opts *CloneOptions) error { return err } hostname, _ := cfg.Authentication().DefaultHost() - protocol := cfg.GitProtocol(hostname) + protocol := cfg.GitProtocol(hostname).Value gistURL = formatRemoteURL(hostname, gistURL, protocol) } diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index f47f579f2..f566cbe1d 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -84,7 +84,7 @@ func checkoutRun(opts *CheckoutOptions) error { if err != nil { return err } - protocol := cfg.GitProtocol(baseRepo.RepoHost()) + protocol := cfg.GitProtocol(baseRepo.RepoHost()).Value remotes, err := opts.Remotes() if err != nil { diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 54b6358ac..7c3faecc3 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -870,7 +870,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { return err } - cloneProtocol := cfg.GitProtocol(headRepo.RepoHost()) + cloneProtocol := cfg.GitProtocol(headRepo.RepoHost()).Value headRepoURL := ghrepo.FormatRemoteURL(headRepo, cloneProtocol) gitClient := ctx.GitClient origin, _ := remotes.FindByName("origin") diff --git a/pkg/cmd/repo/clone/clone.go b/pkg/cmd/repo/clone/clone.go index 28001e6fe..1466cd96a 100644 --- a/pkg/cmd/repo/clone/clone.go +++ b/pkg/cmd/repo/clone/clone.go @@ -153,7 +153,7 @@ func cloneRun(opts *CloneOptions) error { return err } - protocol = cfg.GitProtocol(repo.RepoHost()) + protocol = cfg.GitProtocol(repo.RepoHost()).Value } wantsWiki := strings.HasSuffix(repo.RepoName(), ".wiki") @@ -187,7 +187,7 @@ func cloneRun(opts *CloneOptions) error { // If the repo is a fork, add the parent as an upstream remote and set the parent as the default repo. if canonicalRepo.Parent != nil { - protocol := cfg.GitProtocol(canonicalRepo.Parent.RepoHost()) + protocol := cfg.GitProtocol(canonicalRepo.Parent.RepoHost()).Value upstreamURL := ghrepo.FormatRemoteURL(canonicalRepo.Parent, protocol) upstreamName := opts.UpstreamName diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index af30f4029..48a331bdd 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -396,7 +396,7 @@ func createFromScratch(opts *CreateOptions) error { } if opts.Clone { - protocol := cfg.GitProtocol(repo.RepoHost()) + protocol := cfg.GitProtocol(repo.RepoHost()).Value remoteURL := ghrepo.FormatRemoteURL(repo, protocol) if !opts.AddReadme && opts.LicenseTemplate == "" && opts.GitIgnoreTemplate == "" && opts.Template == "" { @@ -494,7 +494,7 @@ func createFromTemplate(opts *CreateOptions) error { } if opts.Clone { - protocol := cfg.GitProtocol(repo.RepoHost()) + protocol := cfg.GitProtocol(repo.RepoHost()).Value remoteURL := ghrepo.FormatRemoteURL(repo, protocol) if err := cloneWithRetry(opts, remoteURL, templateRepoMainBranch); err != nil { @@ -617,7 +617,7 @@ func createFromLocal(opts *CreateOptions) error { fmt.Fprintln(stdout, repo.URL) } - protocol := cfg.GitProtocol(repo.RepoHost()) + protocol := cfg.GitProtocol(repo.RepoHost()).Value remoteURL := ghrepo.FormatRemoteURL(repo, protocol) if opts.Interactive { diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index a5e9066b2..a49f5d567 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -243,7 +243,9 @@ func forkRun(opts *ForkOptions) error { if err != nil { return err } - protocol := cfg.GitProtocol(repoToFork.RepoHost()) + protocolConfig := cfg.GitProtocol(repoToFork.RepoHost()) + protocolIsConfiguredByUser := protocolConfig.Source == gh.ConfigUserProvided + protocol := protocolConfig.Value gitClient := opts.GitClient ctx := context.Background() @@ -254,7 +256,7 @@ func forkRun(opts *ForkOptions) error { return err } - if protocol == "" { // user has no set preference + if !protocolIsConfiguredByUser { if remote, err := remotes.FindByRepo(repoToFork.RepoOwner(), repoToFork.RepoName()); err == nil { scheme := "" if remote.FetchURL != nil { diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index b1b62e11a..0f94496f0 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -234,6 +234,9 @@ func TestRepoFork(t *testing.T) { Repo: ghrepo.New("OWNER", "REPO"), }, }, + cfgStubs: func(_ *testing.T, c gh.Config) { + c.Set("", "git_protocol", "https") + }, httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { cs.Register(`git remote add fork https://github\.com/someone/REPO\.git`, 0, "") @@ -255,9 +258,6 @@ func TestRepoFork(t *testing.T) { Repo: ghrepo.New("OWNER", "REPO"), }, }, - cfgStubs: func(_ *testing.T, c gh.Config) { - c.Set("", "git_protocol", "") - }, httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { cs.Register(`git remote add fork git@github\.com:someone/REPO\.git`, 0, "") diff --git a/pkg/cmd/repo/rename/rename.go b/pkg/cmd/repo/rename/rename.go index 7a011b4e3..3676f93fe 100644 --- a/pkg/cmd/repo/rename/rename.go +++ b/pkg/cmd/repo/rename/rename.go @@ -153,7 +153,7 @@ func updateRemote(repo ghrepo.Interface, renamed ghrepo.Interface, opts *RenameO return nil, err } - protocol := cfg.GitProtocol(repo.RepoHost()) + protocol := cfg.GitProtocol(repo.RepoHost()).Value remotes, err := opts.Remotes() if err != nil { diff --git a/pkg/cmdutil/legacy.go b/pkg/cmdutil/legacy.go index 0cc9674e1..04e72aedb 100644 --- a/pkg/cmdutil/legacy.go +++ b/pkg/cmdutil/legacy.go @@ -16,7 +16,7 @@ func DetermineEditor(cf func() (gh.Config, error)) (string, error) { if err != nil { return "", fmt.Errorf("could not read config: %w", err) } - editorCommand = cfg.Editor("") + editorCommand = cfg.Editor("").Value } return editorCommand, nil diff --git a/pkg/option/option.go b/pkg/option/option.go new file mode 100644 index 000000000..8d3b70f3f --- /dev/null +++ b/pkg/option/option.go @@ -0,0 +1,137 @@ +// MIT License + +// Copyright (c) 2022 Tom Godkin + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +// o provides an Option type to represent values that may or may not be present. +// +// This code was copies from https://github.com/BooleanCat/go-functional@ae5a155c0e997d1c5de53ea8b49109aca9c53d9f +// and we've added the Map function and associated tests. It was pulled into the project because I believe if we're +// using Option, it should be a core domain type rather than a dependency. +package o + +import "fmt" + +// Option represents an optional value. The [Some] variant contains a value and +// the [None] variant represents the absence of a value. +type Option[T any] struct { + value T + present bool +} + +// Some instantiates an [Option] with a value. +func Some[T any](value T) Option[T] { + return Option[T]{value, true} +} + +// None instantiates an [Option] with no value. +func None[T any]() Option[T] { + return Option[T]{} +} + +// String implements the [fmt.Stringer] interface. +func (o Option[T]) String() string { + if o.present { + return fmt.Sprintf("Some(%v)", o.value) + } + + return "None" +} + +var _ fmt.Stringer = Option[struct{}]{} + +// Unwrap returns the underlying value of a [Some] variant, or panics if called +// on a [None] variant. +func (o Option[T]) Unwrap() T { + if o.present { + return o.value + } + + panic("called `Option.Unwrap()` on a `None` value") +} + +// UnwrapOr returns the underlying value of a [Some] variant, or the provided +// value on a [None] variant. +func (o Option[T]) UnwrapOr(value T) T { + if o.present { + return o.value + } + + return value +} + +// UnwrapOrElse returns the underlying value of a [Some] variant, or the result +// of calling the provided function on a [None] variant. +func (o Option[T]) UnwrapOrElse(f func() T) T { + if o.present { + return o.value + } + + return f() +} + +// UnwrapOrZero returns the underlying value of a [Some] variant, or the zero +// value on a [None] variant. +func (o Option[T]) UnwrapOrZero() T { + if o.present { + return o.value + } + + var value T + return value +} + +// IsSome returns true if the [Option] is a [Some] variant. +func (o Option[T]) IsSome() bool { + return o.present +} + +// IsNone returns true if the [Option] is a [None] variant. +func (o Option[T]) IsNone() bool { + return !o.present +} + +// Value returns the underlying value and true for a [Some] variant, or the +// zero value and false for a [None] variant. +func (o Option[T]) Value() (T, bool) { + return o.value, o.present +} + +// Expect returns the underlying value for a [Some] variant, or panics with the +// provided message for a [None] variant. +func (o Option[T]) Expect(message string) T { + if o.present { + return o.value + } + + panic(message) +} + +// Map applies a function to the contained value of (if [Some]), or returns [None]. +// +// Use this function very sparingly as it can lead to very unidiomatic and surprising Go code. However, +// there are times when used judiciiously, it is significantly more ergonomic than unwrapping the Option. +func Map[T, U any](o Option[T], f func(T) U) Option[U] { + if o.present { + return Some(f(o.value)) + } + + return None[U]() +} diff --git a/pkg/option/option_test.go b/pkg/option/option_test.go new file mode 100644 index 000000000..0f28dd5d4 --- /dev/null +++ b/pkg/option/option_test.go @@ -0,0 +1,199 @@ +// MIT License + +// Copyright (c) 2022 Tom Godkin + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. +package o_test + +import ( + "fmt" + "testing" + + o "github.com/cli/cli/v2/pkg/option" + "github.com/stretchr/testify/require" +) + +func ExampleOption_Unwrap() { + fmt.Println(o.Some(4).Unwrap()) + // Output: 4 +} + +func ExampleOption_UnwrapOr() { + fmt.Println(o.Some(4).UnwrapOr(3)) + fmt.Println(o.None[int]().UnwrapOr(3)) + // Output: + // 4 + // 3 +} + +func ExampleOption_UnwrapOrElse() { + fmt.Println(o.Some(4).UnwrapOrElse(func() int { + return 3 + })) + + fmt.Println(o.None[int]().UnwrapOrElse(func() int { + return 3 + })) + + // Output: + // 4 + // 3 +} + +func ExampleOption_UnwrapOrZero() { + fmt.Println(o.Some(4).UnwrapOrZero()) + fmt.Println(o.None[int]().UnwrapOrZero()) + + // Output + // 4 + // 0 +} + +func ExampleOption_IsSome() { + fmt.Println(o.Some(4).IsSome()) + fmt.Println(o.None[int]().IsSome()) + + // Output: + // true + // false +} + +func ExampleOption_IsNone() { + fmt.Println(o.Some(4).IsNone()) + fmt.Println(o.None[int]().IsNone()) + + // Output: + // false + // true +} + +func ExampleOption_Value() { + value, ok := o.Some(4).Value() + fmt.Println(value) + fmt.Println(ok) + + // Output: + // 4 + // true +} + +func ExampleOption_Expect() { + fmt.Println(o.Some(4).Expect("oops")) + + // Output: 4 +} + +func ExampleMap() { + fmt.Println(o.Map(o.Some(2), double)) + fmt.Println(o.Map(o.None[int](), double)) + + // Output: + // Some(4) + // None +} + +func double(i int) int { + return i * 2 +} + +func TestSomeStringer(t *testing.T) { + require.Equal(t, fmt.Sprintf("%s", o.Some("foo")), "Some(foo)") //nolint:gosimple + require.Equal(t, fmt.Sprintf("%s", o.Some(42)), "Some(42)") //nolint:gosimple +} + +func TestNoneStringer(t *testing.T) { + require.Equal(t, fmt.Sprintf("%s", o.None[string]()), "None") //nolint:gosimple +} + +func TestSomeUnwrap(t *testing.T) { + require.Equal(t, o.Some(42).Unwrap(), 42) +} + +func TestNoneUnwrap(t *testing.T) { + defer func() { + require.Equal(t, fmt.Sprint(recover()), "called `Option.Unwrap()` on a `None` value") + }() + + o.None[string]().Unwrap() + t.Error("did not panic") +} + +func TestSomeUnwrapOr(t *testing.T) { + require.Equal(t, o.Some(42).UnwrapOr(3), 42) +} + +func TestNoneUnwrapOr(t *testing.T) { + require.Equal(t, o.None[int]().UnwrapOr(3), 3) +} + +func TestSomeUnwrapOrElse(t *testing.T) { + require.Equal(t, o.Some(42).UnwrapOrElse(func() int { return 41 }), 42) +} + +func TestNoneUnwrapOrElse(t *testing.T) { + require.Equal(t, o.None[int]().UnwrapOrElse(func() int { return 41 }), 41) +} + +func TestSomeUnwrapOrZero(t *testing.T) { + require.Equal(t, o.Some(42).UnwrapOrZero(), 42) +} + +func TestNoneUnwrapOrZero(t *testing.T) { + require.Equal(t, o.None[int]().UnwrapOrZero(), 0) +} + +func TestIsSome(t *testing.T) { + require.True(t, o.Some(42).IsSome()) + require.False(t, o.None[int]().IsSome()) +} + +func TestIsNone(t *testing.T) { + require.False(t, o.Some(42).IsNone()) + require.True(t, o.None[int]().IsNone()) +} + +func TestSomeValue(t *testing.T) { + value, ok := o.Some(42).Value() + require.Equal(t, value, 42) + require.True(t, ok) +} + +func TestNoneValue(t *testing.T) { + value, ok := o.None[int]().Value() + require.Equal(t, value, 0) + require.False(t, ok) +} + +func TestSomeExpect(t *testing.T) { + require.Equal(t, o.Some(42).Expect("oops"), 42) +} + +func TestNoneExpect(t *testing.T) { + defer func() { + require.Equal(t, fmt.Sprint(recover()), "oops") + }() + + o.None[int]().Expect("oops") + t.Error("did not panic") +} + +func TestMap(t *testing.T) { + require.Equal(t, o.Map(o.Some(2), double), o.Some(4)) + require.True(t, o.Map(o.None[int](), double).IsNone()) +}