Closed udondan closed 4 years ago
I like the way you're thinking of this and definitely agree that it would be a good feature to have. We have had some people ask about the bet way to do this in the past, and I suggested a relatively simple way to solve it by setting a global with the flag's value.:
var myFlag = os.Getenv("MY_FLAG")
If flaggy does not get a flag, it will not change the previously parsed environment variable. However, this still needs added to a help message as a prepend and isn't noted in the help output at all. The only reason this isn't in flaggy is because I wasn't sure of the best clean and intuitive way to solve the issue.
I think it is possible to add an entirely new set of flags that do the flag name to environment variable conversion logic you described. For example, we could create functions named flaggy.StringSliceEnv()
instead of just flaggy.StringSlice
. This would let people easily indicate which values they wanted to have environment variables. We should just be careful to clearly document the environment variable creation logic so it works as expected and isn't a surprise to the user.
If we get that right, it would be easier to use than specifying flags manually. On the flip side, it does not solve the problem of overriding the environment variable use (as you mentioned with the HOME
environment variable.... I am not sure what to do there yet. Maybe that means a new Env
function won't be compatible here - or perhaps it means that the Env
style functions have an additional parameter to specify the environment variable name.
I actually have two long flights across the USA this week. I will take a stab at this in a branch (tomorrow and Tuesday) and report back. I think I know how to work this into the parsers in a way that makes sense.
var myFlag = os.Getenv("MY_FLAG")
Oh dear, I tend to overcomplicate things. Indeed, it is that simple to accomplish. 🤦♂
However, this still needs added to a help message as a prepend and isn't noted in the help output at all.
I don't know, not necessarily. I actually am not aware of any program that indicates env var counterparts for flags in the hep message. That's something that can be added to the documentation, or prepend/append help message. Or if the customer wants it in the flag description, it can be simply added to its desc.
I agree with everything you wrote, wrt to flaggy.StringSliceEnv()
and an additional parameter for defining the flag name. Makes all sense. But I'm no longer sure it is worth the effort now that you made me realize how simple it is with the default value.
Safe travels! 😸
One thing to keep in mind is, that with var myFlag = os.Getenv("MY_FLAG")
, the help message would show the value of the env var as default value. This might be confusing, since it is a not a fixed default.
On the other hand this might also be used to the users advantage, e.g.
var flagLang = os.Getenv("LANG")
var flagTime = os.Getenv("LC_TIME")
...
I ended up leaving my laptop at the TSA in Seattle, so I didn't get to use my 10+ hours of flight time productively as I had hoped... 😞
Perhaps it should be noted how to deal with environment vars as described above in the documentation. It's not obvious that flags not used by the user don't change from their original values.
I think the Env
suffixed functions could be helpful, but maybe it's not worth the extra code complexity... Somehow, I am less motivated to write this feature than I was before my trip. I'll try adding something to the readme.
I'm planning to integrate flaggy into an existing project, replacing the currently used urfave/cli package. This existing program also provides the option to pass in options via environment variables. The env part now could be solved on its own. But I was thinking if it wasn't a cool feature for flaggy itself?
It could look something like this:
If
--flag
was not passed to the program, we would use the env varFLAG
if it is set.or
With a prefix we would look for
MYAPP_FLAG
.Precedence should be clear:
--flag
FLAG
Lowercase flags would always relate to UPPERCASE env vars. Hyphens would be replaced with underscores. CamelCase will be split into separate words. So
--someFlag
would relate toSOME_FLAG
.By default the feature should be disabled, as I can see it could cause problems in some situations, where flags are named like common bash vars, e.g.
HOME
,TERM
, etcI'm not sure about subcommand flags. For my use case I would only need to add support for global flags - for functionality that would trigger special functionality on a global level, like
MYAPP_DEBUG=1
. But IF we want that feature as well for subcommand flags, I guess it will make sense to include the subcommand name into the env var name, to avoid conflicts with flags from other subcommands.Corresponding env vars would be:
MYAPP_FLAG_A
MYAPP_SUBCOMMAND_EXAMPLE_FLAG_B
Happy to work on this. Just wanted to check if you were interested in this feature or would reject it.
I also do like the way urfave/cli has support for custom env vars per flag. But I don't see how we could implement this in a backwards compatible way into flaggy. The function signature of all flag functions would change.