Closed mafredri closed 5 years ago
I agree this makes sense in the port case because 0 isn't valid at all, but as you mentioned in lots of other contexts there's ambiguity as to whether the zero value is intentional/explicit or not, and I think that kills the proposal.
FWIW the flag package has the same behavior here (if either flag or envconfig would leave the existing value alone, you could just call that one second and be done), and probably for the same reason. It's clearest and most straightforward to clobber the existing field regardless.
I think your best bet is to point the flag at a separate variable and then only conditionally overwrite your config struct field.
Thanks for the response, I appreciate you considering this.
I think your best bet is to point the flag at a separate variable and then only conditionally overwrite your config struct field.
I don't think this is possible the way you have suggested because there's no way to tell envconfig "we already have this value, please don't error out" without actually specifying the environment variable (as I showed above). That method is IMO a bit hack-ish and also the reason for this proposal.
"we already have this value, please don't error out"
this would be omitting required:"true"
Fair enough, I suppose that's one option, but it has the downside of the documentation not matching (e.g. via envconfig.Usage()
). It's a bit of a tricky problem, although, probably out of scope for envconfig
considering it seems to be designed to be the only source of configuration.
I have a relatively simple use-case: I would like to primarily use
envconfig
, and occasionally provide flags to change some (otherwise required) configs.Consider:
Currently,
envconfig
will produce an error, even if the program is started via./program -port 8080
. Considering the non-zero values inspec
would allow this program to work without changes.For contrast, this can trivially be rewritten to work with
envconfig
as-is:I realize this proposal is not without it's own set of problems. For example, what if the user wants to provide a zero value as the default. Then either
os.Setenv
is needed or the type should be a pointer (*int
).I also considered if a new tag was merited (e.g.
use_nonzero:"true"
), but this is not ideal and expanding the API for this use-case just isn't worth it.