lab: rework config ordering

The current config order is ~/.config/lab, . (dot, ie the local
directory), and then .git/lab/.  Viper uses a first-found
algorithm to find the configs; if a config is found in the local
directory, then the config in .git/lab/ will not be used.

This model is different from the standard git model of allowing more local
configs to override higher order configs.  In git the ~/.gitconfig file
is overriden by any settings in .git/config.  This model is likely
preferred by most lab users and would allow per-worktree settings of
the host and token.

To be clear the new ordering and behaviour is

	The lab config heirarchy is:
	    1. ENV variables (LAB_CORE_TOKEN, LAB_CORE_HOST)
		    - if specified, core.token and core.host values in
		      config files are not updated.
	    2. "dot" . user specified config
		    - if specified, lower order config files will not
		      override the user specified config
	    3.  .config/lab/lab.toml (global config)
	    4.  .git/lab/lab/toml (worktree config)

Viper does not have a MergeInConfig() but does allow the merging of
data through MergeConfig().  I've used this to merge in the worktree
config.

Rework the config ordering to allow for config overrides.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>

cmd/issue_show.go

@@ -11,25 +11,22 @@ import (
 	"github.com/fatih/color"
 	"github.com/rsteube/carapace"
 	"github.com/spf13/cobra"
-	"github.com/spf13/viper"
 	gitlab "github.com/xanzy/go-gitlab"
 	"github.com/zaquestion/lab/internal/action"
 	"github.com/zaquestion/lab/internal/config"
 	lab "github.com/zaquestion/lab/internal/gitlab"
 )
 
-var (
-	issueShowConfig *viper.Viper
-	issueShowPrefix string = "issue_show."
-)
-
 var issueShowCmd = &cobra.Command{
 	Use:        "show [remote] <id>",
 	Aliases:    []string{"get"},
 	ArgAliases: []string{"s"},
 	Short:      "Describe an issue",
 	Long:       ``,
 	Run: func(cmd *cobra.Command, args []string) {
+
+		setCommandPrefix()
+
 		rn, issueNum, err := parseArgs(args)
 		if err != nil {
 			log.Fatal(err)
@@ -40,8 +37,6 @@ var issueShowCmd = &cobra.Command{
 			log.Fatal(err)
 		}
 
-		issueShowConfig = config.LoadConfig("", "")
-
 		noMarkdown, _ := cmd.Flags().GetBool("no-markdown")
 		if err != nil {
 			log.Fatal(err)
@@ -52,7 +47,7 @@ var issueShowCmd = &cobra.Command{
 
 		showComments, _ := cmd.Flags().GetBool("comments")
 		if showComments == false {
-			showComments = issueShowConfig.GetBool(issueShowPrefix + "comments")
+			showComments = getMainConfig().GetBool(CommandPrefix + "comments")
 		}
 		if showComments {
 			discussions, err := lab.IssueListDiscussions(rn, int(issueNum))
@@ -139,7 +134,7 @@ func printDiscussions(discussions []*gitlab.Discussion, since string, issueNum i
 	)
 	CompareTime, err = dateparse.ParseLocal(since)
 	if err != nil || CompareTime.IsZero() {
-		CompareTime = issueShowConfig.GetTime(issueShowPrefix + issueEntry)
+		CompareTime = getMainConfig().GetTime(CommandPrefix + issueEntry)
 		if CompareTime.IsZero() {
 			CompareTime = time.Now().UTC()
 		}
@@ -190,7 +185,7 @@ func printDiscussions(discussions []*gitlab.Discussion, since string, issueNum i
 	}
 
 	if sinceIsSet == false {
-		config.WriteConfigEntry(issueShowPrefix+issueEntry, NewAccessTime, "", "")
+		config.WriteConfigEntry(CommandPrefix+issueEntry, NewAccessTime, "", "")
 	}
 }
 

cmd/mr_show.go

@@ -11,7 +11,6 @@ import (
 	"github.com/fatih/color"
 	"github.com/rsteube/carapace"
 	"github.com/spf13/cobra"
-	"github.com/spf13/viper"
 	gitlab "github.com/xanzy/go-gitlab"
 	"github.com/zaquestion/lab/internal/action"
 	"github.com/zaquestion/lab/internal/config"
@@ -22,8 +21,6 @@ import (
 var (
 	mrShowPatch        bool
 	mrShowPatchReverse bool
-	mrShowConfig       *viper.Viper
-	mrShowPrefix       string = "mr_show."
 )
 
 var mrShowCmd = &cobra.Command{
@@ -33,6 +30,9 @@ var mrShowCmd = &cobra.Command{
 	Short:      "Describe a merge request",
 	Long:       ``,
 	Run: func(cmd *cobra.Command, args []string) {
+
+		setCommandPrefix()
+
 		rn, mrNum, err := parseArgs(args)
 		if err != nil {
 			log.Fatal(err)
@@ -43,8 +43,6 @@ var mrShowCmd = &cobra.Command{
 			log.Fatal(err)
 		}
 
-		mrShowConfig = config.LoadConfig("", "")
-
 		noMarkdown, _ := cmd.Flags().GetBool("no-markdown")
 		if err != nil {
 			log.Fatal(err)
@@ -73,7 +71,7 @@ var mrShowCmd = &cobra.Command{
 
 		showComments, _ := cmd.Flags().GetBool("comments")
 		if showComments == false {
-			showComments = mrShowConfig.GetBool(mrShowPrefix + "comments")
+			showComments = getMainConfig().GetBool(CommandPrefix + "comments")
 		}
 		if showComments {
 			discussions, err := lab.MRListDiscussions(rn, int(mrNum))
@@ -180,7 +178,7 @@ func printMRDiscussions(discussions []*gitlab.Discussion, since string, mrNum in
 	)
 	CompareTime, err = dateparse.ParseLocal(since)
 	if err != nil || CompareTime.IsZero() {
-		CompareTime = mrShowConfig.GetTime(mrShowPrefix + mrEntry)
+		CompareTime = getMainConfig().GetTime(CommandPrefix + mrEntry)
 		if CompareTime.IsZero() {
 			CompareTime = time.Now().UTC()
 		}
@@ -232,7 +230,7 @@ func printMRDiscussions(discussions []*gitlab.Discussion, since string, mrNum in
 	}
 
 	if sinceIsSet == false {
-		config.WriteConfigEntry(mrShowPrefix+mrEntry, NewAccessTime, "", "")
+		config.WriteConfigEntry(CommandPrefix+mrEntry, NewAccessTime, "", "")
 	}
 }
 

cmd/util.go

@@ -2,9 +2,35 @@
 package cmd
 
 import (
+	"path"
+	"runtime"
 	"strings"
+
+	"github.com/spf13/viper"
+	"github.com/zaquestion/lab/internal/config"
+)
+
+var (
+	CommandPrefix string
 )
 
+// getMainConfig returns the merged config of ~/.config/lab/lab.toml and
+// .git/lab/lab.toml
+func getMainConfig() *viper.Viper {
+	return config.MainConfig
+}
+
+// setCommandPrefix sets command name that is used in the config
+// files to set per-command options.  For example, the "lab issue show"
+// command has a prefix of "issue_show.", and "lab mr list" as a
+// prefix of "mr_list."
+func setCommandPrefix() {
+	_, file, _, _ := runtime.Caller(1)
+	_, filename := path.Split(file)
+	s := strings.Split(filename, ".")
+	CommandPrefix = s[0] + "."
+}
+
 // textToMarkdown converts text with markdown friendly line breaks
 // See https://gist.github.com/shaunlebron/746476e6e7a4d698b373 for more info.
 func textToMarkdown(text string) string {

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/pkg/errors v0.8.1
 	github.com/rivo/tview v0.0.0-20191129065140-82b05c9fb329
 	github.com/rsteube/carapace v0.0.16
+	github.com/spf13/afero v1.1.2
 	github.com/spf13/cobra v1.0.0
 	github.com/spf13/pflag v1.0.5
 	github.com/spf13/viper v1.4.0

internal/config/config.go

@@ -16,6 +16,7 @@ import (
 	"strings"
 	"syscall"
 
+	"github.com/spf13/afero"
 	"github.com/spf13/viper"
 	gitlab "github.com/xanzy/go-gitlab"
 	"github.com/zaquestion/lab/internal/git"
@@ -73,6 +74,10 @@ func New(confpath string, r io.Reader) error {
 		return err
 	}
 	fmt.Printf("\nConfig saved to %s\n", confpath)
+	err = MainConfig.ReadInConfig()
+	if err != nil {
+		log.Fatal(err)
+	}
 	return nil
 }
 
@@ -171,6 +176,11 @@ func ConvertHCLtoTOML(oldpath string, newpath string, file string) {
 }
 
 func getUser(host, token string, skipVerify bool) string {
+	user := MainConfig.GetString("core.user")
+	if user != "" {
+		return user
+	}
+
 	httpClient := &http.Client{
 		Transport: &http.Transport{
 			TLSClientConfig: &tls.Config{
@@ -183,6 +193,12 @@ func getUser(host, token string, skipVerify bool) string {
 	if err != nil {
 		log.Fatal(err)
 	}
+
+	if strings.TrimSpace(os.Getenv("LAB_CORE_TOKEN")) == "" && strings.TrimSpace(os.Getenv("LAB_CORE_HOST")) == "" {
+		MainConfig.Set("core.user", u.Username)
+		MainConfig.WriteConfig()
+	}
+
 	return u.Username
 }
 
@@ -211,6 +227,17 @@ func GetToken() string {
 // LoadMainConfig() loads the main config file and returns a tuple of
 //  host, user, token, ca_file, skipVerify
 func LoadMainConfig() (string, string, string, string, bool) {
+	// The lab config heirarchy is:
+	//	1. ENV variables (LAB_CORE_TOKEN, LAB_CORE_HOST)
+	//		- if specified, core.token and core.host values in
+	//		  config files are not updated.
+	//	2. "dot" . user specified config
+	//		- if specified, lower order config files will not override
+	//		  the user specified config
+	//	3.  .config/lab/lab.toml (global config)
+	//	4.  .git/lab/lab/toml (worktree config)
+	//
+	// Values from the worktree config will override any global config settings.
 
 	// Attempt to auto-configure for GitLab CI.
 	// Always do this before reading in the config file o/w CI will end up
@@ -250,6 +277,8 @@ func LoadMainConfig() (string, string, string, string, bool) {
 	MainConfig = viper.New()
 	MainConfig.SetConfigName("lab")
 	MainConfig.SetConfigType("toml")
+	// The local path (aka 'dot slash') does not allow for any
+	// overrides from the work tree lab.toml
 	MainConfig.AddConfigPath(".")
 	MainConfig.AddConfigPath(labconfpath)
 	if labgitDir != "" {
@@ -261,30 +290,28 @@ func LoadMainConfig() (string, string, string, string, bool) {
 	MainConfig.AutomaticEnv()
 
 	if _, ok := MainConfig.ReadInConfig().(viper.ConfigFileNotFoundError); ok {
+		// Create a new config
 		err := New(labconfpath, os.Stdin)
 		if err != nil {
 			log.Fatal(err)
 		}
-
-		err = MainConfig.ReadInConfig()
-		if err != nil {
-			log.Fatal(err)
+	} else {
+		// Config already exists.  Merge in .git/lab/lab.toml file
+		_, err := os.Stat(labgitDir + "/lab.toml")
+		if MainConfig.ConfigFileUsed() == labconfpath+"/lab.toml" && !os.IsNotExist(err) {
+			file, err := afero.ReadFile(afero.NewOsFs(), labgitDir+"/lab.toml")
+			if err != nil {
+				log.Fatal(err)
+			}
+			MainConfig.MergeConfig(bytes.NewReader(file))
 		}
 	}
 
 	host = MainConfig.GetString("core.host")
-	user = MainConfig.GetString("core.user")
 	token = GetToken()
-	tlsSkipVerify := MainConfig.GetBool("tls.skip_verify")
 	ca_file := MainConfig.GetString("tls.ca_file")
-
-	if user == "" {
-		user = getUser(host, token, tlsSkipVerify)
-		if strings.TrimSpace(os.Getenv("LAB_CORE_TOKEN")) == "" && strings.TrimSpace(os.Getenv("LAB_CORE_HOST")) == "" {
-			MainConfig.Set("core.user", user)
-			MainConfig.WriteConfig()
-		}
-	}
+	tlsSkipVerify := MainConfig.GetBool("tls.skip_verify")
+	user = getUser(host, token, tlsSkipVerify)
 
 	return host, user, token, ca_file, tlsSkipVerify
 }