Merge pull request #36 from Rican7/feature/config-improvement-and-xdg…

…-basedir-spec-compliance

Feature - Config Loading Improvements

README.md

@@ -26,25 +26,17 @@ go install github.com/Rican7/define@latest
 
 The **define** app allows configuration through multiple means. You can either set configuration via:
 
-- Command line flags (good for one-off use)
-- A configuration file (good for your "dotfiles")
-- Environment variables (especially useful for API keys)
+1. Command line flags (good for one-off use)
+2. Environment variables (good for API keys)
+3. A configuration file (good for your "dotfiles")
+
+When multiple means of configuration are used, the values will take precedence in the aforementioned priority.
 
 
 ### Command line flags
 
 The list of command line flags is easily discovered via the `--help` flag. Any passed command line flag will take precedence over any other configuration mechanism.
 
-### Configuration file
-
-A configuration file can be stored at `~/.define.conf.json` and **define** will automatically load the values specified there.
-
-To print the default values of the configuration, simply use the `--print-config` flag. This can also be used to initialize a configuration file, for example:
-
-```shell
-define --print-config > ~/.define.conf.json
-```
-
 ### Environment variables
 
 Some configuration values can also be specified via environment variables. This is especially useful for API keys of different sources.
@@ -55,6 +47,23 @@ The following environment variables are read by **define**'s sources:
 - `OXFORD_DICTIONARY_APP_ID`
 - `OXFORD_DICTIONARY_APP_KEY`
 
+### Configuration file
+
+A configuration file can be stored that **define** will automatically load the values from.
+
+The path of the configuration file to load can be specified via the `--config-file` flag. If no config file path is specified, **define** will search for a config file in your OS's standard config directory paths. While these paths are OS-specific, there are two locations that are searched for that are shared among all platforms:
+
+1. `$XDG_CONFIG_HOME/define/config.json` (This is only searched for when the `$XDG_CONFIG_HOME` env variable is set)
+2. `~/.define.conf.json` (Where `~` is equal to your `$HOME` or user directory for your OS)
+
+To see which config file has been loaded, and to check what paths are searched for config files, use the `--debug-config` flag.
+
+To print the default values of the configuration, simply use the `--print-config` flag. This can also be used to initialize a configuration file, for example:
+
+```shell
+define --print-config > ~/.define.conf.json
+```
+
 
 ## Sources
 

define.go

@@ -26,9 +26,8 @@ import (
 
 const (
 	// Configuration defaults
-	defaultConfigFileLocation = "~/.define.conf.json"
-	defaultIndentationSize    = 2
-	defaultPreferredSource    = oxford.JSONKey
+	defaultIndentationSize = 2
+	defaultPreferredSource = oxford.JSONKey
 
 	fallbackSearchResultLimit = 5
 )
@@ -67,7 +66,7 @@ func init() {
 		providerConfsList = append(providerConfsList, providerConf)
 	}
 
-	conf, err = config.NewFromRuntime(flags, providerConfs, defaultConfigFileLocation, config.Configuration{
+	conf, err = config.NewFromRuntime(flags, providerConfs, config.Configuration{
 		IndentationSize: defaultIndentationSize,
 		PreferredSource: defaultPreferredSource,
 	})
@@ -151,6 +150,27 @@ func printConfig() {
 	stdOutWriter.WriteStringLine(string(encoded))
 }
 
+func printConfigDebug() {
+	stdOutWriter.IndentWrites(func(writer *defineio.PanicWriter) {
+		writer.WriteNewLine()
+
+		switch configFilePath := conf.FilePath(); configFilePath {
+		case "":
+			writer.WriteStringLine("No config file was loaded.")
+		default:
+			writer.WriteStringLine(fmt.Sprintf("A config file was loaded from %q", configFilePath))
+		}
+
+		writer.WritePaddedStringLine("The following locations are searched for config files (in this order):", 1)
+
+		for i, filePath := range config.FilePaths() {
+			writer.WriteStringLine(fmt.Sprintf("%d. %s", i+1, filePath))
+		}
+
+		writer.WriteNewLine()
+	})
+}
+
 func printSources() {
 	var sourceStrings []string
 
@@ -238,6 +258,8 @@ func main() {
 	switch act.Type() {
 	case action.PrintConfig:
 		printConfig()
+	case action.DebugConfig:
+		printConfigDebug()
 	case action.ListSources:
 		printSources()
 	case action.PrintVersion:

go.mod

@@ -4,8 +4,10 @@ go 1.22.0
 
 require (
 	dario.cat/mergo v1.0.0
+	github.com/adrg/xdg v0.4.0
 	github.com/fatih/structs v1.1.0
-	github.com/mitchellh/go-homedir v1.1.0
 	github.com/ogier/pflag v0.0.0-20160129220114-45c278ab3607
 	golang.org/x/text v0.14.0
 )
+
+require golang.org/x/sys v0.5.0 // indirect

go.sum

@@ -1,13 +1,24 @@
 dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk=
 dario.cat/mergo v1.0.0/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk=
+github.com/adrg/xdg v0.4.0 h1:RzRqFcjH4nE5C6oTAxhBtoE2IRyjBSa62SCbyPidvls=
+github.com/adrg/xdg v0.4.0/go.mod h1:N6ag73EX4wyxeaoeHctc1mas01KZgsj5tYiAIwqJE/E=
+github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
+github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/fatih/structs v1.1.0 h1:Q7juDM0QtcnhCpeyLGQKyg4TOIghuNXrkL32pHAUMxo=
 github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M=
-github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
-github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/ogier/pflag v0.0.0-20160129220114-45c278ab3607 h1:db+rES1EpSjP45xOU3hgS41oawQiZzqfnl6dUgBdFjY=
 github.com/ogier/pflag v0.0.0-20160129220114-45c278ab3607/go.mod h1:zkFki7tvTa0tafRvTBIZTvzYyAu6kQhPZFnshFFPE+g=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
+github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
+golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
 golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

internal/action/action.go

@@ -12,6 +12,7 @@ import (
 const (
 	DefineWord Type = iota
 	PrintConfig
+	DebugConfig
 	ListSources
 	PrintVersion
 )
@@ -24,6 +25,7 @@ type Action struct {
 	flagSet *flag.FlagSet
 	flag    struct {
 		printConfig  bool
+		debugConfig  bool
 		listSources  bool
 		printVersion bool
 	}
@@ -38,6 +40,7 @@ func Setup(flags *flag.FlagSet) *Action {
 
 	// Define our flags
 	flags.BoolVar(&act.flag.printConfig, "print-config", false, "To print the current configuration")
+	flags.BoolVar(&act.flag.debugConfig, "debug-config", false, "To print debug info about the configuration")
 	flags.BoolVar(&act.flag.listSources, "list-sources", false, "To print the available sources")
 	flags.BoolVar(&act.flag.printVersion, "version", false, "To print the app's version info")
 
@@ -61,6 +64,8 @@ func (a *Action) Type() Type {
 	switch {
 	case a.flag.printConfig:
 		return PrintConfig
+	case a.flag.debugConfig:
+		return DebugConfig
 	case a.flag.listSources:
 		return ListSources
 	case a.flag.printVersion:

internal/config/config.go

@@ -5,14 +5,14 @@
 package config
 
 import (
+	"cmp"
 	"encoding/json"
 	"fmt"
 	"os"
 	"strconv"
 
 	"github.com/Rican7/define/registry"
 	"github.com/fatih/structs"
-	homedir "github.com/mitchellh/go-homedir"
 	flag "github.com/ogier/pflag"
 
 	"dario.cat/mergo"
@@ -25,42 +25,25 @@ type Configuration struct {
 	Source          string
 
 	// Private fields that shouldn't be externally set or output
-	providerConfigs    map[string]registry.Configuration
-	configFileLocation string
-	noConfigFile       bool
+	providerConfigs map[string]registry.Configuration
+	configFilePath  string
+	noConfigFile    bool
 }
 
 // initializeCommandLineConfig initializes the command line configuration.
-func initializeCommandLineConfig(flags *flag.FlagSet) *Configuration {
+func initializeCommandLineConfig(flags *flag.FlagSet, defaults Configuration) *Configuration {
 	var conf Configuration
 
 	// Define our flags
-	flags.StringVarP(&conf.configFileLocation, "config-file", "c", "", "The location of the config file to use")
+	flags.StringVarP(&conf.configFilePath, "config-file", "c", defaults.configFilePath, "The path of the config file to use")
 	flags.BoolVar(&conf.noConfigFile, "no-config-file", false, "To not load any config file")
-	flags.UintVar(&conf.IndentationSize, "indent-size", 0, "The number of spaces to indent output by")
-	flags.StringVar(&conf.PreferredSource, "preferred-source", "", "The preferred source to use, if available and able to be provided")
-	flags.StringVarP(&conf.Source, "source", "s", "", "The source to use (will error if unavailable or unable to be provided)")
+	flags.UintVar(&conf.IndentationSize, "indent-size", defaults.IndentationSize, "The number of spaces to indent output by")
+	flags.StringVar(&conf.PreferredSource, "preferred-source", defaults.PreferredSource, "The preferred source to use, if available and able to be provided")
+	flags.StringVarP(&conf.Source, "source", "s", defaults.Source, "The source to use (will error if unavailable or unable to be provided)")
 
 	return &conf
 }
 
-// initializeFileConfig initializes the file configuration by loading the
-// configuration from a file at the given location.
-func initializeFileConfig(fileLocation string) (Configuration, error) {
-	var conf Configuration
-
-	fileContents, err := os.ReadFile(tryExpandPath(fileLocation))
-	if err != nil {
-		return conf, err
-	}
-
-	if len(fileContents) > 0 {
-		err = json.Unmarshal(fileContents, &conf)
-	}
-
-	return conf, err
-}
-
 // initializeEnvironmentConfig initializes the environment configuration from
 // the application's environment.
 func initializeEnvironmentConfig() Configuration {
@@ -76,6 +59,27 @@ func initializeEnvironmentConfig() Configuration {
 	return conf
 }
 
+// initializeFileConfig initializes the file configuration by loading the
+// configuration from a file at the given path.
+func initializeFileConfig(filePath string) (Configuration, error) {
+	var conf Configuration
+
+	filePath = tryExpandUserPath(filePath)
+
+	fileContents, err := os.ReadFile(filePath)
+	if err != nil {
+		return conf, err
+	}
+
+	if len(fileContents) > 0 {
+		err = json.Unmarshal(fileContents, &conf)
+	}
+
+	conf.configFilePath = filePath
+
+	return conf, err
+}
+
 // mergeConfigurations merges multiple configurations values together, from left
 // to right argument position, by filling any of the left arguments zero-values
 // with any non-zero-values from the right.
@@ -86,19 +90,13 @@ func mergeConfigurations(confs ...Configuration) (Configuration, error) {
 		if err := mergo.Merge(&merged, conf); err != nil {
 			return merged, err
 		}
-	}
 
-	return merged, nil
-}
-
-// tryExpandPath attempts to expand a given path and returns the expanded path
-// if successful. Otherwise, if expansion failed, the original path is returned.
-func tryExpandPath(path string) string {
-	if expanded, err := homedir.Expand(path); err == nil {
-		path = expanded
+		// Set private (unexported values), as mergo can't handle those.
+		merged.configFilePath = cmp.Or(merged.configFilePath, conf.configFilePath)
+		merged.noConfigFile = cmp.Or(merged.noConfigFile, conf.noConfigFile)
 	}
 
-	return path
+	return merged, nil
 }
 
 // NewFromRuntime builds a Configuration by merging values from multiple
@@ -108,54 +106,45 @@ func tryExpandPath(path string) string {
 //
 // The merging of values from different sources will take this priority:
 // 1. Command line arguments
-// 2. A loaded config file, if available
-// 3. Environment variables
+// 2. Environment variables
+// 3. A loaded config file, if available
 // 4. Passed in default values
 func NewFromRuntime(
 	flags *flag.FlagSet,
 	providerConfigs map[string]registry.Configuration,
-	defaultConfigFileLocation string,
 	defaults Configuration,
 ) (Configuration, error) {
 	var conf Configuration
 	var err error
 
 	var fileConfig Configuration
 
-	// Set our config file location
-	defaults.configFileLocation = tryExpandPath(defaultConfigFileLocation)
+	// Set our config file path based on our first found default location.
+	defaults.configFilePath = findConfigFile()
 
-	commandLineConfig := initializeCommandLineConfig(flags)
+	commandLineConfig := initializeCommandLineConfig(flags, defaults)
 
 	// Parse our flag set, as we need the values from the commandLineConfig
 	err = flags.Parse(os.Args[1:])
 
 	if err == nil && !commandLineConfig.noConfigFile {
-		configFileLocation := tryExpandPath(commandLineConfig.configFileLocation)
-
-		if configFileLocation == "" && defaults.configFileLocation != "" {
-			// If we haven't passed a config file flag, and our default exists
-			if _, err := os.Stat(defaults.configFileLocation); !os.IsNotExist(err) {
-				// Set our location to the default, since it exists
-				// (if there are problems reading the file, we'll handle later)
-				configFileLocation = defaults.configFileLocation
-			}
-		}
+		// This path should have either the user-passed value or a found default
+		configFilePath := tryExpandUserPath(commandLineConfig.configFilePath)
 
 		// If we have a config file to load
-		if configFileLocation != "" {
-			fileConfig, err = initializeFileConfig(configFileLocation)
+		if configFilePath != "" {
+			fileConfig, err = initializeFileConfig(configFilePath)
 			if err != nil {
-				err = fmt.Errorf("error reading config file %q with error: %s", configFileLocation, err)
+				err = fmt.Errorf("error reading config file %q with error: %s", configFilePath, err)
 			}
 		}
 	}
 
 	if err == nil {
 		conf, err = mergeConfigurations(
 			*commandLineConfig,
-			fileConfig,
 			initializeEnvironmentConfig(),
+			fileConfig,
 			defaults,
 		)
 	}
@@ -176,6 +165,11 @@ func (c Configuration) ProviderConfigs() []registry.Configuration {
 	return list
 }
 
+// FilePath returns the path of the file that was loaded for the configuration.
+func (c Configuration) FilePath() string {
+	return c.configFilePath
+}
+
 // MarshalJSON defines how the configuration should be JSON marshalled.
 func (c Configuration) MarshalJSON() ([]byte, error) {
 	configMap := structs.Map(c)