Add environment variable support

[jannickj]

Fixes: #677

Allows for enviroment variables to be part of a cli setup the order is Args > Env > Default
NB. Currently does not support sequences (Future work - will instead crash with a helpful error msg)

I also changed 0/1 to be parsable booleans as that's the language agnotisc / most common way to set booleans via environment variables.

[jannickj]

Hello?

[jannickj]

👋

[jannickj]

🌧️
🚶‍♂️

[SommerEngineering]

@ericnewton76 Could this be part of the 2.9.0 release, please? 🙂

[jannickj]

🛌

[jannickj]

🌤️

[jannickj]

🌞

[BoBiene]

@ericnewton76 & @moh-hassan would be great to get this feature :)

[johnjaylward]

It looks like @moh-hassan has left this project (he's not listed in the org anymore), and @ericnewton76 has been inactive on GitHub since November. It effectively looks like this project has been abandoned, which is a shame :(

[BoBiene]

So there is no one able to merge pullrequests to this repo?
@gsscoder you are listed as contact, are you able to maintain this great project?

[jannickj]

🌇

[jannickj]

🏙️

[jannickj]

🦗

[jannickj]

💀

[devedse]

I created a NuGet package based on your branch:
https://www.nuget.org/packages/CommandLineParserWithEnvironmentVariables/2.8.0

[devedse]

@jannickj , I did find one issue though. If Required = true and you did provide the value for the variable through an EnvironmentVariable it's still showing the error that a required-option is missing.

Not sure if that is desirable.

[jannickj]

@devedse thx :D, also would say that is in my view expected behavior :)

[devedse]

@jannickj , the thing is that this results in me having to check all variables in the code to see if they are not null. Even though it is required, thus, should be provided.

But I'm not sure what the best solution would be.

[jannickj]

yeah it's a bit annoying the way to get around it is to just set it equal to null!
ex.

[Option("host",
	HelpText = "The host for the client",
	Default = "localhost:80")]
public string Host { get; set; } = null!;

[devedse]

That's not really what I mean. What I mean is that if you set Required = true, you need to provide the variable by passing in the flag.

For example, let's assume we have:

[Option("data",
        Required = true,
	HelpText = "The data",
        Env = "DATA"
)]
public string Data { get; set; } = null!;

#This would work:
testapp --data 123

#This would not work:
SET DATA=123
testapp

#Now we're getting the message that --data is missing.

[jannickj]

right... I wasn't clear in my first message then, I tried to argue that it could make sense since required sort of strongly requests that you set the field. That said I understand what you mean and the more I think about the more I'm convinced that it's the behavior you expect.

[devedse]

That's what I thought first as well. But I'm doubting it a bit now since if you look at it from the consumer end it just requires the data. It doesn't matter where it comes from. I can't think of a scenario where actually passing it as a flag is required if there is also an environment variable.

But either way is fine. I'll just work with the way is implemented now.

[jannickj]

That said I understand what you mean and the more I think about the more I'm convinced that it's the behavior you expect.

just read what I wrote, what I meant to say is that the behavior you (@devedse ) expected is the correct behavior, i.e. required accepts environment variables 😅

[devedse]

@jannickj , are you planning on implementing this?

[jannickj]

@devedse probably not, I made this PR 1.5 years ago, so at this point I'm okay with it, if someone revived this git-repo then maybe but not on a half-dead project.

[devedse]

I'll see if I can make a quick fix then. I'll just release a new version of my NuGet Package

[jannickj]

💪

[devedse]

I think I fixed it. I actually moved all the code for setting the Value from the BuildMutable Method to the OptionMapper.MapValues as this is probably the place where we'd want to add this stuff.
devedse@9ef1591

I also wrote some test scenario's that test Required = true in combination with providing the value through Environment values or arguments or neither.

I've released this version to NuGet:
https://www.nuget.org/packages/CommandLineParserWithEnvironmentVariables/2.8.1

[devedse]

Feel free to do a code review, Incase I missed something.