spf13 / viper

Go configuration with fangs
MIT License
26.89k stars 2.01k forks source link

ConfigFileUsed returns empty after setting config name and config path #900

Open wfernandes opened 4 years ago

wfernandes commented 4 years ago

I have the file ~/.myconfdir/myconf.yaml When I do

viper.SetConfigName("myconf")
viper.AddConfigPath("~/.myconfdir")

And then call viper.ConfigFileUsed() I get an empty string because it doesn't set v.configFile. The current implementation is

func (v *Viper) ConfigFileUsed() string {
  return v.configFile
}

I propose that the following suggestion

func (v *Viper) ConfigFileUsed() string {
  file, _ := v.getConfigFile()
  return file
}
sagikazarmark commented 4 years ago

The documentation for ConfigFileUsed says:

ConfigFileUsed returns the file used to populate the config registry.

Before calling ReadInConfig this is empty, because nothing is read at that point.

Another issue with the proposed solution is that Viper is actually mutable, so after reading you can still call SetConfigName and AddConfigPath, but ConfigFileUsed should still return the original file the last ReadInConfig resolved, especially because Viper can write changes back to the same file.

For these reasons, I'm a bit reluctant to accept the proposed change, especially since calling ReadInConfig gives you the correct path.

wfernandes commented 4 years ago

@sagikazarmark Thanks for getting back to me.

Regarding the fact that viper can write back changes to the file. It looks like WriteConfig doesn't use v.configFile but rather calls v.getConfigFile().

https://github.com/spf13/viper/blob/c6ee9808ab8547ad8021467022fbc955d750a7ea/viper.go#L1381-L1387

To better explain our use case. Essentially, if there is no user specified path and there is no config in the ConfigFolder, we don't want it to err and read in the config. But if there is a config file in the ConfigFolder then we want to read it in.

var ConfigFolder = ".my-app-folder"
var ConfigName = "myconf"

if userSpecifiedPath != "" {
    if _, err := os.Stat(userSpecifiedPath); err != nil {
        return err
    }
    // Use path file from the flag.
    viper.SetConfigFile(userSpecifiedPath)
} else {
    // Checks if there is a default .my-app-folder/myConf{.extension} file in home directory
    viper.SetConfigName(ConfigName)
        viper.AddConfigPath(filepath.Join(homedir.HomeDir(), ConfigFolder))
        _, err := os.Stat(viper.ConfigFileUsed())
       if err != nil {
         // we don't want to return an error if there is no default config file available
         return nil
       }
}

if err := viper.ReadInConfig(); err != nil {
  // if there is a default config and there is an issue reading it, then we want to error.
  return err
}

I guess it would be nice to know what config is going to be used before having to read in the config.

sagikazarmark commented 4 years ago

For the use case described above I would suggest checking if the error is a ConfigFileNotFoundError:

    err := v.ReadInConfig()
    if _, configFileNotFound := err.(viper.ConfigFileNotFoundError); err != nil && !configFileNotFound {
        return err
    }

Does that work for you? I don't think changing the existing behavior of a function is a good idea, so adding the feature would have to go into a separate function, but Viper's API is already jumbled enough, so I would prefer not adding more weird getters with probably no or very little benefit.

wfernandes commented 4 years ago

@sagikazarmark Yeah I thought that adding a method like ConfigFileWhichWillBeUsed() would be silly 😆 I had initially done what you had suggested but I ended by using viper.SupportedExts.

Feel free to close this Issue and related PR if you don't think this is going to get merged in. 🙂

DanielRuf commented 3 years ago

I face the same issue in Hugo. When I run hugo config --debug it always prints an empty string, even when we set a different config file via the --config option:

INFO 2020/11/29 20:38:30 Using config file: