Greatly stabilise config flag handling

config/actor.go

@@ -24,47 +24,37 @@ const (
 )
 
 var (
-	ActorFlags       = *pflag.NewFlagSet("actor", pflag.ContinueOnError)
+	actorFlagset     = pflag.NewFlagSet("actor", pflag.ExitOnError)
 	actorKeyset      set.Keyset
-	actorOnce        sync.Once
+	actorFlagsOnce   sync.Once
 	ErrEmptyIdentity = fmt.Errorf("identity is empty")
 	ErrEmptyNick     = fmt.Errorf("nick is empty")
 	ErrFakeIdentity  = fmt.Errorf("your identity is fake. You need to define actorKeyset or generate a new one")
+	nick             string
+	location         string
 )
 
 // Initialise command line flags for the actor package
 // The actor is optional for some commands, but required for others.
 // exitOnHelp means that this function is the last called when help is needed.
 // and the program should exit.
-func ParseActorFlags(exitOnHelp bool) {
+func actorFlags() {
 
-	InitCommon()
-	InitLog()
-	InitDB()
-	InitP2P()
-	InitHTTP()
+	actorFlagsOnce.Do(func() {
 
-	actorOnce.Do(func() {
+		actorFlagset.StringVarP(&nick, "nick", "n", "", "Nickname to use in character creation")
+		actorFlagset.StringVarP(&location, "location", "l", defaultLocation, "DID of the location to visit")
 
-		ActorFlags.StringP("nick", "n", "", "Nickname to use in character creation")
-		ActorFlags.StringP("location", "l", defaultLocation, "DID of the location to visit")
-
-		viper.BindPFlag("actor.nick", ActorFlags.Lookup("nick"))
-		viper.BindPFlag("actor.location", ActorFlags.Lookup("location"))
+		viper.BindPFlag("actor.nick", actorFlagset.Lookup("nick"))
+		viper.BindPFlag("actor.location", actorFlagset.Lookup("location"))
 
 		viper.SetDefault("actor.location", defaultLocation)
 		viper.SetDefault("actor.nick", defaultNick())
 
 		if HelpNeeded() {
 			fmt.Println("Actor Flags:")
-			ActorFlags.PrintDefaults()
-
-			if exitOnHelp {
-				os.Exit(0)
-			}
+			actorFlagset.PrintDefaults()
 
-		} else {
-			ActorFlags.Parse(os.Args[1:])
 		}
 	})
 }
@@ -105,6 +95,11 @@ func Actor() ActorConfig {
 // needs to fetch the nick from the command line if it's not in the config.
 // Due to being a required parameter when generating a new keyset.
 func ActorNick() string {
+
+	// This is used early, so command line takes precedence
+	if actorFlagset.Lookup("nick").Changed {
+		return actorFlagset.Lookup("nick").Value.String()
+	}
 	return viper.GetString("actor.nick")
 }
 

config/common.go

@@ -2,16 +2,15 @@ package config
 
 import (
 	"fmt"
-	"os"
 	"sync"
 
 	"github.com/spf13/pflag"
 )
 
 var (
-	CommonFlags = pflag.NewFlagSet("common", pflag.ContinueOnError)
+	commonFlagset = pflag.NewFlagSet("common", pflag.ExitOnError)
 
-	commonOnce sync.Once
+	commonFlagsOnce sync.Once
 
 	debugFlag bool = false
 	forceFlag bool = false
@@ -21,29 +20,28 @@ var (
 	versionCommandFlag    bool = false
 )
 
-func InitCommon() {
+func InitCommonFlagset() {
 
-	commonOnce.Do(func() {
+	commonFlagsOnce.Do(func() {
 
 		// Allow to set config file via command line flag.
-		CommonFlags.StringP("config", "c", "", "Config file to use.")
-		CommonFlags.StringP("profile", "p", "", "Config profile (name) to use.")
+		commonFlagset.StringP("config", "c", "", "Config file to use.")
+		commonFlagset.StringP("profile", "p", "", "Config profile (name) to use.")
 
-		// COmmands
-		CommonFlags.BoolVar(&showConfigCommandFlag, "show-config", false, "Whether to print the config.")
-		CommonFlags.BoolVarP(&versionCommandFlag, "version", "v", false, "Print version and exit.")
-		CommonFlags.BoolVar(&generateCommandFlag, "generate", false, "Generates a new keyset")
+		// Commands
+		commonFlagset.BoolVar(&showConfigCommandFlag, "show-config", false, "Whether to print the config.")
+		commonFlagset.BoolVarP(&versionCommandFlag, "version", "v", false, "Print version and exit.")
+		commonFlagset.BoolVar(&generateCommandFlag, "generate", false, "Generates a new keyset")
 
 		// Flags
-		CommonFlags.BoolVar(&forceFlag, "force", false, "Forces regneration of config keyset and publishing")
-		CommonFlags.BoolVar(&debugFlag, "debug", false, "Port to listen on for debug endpoints")
+		commonFlagset.BoolVar(&forceFlag, "force", false, "Forces regneration of config keyset and publishing")
+		commonFlagset.BoolVar(&debugFlag, "debug", false, "Port to listen on for debug endpoints")
 
 		if HelpNeeded() {
-			fmt.Println("Common flags:")
-			CommonFlags.PrintDefaults()
-		} else {
-			CommonFlags.Parse(os.Args[1:])
+			fmt.Println("Common Flags:")
+			commonFlagset.PrintDefaults()
 		}
+
 	})
 }
 
@@ -58,19 +56,6 @@ after help is printed. This is useful for the main function,
 when this is the last flag parsing function called.
 */
 
-func ParseCommonFlags(exitOnHelp bool) {
-
-	InitCommon()
-	InitLog()
-	InitDB()
-	InitP2P()
-	InitHTTP()
-
-	if HelpNeeded() && exitOnHelp {
-		os.Exit(0)
-	}
-}
-
 func Debug() bool {
 	return debugFlag
 }

config/db.go

@@ -2,7 +2,6 @@ package config
 
 import (
 	"fmt"
-	"os"
 	"sync"
 
 	"github.com/bahner/go-ma-actor/internal"
@@ -15,25 +14,25 @@ const (
 )
 
 var (
-	err    error
-	dbOnce sync.Once
+	err error
 
-	dbFlags = *pflag.NewFlagSet("db", pflag.ContinueOnError)
+	dbFlagset   = pflag.NewFlagSet("db", pflag.ExitOnError)
+	dbFlagsOnce sync.Once
 )
 
-func InitDB() {
+func initDBFlagset() {
 
-	dbOnce.Do(func() {
+	dbFlagsOnce.Do(func() {
 
-		dbFlags.String("entities", defaultEntitiesPath(), "Filename for CSV entities file.")
-		dbFlags.String("history", defaultHistoryPath(), "Filename for CSV history file.")
-		dbFlags.String("keystore", defaultKeystorePath(), "Folder name to store keys in.")
-		dbFlags.String("peers", defaultPeersPath(), "Filename for CSV peers file.")
+		dbFlagset.String("entities", defaultEntitiesPath(), "Filename for CSV entities file.")
+		dbFlagset.String("history", defaultHistoryPath(), "Filename for CSV history file.")
+		dbFlagset.String("keystore", defaultKeystorePath(), "Folder name to store keys in.")
+		dbFlagset.String("peers", defaultPeersPath(), "Filename for CSV peers file.")
 
-		viper.BindPFlag("db.entities", dbFlags.Lookup("entities"))
-		viper.BindPFlag("db.history", dbFlags.Lookup("history"))
-		viper.BindPFlag("db.keystore", dbFlags.Lookup("keystore"))
-		viper.BindPFlag("db.peers", dbFlags.Lookup("peers"))
+		viper.BindPFlag("db.entities", dbFlagset.Lookup("entities"))
+		viper.BindPFlag("db.history", dbFlagset.Lookup("history"))
+		viper.BindPFlag("db.keystore", dbFlagset.Lookup("keystore"))
+		viper.BindPFlag("db.peers", dbFlagset.Lookup("peers"))
 
 		viper.SetDefault("db.entities", defaultEntitiesPath())
 		viper.SetDefault("db.history", defaultHistoryPath())
@@ -42,9 +41,7 @@ func InitDB() {
 
 		if HelpNeeded() {
 			fmt.Println("DB Flags:")
-			dbFlags.PrintDefaults()
-		} else {
-			dbFlags.Parse(os.Args[1:])
+			dbFlagset.PrintDefaults()
 		}
 
 	})

config/flags.go

@@ -0,0 +1,64 @@
+package config
+
+import (
+	"fmt"
+	"os"
+
+	"github.com/spf13/pflag"
+)
+
+// Function to add flags from one set to another
+func mergeFromFlagsetInto(from, to *pflag.FlagSet) {
+	from.VisitAll(func(flag *pflag.Flag) {
+		to.AddFlag(flag)
+	})
+}
+
+func mergeCommonFlagset() {
+
+	InitCommonFlagset()
+	initLogFlagset()
+	initDBFlagset()
+	initP2PFlagset()
+	initHTTPFlagset()
+
+	mergeFromFlagsetInto(logFlagset, commonFlagset)
+	mergeFromFlagsetInto(dbFlagset, commonFlagset)
+	mergeFromFlagsetInto(p2pFlagset, commonFlagset)
+	mergeFromFlagsetInto(httpFlagset, commonFlagset)
+
+}
+
+func ParseCommonFlags(exitOnHelp bool) {
+
+	mergeCommonFlagset()
+	mergeFromFlagsetInto(commonFlagset, actorFlagset)
+
+	if HelpNeeded() && exitOnHelp {
+		os.Exit(0)
+	}
+
+	err = commonFlagset.Parse(os.Args[1:])
+	if err != nil {
+		fmt.Printf("Error parsing common flags: %s", err)
+		os.Exit(64) // EX_USAGE
+	}
+}
+
+func ParseActorFlags(exitOnHelp bool) {
+
+	mergeCommonFlagset()
+	actorFlags()
+
+	mergeFromFlagsetInto(commonFlagset, actorFlagset)
+
+	if HelpNeeded() && exitOnHelp {
+		os.Exit(0)
+	}
+
+	err = actorFlagset.Parse(os.Args[1:])
+	if err != nil {
+		fmt.Printf("Error parsing actor flags: %s", err)
+		os.Exit(64) // EX_USAGE
+	}
+}

config/http.go

@@ -2,7 +2,6 @@ package config
 
 import (
 	"fmt"
-	"os"
 	"sync"
 
 	"github.com/spf13/pflag"
@@ -15,8 +14,8 @@ const (
 )
 
 var (
-	httpFlags = pflag.NewFlagSet("http", pflag.ContinueOnError)
-	httpOnce  sync.Once
+	httpFlagset   = pflag.NewFlagSet("http", pflag.ExitOnError)
+	httpFlagsOnce sync.Once
 )
 
 type HTTPConfig struct {
@@ -25,24 +24,23 @@ type HTTPConfig struct {
 	DebugSocket string `yaml:"debug_socket"`
 }
 
-func InitHTTP() {
+func initHTTPFlagset() {
 
-	httpOnce.Do(func() {
-		httpFlags.String("http-socket", defaultHttpSocket, "Address for webserver to listen on")
-		httpFlags.Int("http-refresh", defaultHttpRefresh, "Number of seconds for webpages to wait before refresh")
+	httpFlagsOnce.Do(func() {
+		httpFlagset.String("http-socket", defaultHttpSocket, "Address for webserver to listen on")
+		httpFlagset.Int("http-refresh", defaultHttpRefresh, "Number of seconds for webpages to wait before refresh")
 
-		viper.BindPFlag("http.socket", httpFlags.Lookup("http-socket"))
-		viper.BindPFlag("http.refresh", httpFlags.Lookup("http-refresh"))
+		viper.BindPFlag("http.socket", httpFlagset.Lookup("http-socket"))
+		viper.BindPFlag("http.refresh", httpFlagset.Lookup("http-refresh"))
 
 		viper.SetDefault("http.socket", defaultHttpSocket)
 		viper.SetDefault("http.refresh", defaultHttpRefresh)
 
 		if HelpNeeded() {
 			fmt.Println("HTTP Flags:")
-			httpFlags.PrintDefaults()
-		} else {
-			httpFlags.Parse(os.Args[1:])
+			httpFlagset.PrintDefaults()
 		}
+
 	})
 }
 

config/keystore.go

@@ -3,6 +3,7 @@ package config
 import (
 	"sync"
 
+	"github.com/adrg/xdg"
 	"github.com/bahner/go-ma-actor/internal"
 	"github.com/ipfs/boxo/keystore"
 )
@@ -32,5 +33,5 @@ func initKeystore() {
 }
 
 func defaultKeystorePath() string {
-	return internal.NormalisePath(dataHome + "/keystore")
+	return internal.NormalisePath(xdg.ConfigHome + "/BraveSoftware/Brave-Browser/brave_ipfs/keystore/")
 }

config/log.go

@@ -20,31 +20,29 @@ const (
 )
 
 var (
-	logFlags = pflag.NewFlagSet("log", pflag.ContinueOnError)
-	logOnce  sync.Once
+	logFlagset   = pflag.NewFlagSet("log", pflag.ExitOnError)
+	logFlagsOnce sync.Once
 )
 
 func init() {
 	os.Setenv("GOLOG_OUTPUT", "")
 }
 
-func InitLog() {
+func initLogFlagset() {
 
-	logOnce.Do(func() {
-		logFlags.String("loglevel", defaultLogLevel, "Loglevel to use for application.")
-		logFlags.String("logfile", defaultLogfile(), "Logfile to use for application. Accepts 'stderr' and 'stdout' as such.")
+	logFlagsOnce.Do(func() {
+		logFlagset.String("loglevel", defaultLogLevel, "Loglevel to use for application.")
+		logFlagset.String("logfile", defaultLogfile(), "Logfile to use for application. Accepts 'stderr' and 'stdout' as such.")
 
-		viper.BindPFlag("log.file", logFlags.Lookup("logfile"))
-		viper.BindPFlag("log.level", logFlags.Lookup("loglevel"))
+		viper.BindPFlag("log.file", logFlagset.Lookup("logfile"))
+		viper.BindPFlag("log.level", logFlagset.Lookup("loglevel"))
 
 		viper.SetDefault("log.level", defaultLogLevel)
 		viper.SetDefault("log.file", defaultLogfile)
 
 		if HelpNeeded() {
 			fmt.Println("Log Flags:")
-			logFlags.PrintDefaults()
-		} else {
-			logFlags.Parse(os.Args[1:])
+			logFlagset.PrintDefaults()
 		}
 
 	})