Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conf.ConfigFileVariable doesn't support 'short:' option #35

Open
costela opened this issue Jan 29, 2019 · 1 comment
Open

Conf.ConfigFileVariable doesn't support 'short:' option #35

costela opened this issue Jan 29, 2019 · 1 comment

Comments

@costela
Copy link
Contributor

costela commented Jan 29, 2019

Setting the short: struct tag on the config element referred to by ConfigFileVariable is currently a noop. Currently the docs are unclear about this little gotcha.

E.g.: using something like:

var conf = &struct{
    ConfPath string `id:"confpath" short:"c"`
}{}

gonfig.Load(&conf, gonfig.Conf{
        ConfigFileVariable:  "confpath",
})

And then calling go run . -c /some/file will generate no errors, since the -c flag is recognized, but its value is ignored when searching the actual config file.

I suspect this is something we can work around in lookupConfigFileFlag() (maybe just naively check flagsMap[configOpt.short]?) but it just wasn't enough of an itch for me to scratch. So I'm leaving the issue here until either I get around to it, or someone else beats me to it! 😉

Cheers

@stevenroose
Copy link
Owner

Aha, good catch! Yeah that shouldn't be that hard a fix. Might give it a look next time I'm using gonfig myself. Thanks for reporting!

costela added a commit to RegioHelden/innovazammad that referenced this issue Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants