From 64c18b15a4a0c8b7e59587aa09de92d52c698ede Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Sun, 27 Nov 2022 20:51:37 -0500 Subject: [PATCH 1/3] feat: wrap ErrNoConfigEntry to report missing key Resolves #935. --- commands/webui.go | 3 ++- entities/identity/identity_user.go | 4 ++-- repository/config.go | 6 +++--- repository/config_mem.go | 2 +- repository/config_testing.go | 4 ++-- repository/gogit.go | 3 ++- repository/gogit_config.go | 8 ++++---- 7 files changed, 16 insertions(+), 14 deletions(-) diff --git a/commands/webui.go b/commands/webui.go index 5fe66aa7c0ac6e154d1c2443d85d64c4319a5849..a2a016454f9aba469821f1f0a380369aad78f7df 100644 --- a/commands/webui.go +++ b/commands/webui.go @@ -2,6 +2,7 @@ package commands import ( "context" + "errors" "fmt" "io" "log" @@ -162,7 +163,7 @@ func runWebUI(env *execenv.Env, opts webUIOptions) error { env.Out.Println("Press Ctrl+c to quit") configOpen, err := env.Repo.AnyConfig().ReadBool(webUIOpenConfigKey) - if err == repository.ErrNoConfigEntry { + if errors.Is(err, repository.ErrNoConfigEntry) { // default to true configOpen = true } else if err != nil { diff --git a/entities/identity/identity_user.go b/entities/identity/identity_user.go index cd67459e6958a9de53514f8a6cbfb67fd9f400a4..435af060af744ae33f5aa55e8670b30b7b7d51da 100644 --- a/entities/identity/identity_user.go +++ b/entities/identity/identity_user.go @@ -36,7 +36,7 @@ func GetUserIdentity(repo repository.Repo) (*Identity, error) { func GetUserIdentityId(repo repository.Repo) (entity.Id, error) { val, err := repo.LocalConfig().ReadString(identityConfigKey) - if err == repository.ErrNoConfigEntry { + if errors.Is(err, repository.ErrNoConfigEntry) { return entity.UnsetId, ErrNoIdentitySet } if err == repository.ErrMultipleConfigEntry { @@ -58,7 +58,7 @@ func GetUserIdentityId(repo repository.Repo) (entity.Id, error) { // IsUserIdentitySet say if the user has set his identity func IsUserIdentitySet(repo repository.Repo) (bool, error) { _, err := repo.LocalConfig().ReadString(identityConfigKey) - if err == repository.ErrNoConfigEntry { + if errors.Is(err, repository.ErrNoConfigEntry) { return false, nil } if err != nil { diff --git a/repository/config.go b/repository/config.go index c6880b7d0bae5824c8d71a3393e521b5a9dde49a..4d6a8c6c4decbef44649cef074a357a56635e058 100644 --- a/repository/config.go +++ b/repository/config.go @@ -96,7 +96,7 @@ func (m *mergedConfig) ReadBool(key string) (bool, error) { if err == nil { return v, nil } - if err != ErrNoConfigEntry && err != ErrMultipleConfigEntry { + if !errors.Is(err, ErrNoConfigEntry) && err != ErrMultipleConfigEntry { return false, err } return m.global.ReadBool(key) @@ -107,7 +107,7 @@ func (m *mergedConfig) ReadString(key string) (string, error) { if err == nil { return val, nil } - if err != ErrNoConfigEntry && err != ErrMultipleConfigEntry { + if !errors.Is(err, ErrNoConfigEntry) && err != ErrMultipleConfigEntry { return "", err } return m.global.ReadString(key) @@ -118,7 +118,7 @@ func (m *mergedConfig) ReadTimestamp(key string) (time.Time, error) { if err == nil { return val, nil } - if err != ErrNoConfigEntry && err != ErrMultipleConfigEntry { + if !errors.Is(err, ErrNoConfigEntry) && err != ErrMultipleConfigEntry { return time.Time{}, err } return m.global.ReadTimestamp(key) diff --git a/repository/config_mem.go b/repository/config_mem.go index 019bc11167f1d2d939ac569ff4be0a6edfe234b5..bc532373619c1267f74e3d61e2d20835da094437 100644 --- a/repository/config_mem.go +++ b/repository/config_mem.go @@ -49,7 +49,7 @@ func (mc *MemConfig) ReadString(key string) (string, error) { key = normalizeKey(key) val, ok := mc.config[key] if !ok { - return "", ErrNoConfigEntry + return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) } return val, nil diff --git a/repository/config_testing.go b/repository/config_testing.go index f8a2762b6eecde560264b4f0d9382db4007dac1d..8c22934a7d8f8f45164f9bc3ec9a34406acf0096 100644 --- a/repository/config_testing.go +++ b/repository/config_testing.go @@ -53,7 +53,7 @@ func testConfig(t *testing.T, config Config) { }, configs) _, err = config.ReadBool("section.true") - require.Equal(t, ErrNoConfigEntry, err) + require.ErrorIs(t, err, ErrNoConfigEntry) err = config.RemoveAll("section.nonexistingkey") require.Error(t, err) @@ -62,7 +62,7 @@ func testConfig(t *testing.T, config Config) { require.NoError(t, err) _, err = config.ReadString("section.key") - require.Equal(t, ErrNoConfigEntry, err) + require.ErrorIs(t, err, ErrNoConfigEntry) err = config.RemoveAll("nonexistingsection") require.Error(t, err) diff --git a/repository/gogit.go b/repository/gogit.go index c1f1fe37d35c6c410ca1a894863c6269bf225d09..35934c911e8c63cc235a605ee0c42f189e915c40 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -2,6 +2,7 @@ package repository import ( "bytes" + "errors" "fmt" "io/ioutil" "os" @@ -270,7 +271,7 @@ func (repo *GoGitRepo) GetCoreEditor() (string, error) { if err == nil && val != "" { return val, nil } - if err != nil && err != ErrNoConfigEntry { + if err != nil && !errors.Is(err, ErrNoConfigEntry) { return "", err } diff --git a/repository/gogit_config.go b/repository/gogit_config.go index 891e3ffb8525213bd99979ec9ee500204b159f4e..87141e9de7b81d692975117296fe653abbe9fdf8 100644 --- a/repository/gogit_config.go +++ b/repository/gogit_config.go @@ -119,7 +119,7 @@ func (cr *goGitConfigReader) ReadString(key string) (string, error) { sectionName := split[0] if !cfg.Raw.HasSection(sectionName) { - return "", ErrNoConfigEntry + return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) } section := cfg.Raw.Section(sectionName) @@ -127,7 +127,7 @@ func (cr *goGitConfigReader) ReadString(key string) (string, error) { case len(split) == 2: optionName := split[1] if !section.HasOption(optionName) { - return "", ErrNoConfigEntry + return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) } if len(section.OptionAll(optionName)) > 1 { return "", ErrMultipleConfigEntry @@ -137,11 +137,11 @@ func (cr *goGitConfigReader) ReadString(key string) (string, error) { subsectionName := strings.Join(split[1:len(split)-1], ".") optionName := split[len(split)-1] if !section.HasSubsection(subsectionName) { - return "", ErrNoConfigEntry + return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) } subsection := section.Subsection(subsectionName) if !subsection.HasOption(optionName) { - return "", ErrNoConfigEntry + return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) } if len(subsection.OptionAll(optionName)) > 1 { return "", ErrMultipleConfigEntry From 49929c034f52272986ea6ac7457e64dbaa9454cb Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Sun, 27 Nov 2022 20:56:45 -0500 Subject: [PATCH 2/3] feat: wrap ErrMultipleConfigEntry to report duplicate key References #935. --- entities/identity/identity_user.go | 2 +- repository/config.go | 6 +++--- repository/gogit_config.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/entities/identity/identity_user.go b/entities/identity/identity_user.go index 435af060af744ae33f5aa55e8670b30b7b7d51da..cbbb89744f5f974745c5277f30f401cfbc099c7b 100644 --- a/entities/identity/identity_user.go +++ b/entities/identity/identity_user.go @@ -39,7 +39,7 @@ func GetUserIdentityId(repo repository.Repo) (entity.Id, error) { if errors.Is(err, repository.ErrNoConfigEntry) { return entity.UnsetId, ErrNoIdentitySet } - if err == repository.ErrMultipleConfigEntry { + if errors.Is(err, repository.ErrMultipleConfigEntry) { return entity.UnsetId, ErrMultipleIdentitiesSet } if err != nil { diff --git a/repository/config.go b/repository/config.go index 4d6a8c6c4decbef44649cef074a357a56635e058..781a0c1deb9681de422e5be0bfaa4588a16cfdb9 100644 --- a/repository/config.go +++ b/repository/config.go @@ -96,7 +96,7 @@ func (m *mergedConfig) ReadBool(key string) (bool, error) { if err == nil { return v, nil } - if !errors.Is(err, ErrNoConfigEntry) && err != ErrMultipleConfigEntry { + if !errors.Is(err, ErrNoConfigEntry) && !errors.Is(err, ErrMultipleConfigEntry) { return false, err } return m.global.ReadBool(key) @@ -107,7 +107,7 @@ func (m *mergedConfig) ReadString(key string) (string, error) { if err == nil { return val, nil } - if !errors.Is(err, ErrNoConfigEntry) && err != ErrMultipleConfigEntry { + if !errors.Is(err, ErrNoConfigEntry) && !errors.Is(err, ErrMultipleConfigEntry) { return "", err } return m.global.ReadString(key) @@ -118,7 +118,7 @@ func (m *mergedConfig) ReadTimestamp(key string) (time.Time, error) { if err == nil { return val, nil } - if !errors.Is(err, ErrNoConfigEntry) && err != ErrMultipleConfigEntry { + if !errors.Is(err, ErrNoConfigEntry) && !errors.Is(err, ErrMultipleConfigEntry) { return time.Time{}, err } return m.global.ReadTimestamp(key) diff --git a/repository/gogit_config.go b/repository/gogit_config.go index 87141e9de7b81d692975117296fe653abbe9fdf8..2bdbadc0313d05698644dfb138af978758c9f78f 100644 --- a/repository/gogit_config.go +++ b/repository/gogit_config.go @@ -130,7 +130,7 @@ func (cr *goGitConfigReader) ReadString(key string) (string, error) { return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) } if len(section.OptionAll(optionName)) > 1 { - return "", ErrMultipleConfigEntry + return "", fmt.Errorf("%w: duplicated key %s", ErrMultipleConfigEntry, key) } return section.Option(optionName), nil default: @@ -144,7 +144,7 @@ func (cr *goGitConfigReader) ReadString(key string) (string, error) { return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) } if len(subsection.OptionAll(optionName)) > 1 { - return "", ErrMultipleConfigEntry + return "", fmt.Errorf("%w: duplicated key %s", ErrMultipleConfigEntry, key) } return subsection.Option(optionName), nil } From 0cd2f3b4bda294b040e31337ddb380fb72192a63 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Mon, 28 Nov 2022 15:15:41 -0500 Subject: [PATCH 3/3] fix: remove repeated use of the same fmt.Errorf() calls --- repository/config.go | 9 +++++++++ repository/config_mem.go | 2 +- repository/gogit_config.go | 12 ++++++------ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/repository/config.go b/repository/config.go index 781a0c1deb9681de422e5be0bfaa4588a16cfdb9..7e1ee6e85ec6e45dfa224e7df7e751f5cfc8af23 100644 --- a/repository/config.go +++ b/repository/config.go @@ -2,6 +2,7 @@ package repository import ( "errors" + "fmt" "strconv" "time" ) @@ -11,6 +12,14 @@ var ( ErrMultipleConfigEntry = errors.New("multiple config entry for the given key") ) +func newErrNoConfigEntry(key string) error { + return fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) +} + +func newErrMultipleConfigEntry(key string) error { + return fmt.Errorf("%w: duplicated key %s", ErrMultipleConfigEntry, key) +} + // Config represent the common function interacting with the repository config storage type Config interface { ConfigRead diff --git a/repository/config_mem.go b/repository/config_mem.go index bc532373619c1267f74e3d61e2d20835da094437..55b12fd7c3ba9fc413eeb4095ca92d3ee8f9e30f 100644 --- a/repository/config_mem.go +++ b/repository/config_mem.go @@ -49,7 +49,7 @@ func (mc *MemConfig) ReadString(key string) (string, error) { key = normalizeKey(key) val, ok := mc.config[key] if !ok { - return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) + return "", newErrNoConfigEntry(key) } return val, nil diff --git a/repository/gogit_config.go b/repository/gogit_config.go index 2bdbadc0313d05698644dfb138af978758c9f78f..afa652b1983795dafd4b2f1fd984773b19f385fb 100644 --- a/repository/gogit_config.go +++ b/repository/gogit_config.go @@ -119,7 +119,7 @@ func (cr *goGitConfigReader) ReadString(key string) (string, error) { sectionName := split[0] if !cfg.Raw.HasSection(sectionName) { - return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) + return "", newErrNoConfigEntry(key) } section := cfg.Raw.Section(sectionName) @@ -127,24 +127,24 @@ func (cr *goGitConfigReader) ReadString(key string) (string, error) { case len(split) == 2: optionName := split[1] if !section.HasOption(optionName) { - return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) + return "", newErrNoConfigEntry(key) } if len(section.OptionAll(optionName)) > 1 { - return "", fmt.Errorf("%w: duplicated key %s", ErrMultipleConfigEntry, key) + return "", newErrMultipleConfigEntry(key) } return section.Option(optionName), nil default: subsectionName := strings.Join(split[1:len(split)-1], ".") optionName := split[len(split)-1] if !section.HasSubsection(subsectionName) { - return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) + return "", newErrNoConfigEntry(key) } subsection := section.Subsection(subsectionName) if !subsection.HasOption(optionName) { - return "", fmt.Errorf("%w: missing key %s", ErrNoConfigEntry, key) + return "", newErrNoConfigEntry(key) } if len(subsection.OptionAll(optionName)) > 1 { - return "", fmt.Errorf("%w: duplicated key %s", ErrMultipleConfigEntry, key) + return "", newErrMultipleConfigEntry(key) } return subsection.Option(optionName), nil }