refine around type config

config.go

@@ -54,10 +54,10 @@ const (
 )
 
 type config struct {
-	releaseBranch *configValue
-	versionFile   *configValue
-	command       *configValue
-	template      *configValue
+	releaseBranch *string
+	versionFile   *string
+	command       *string
+	template      *string
 	vPrefix       *bool
 	changelog     *bool
 
@@ -76,32 +76,20 @@ func newConfig(gitPath string) (*config, error) {
 
 func (cfg *config) Reload() error {
 	if rb := os.Getenv(envReleaseBranch); rb != "" {
-		cfg.releaseBranch = &configValue{
-			value:  rb,
-			source: srcEnv,
-		}
+		cfg.releaseBranch = github.String(rb)
 	} else {
 		out, err := cfg.gitconfig.Get(configReleaseBranch)
 		if err == nil {
-			cfg.releaseBranch = &configValue{
-				value:  out,
-				source: srcConfigFile,
-			}
+			cfg.releaseBranch = github.String(out)
 		}
 	}
 
 	if rb := os.Getenv(envVersionFile); rb != "" {
-		cfg.versionFile = &configValue{
-			value:  rb,
-			source: srcEnv,
-		}
+		cfg.versionFile = github.String(rb)
 	} else {
 		out, err := cfg.gitconfig.Get(configVersionFile)
 		if err == nil {
-			cfg.versionFile = &configValue{
-				value:  out,
-				source: srcConfigFile,
-			}
+			cfg.versionFile = github.String(out)
 		}
 	}
 
@@ -132,32 +120,20 @@ func (cfg *config) Reload() error {
 	}
 
 	if command := os.Getenv(envCommand); command != "" {
-		cfg.command = &configValue{
-			value:  command,
-			source: srcEnv,
-		}
+		cfg.command = github.String(command)
 	} else {
 		command, err := cfg.gitconfig.Get(configCommand)
 		if err == nil {
-			cfg.command = &configValue{
-				value:  command,
-				source: srcConfigFile,
-			}
+			cfg.command = github.String(command)
 		}
 	}
 
 	if tmpl := os.Getenv(envTemplate); tmpl != "" {
-		cfg.template = &configValue{
-			value:  tmpl,
-			source: srcEnv,
-		}
+		cfg.template = github.String(tmpl)
 	} else {
-		template, err := cfg.gitconfig.Get(configTemplate)
+		tmpl, err := cfg.gitconfig.Get(configTemplate)
 		if err == nil {
-			cfg.template = &configValue{
-				value:  template,
-				source: srcConfigFile,
-			}
+			cfg.template = github.String(tmpl)
 		}
 	}
 
@@ -188,7 +164,7 @@ func (cfg *config) initializeFile() error {
 	if err := os.RemoveAll(cfg.conf); err != nil {
 		return err
 	}
-	if err := os.WriteFile(cfg.conf, []byte(defaultConfigContent), 0666); err != nil {
+	if err := os.WriteFile(cfg.conf, []byte(defaultConfigContent), 0644); err != nil {
 		return err
 	}
 	return nil
@@ -198,21 +174,15 @@ func (cfg *config) SetRelaseBranch(br string) error {
 	if err := cfg.set(configReleaseBranch, br); err != nil {
 		return err
 	}
-	cfg.releaseBranch = &configValue{
-		value:  br,
-		source: srcDetect,
-	}
+	cfg.releaseBranch = github.String(br)
 	return nil
 }
 
 func (cfg *config) SetVersionFile(fpath string) error {
 	if err := cfg.set(configVersionFile, fpath); err != nil {
 		return err
 	}
-	cfg.versionFile = &configValue{
-		value:  fpath,
-		source: srcDetect,
-	}
+	cfg.versionFile = github.String(fpath)
 	return nil
 }
 
@@ -224,42 +194,25 @@ func (cfg *config) SetVPrefix(vPrefix bool) error {
 	return nil
 }
 
-func (cfg *config) ReleaseBranch() *configValue {
-	return cfg.releaseBranch
-}
-
-func (cfg *config) VersionFile() *configValue {
-	return cfg.versionFile
-}
-
-func (cfg *config) Command() *configValue {
-	return cfg.command
+func stringify(pstr *string) string {
+	if pstr == nil || *pstr == "-" {
+		return ""
+	}
+	return *pstr
 }
 
-func (cfg *config) Template() *configValue {
-	return cfg.template
+func (cfg *config) ReleaseBranch() string {
+	return stringify(cfg.releaseBranch)
 }
 
-type configValue struct {
-	value  string
-	source configSource
+func (cfg *config) VersionFile() string {
+	return stringify(cfg.versionFile)
 }
 
-func (cv *configValue) String() string {
-	if cv.value == "-" {
-		return ""
-	}
-	return cv.value
+func (cfg *config) Command() string {
+	return stringify(cfg.command)
 }
 
-func (cv *configValue) Empty() bool {
-	return cv.String() == ""
+func (cfg *config) Template() string {
+	return stringify(cfg.template)
 }
-
-type configSource int
-
-const (
-	srcEnv configSource = iota
-	srcConfigFile
-	srcDetect
-)

config_test.go

@@ -0,0 +1,57 @@
+package tagpr
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/Songmu/gitconfig"
+)
+
+func TestConfig(t *testing.T) {
+	tmpdir := t.TempDir()
+	confPath := filepath.Join(tmpdir, defaultConfigFile)
+	cfg := &config{
+		conf:      confPath,
+		gitconfig: &gitconfig.Config{GitPath: "git", File: confPath},
+	}
+
+	if err := cfg.Reload(); err != nil {
+		t.Error(err)
+	}
+	if e, g := "", cfg.ReleaseBranch(); e != g {
+		t.Errorf("got: %s, expext: %s", g, e)
+	}
+	if err := cfg.SetRelaseBranch("main"); err != nil {
+		t.Error(err)
+	}
+	if e, g := "main", cfg.ReleaseBranch(); e != g {
+		t.Errorf("got: %s, expext: %s", g, e)
+	}
+	if err := cfg.SetVersionFile(""); err != nil {
+		t.Error(err)
+	}
+	if e, g := "", cfg.VersionFile(); e != g {
+		t.Errorf("got: %s, expext: %s", g, e)
+	}
+
+	b, err := os.ReadFile(confPath)
+	if err != nil {
+		t.Error(err)
+	}
+
+	var out string
+	for _, line := range strings.Split(string(b), "\n") {
+		if line != "" && !strings.HasPrefix(line, "#") {
+			out += line + "\n"
+		}
+	}
+	expect := `[tagpr]
+	releaseBranch = main
+	versionFile = -
+`
+	if out != expect {
+		t.Errorf("got:\n%s\nexpect:\n%s", out, expect)
+	}
+}

tag.go

@@ -29,12 +29,12 @@ func (tp *tagpr) tagRelease(ctx context.Context, pr *github.PullRequest, currVer
 		vfile string
 		err   error
 	)
-	releaseBranch := tp.cfg.releaseBranch.String()
+	releaseBranch := tp.cfg.ReleaseBranch()
 
 	// Using "HEAD~" to retrieve the one previous commit before merging does not work well in cases
 	// "Rebase and merge" was used. However, we don't care about "Rebase and merge" and only support
 	// "Create a merge commit" and "Squash and merge."
-	if tp.cfg.versionFile == nil {
+	if tp.cfg.VersionFile() == "" {
 		if _, _, err := tp.c.Git("checkout", "HEAD~"); err != nil {
 			return err
 		}
@@ -46,7 +46,7 @@ func (tp *tagpr) tagRelease(ctx context.Context, pr *github.PullRequest, currVer
 			return err
 		}
 	} else {
-		vfiles := strings.Split(tp.cfg.versionFile.String(), ",")
+		vfiles := strings.Split(tp.cfg.VersionFile(), ",")
 		vfile = strings.TrimSpace(vfiles[0])
 	}
 

tagpr.go

@@ -131,10 +131,7 @@ func (tp *tagpr) Run(ctx context.Context) error {
 		currVer.vPrefix = *tp.cfg.vPrefix
 	}
 
-	var releaseBranch string
-	if r := tp.cfg.ReleaseBranch(); r != nil {
-		releaseBranch = r.String()
-	}
+	releaseBranch := tp.cfg.ReleaseBranch()
 	if releaseBranch == "" {
 		releaseBranch, _ = tp.defaultBranch()
 		if releaseBranch == "" {
@@ -311,8 +308,8 @@ OUT:
 		addingLabels = append(addingLabels, l)
 	}
 	var vfiles []string
-	if vf := tp.cfg.VersionFile(); vf != nil {
-		vfiles = strings.Split(vf.String(), ",")
+	if vf := tp.cfg.VersionFile(); vf != "" {
+		vfiles = strings.Split(vf, ",")
 		for i, v := range vfiles {
 			vfiles[i] = strings.TrimSpace(v)
 		}
@@ -327,12 +324,11 @@ OUT:
 		vfiles = []string{vfile}
 	}
 
-	if com := tp.cfg.Command(); com != nil {
-		prog := com.String()
+	if prog := tp.cfg.Command(); prog != "" {
 		var progArgs []string
 		if strings.ContainsAny(prog, " \n") {
-			prog = "sh"
 			progArgs = []string{"-c", prog}
+			prog = "sh"
 		}
 		tp.c.Cmd(prog, progArgs...)
 	}
@@ -402,8 +398,8 @@ OUT:
 
 	// Reread the configuration file (.tagpr) as it may have been rewritten during the cherry-pick process.
 	tp.cfg.Reload()
-	if tp.cfg.VersionFile() != nil {
-		vfiles = strings.Split(tp.cfg.VersionFile().String(), ",")
+	if tp.cfg.VersionFile() != "" {
+		vfiles = strings.Split(tp.cfg.VersionFile(), ",")
 		for i, v := range vfiles {
 			vfiles[i] = strings.TrimSpace(v)
 		}
@@ -452,8 +448,8 @@ OUT:
 	}
 
 	var tmpl *template.Template
-	if t := tp.cfg.Template(); t != nil {
-		tmpTmpl, err := template.ParseFiles(t.String())
+	if t := tp.cfg.Template(); t != "" {
+		tmpTmpl, err := template.ParseFiles(t)
 		if err == nil {
 			tmpl = tmpTmpl
 		} else {