From 7dcf390188c61d8db988254a35c6bd439b779844 Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Wed, 16 Sep 2020 14:39:51 -0400 Subject: [PATCH] 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 --- cmd/issue_show.go | 17 +++++------- cmd/mr_show.go | 14 +++++----- cmd/util.go | 26 ++++++++++++++++++ go.mod | 1 + internal/config/config.go | 55 +++++++++++++++++++++++++++++---------- 5 files changed, 80 insertions(+), 33 deletions(-) diff --git a/cmd/issue_show.go b/cmd/issue_show.go index 06108e09..416784bf 100644 --- a/cmd/issue_show.go +++ b/cmd/issue_show.go @@ -11,18 +11,12 @@ 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] ", Aliases: []string{"get"}, @@ -30,6 +24,9 @@ var issueShowCmd = &cobra.Command{ 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, "", "") } } diff --git a/cmd/mr_show.go b/cmd/mr_show.go index a90fef21..68cd998c 100644 --- a/cmd/mr_show.go +++ b/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, "", "") } } diff --git a/cmd/util.go b/cmd/util.go index cfd80390..6c62dab0 100644 --- a/cmd/util.go +++ b/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 { diff --git a/go.mod b/go.mod index 30be0ca6..cb7e7069 100644 --- a/go.mod +++ b/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 diff --git a/internal/config/config.go b/internal/config/config.go index f5fb190f..791176dd 100644 --- a/internal/config/config.go +++ b/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 }