Tyrrrz / CliFx

Class-first framework for building command-line interfaces
MIT License
1.48k stars 60 forks source link

Environment variables #3

Closed Tyrrrz closed 4 years ago

Tyrrrz commented 5 years ago

Extend CommandOptionAttribute by adding EnvironmentVariableName. If the corresponding environment variable is set, the option property will take on its value, but only if the corresponding command line argument was not set. In other words, command line argument > environment variable > default value.

federico-paolillo commented 5 years ago

Hey @Tyrrrz I'm interested to give this issue a shot and try to implement it as it would be useful for some applications that I'm working on at work.

By looking around in your code I was thinking that maybe a good starting point to add the fallback to Environment Variables would be in CommandInitializer.InitializeCommand where if an OptionInput was missing values, Environment Variables could be read to provide the missing value.

What do you think ? Do you have any suggestions ?

Thank you !

Tyrrrz commented 5 years ago

Hi @federico-paolillo!

Thanks for taking interest.

I think your approach sounds good, but keep in mind that environment variables will need to be testable. Hence why I think they should be exposed via CommandInput. That would allow us to create an instance of CommandInput with environment variables without having to actually set them, and run appropriate tests. I think IReadOnlyDictionary<string, string> CommandInput.EnvironmentVariables {get;} would work for that.

Finally, CommandOptionSchema and CommandOptionAttribute will need to be extended so that the user can provide EnvironmentVariableName, so that it can be used like this:

[CommandOption("value", 'v', EnvironmentVariableName = "MYAPP_VALUE")]
public string MyValue { get; set; }

This is how I see it at the moment, if you have some ideas fire away.

federico-paolillo commented 5 years ago

Yes indeed, my idea was to abstract away Environment Variable access behind some interface like IEnvironmentVariablesProvider to easily test them.

About CommandInput.EnvironmentVariables that sound good and leaves the remainder of the command line processing as it is now.

As far as I understood CommandInput is parsed from ICommandInputParser and it looks like it is made to support command line arguments parsing only. My idea is to refactor away the command line arguments parsing logic in it's own class and add another class to process Environment Variables, then ICommandInputParser could orchestrate both class to produce a valid CommandInput. Does it sound good to you ?

Anyway, I'll start working on it and get back to you.

Tyrrrz commented 5 years ago

Yes indeed, my idea was to abstract away Environment Variable access behind some interface like IEnvironmentVariablesProvider to easily test them.

That's also an option, I just hate mocks :)

As far as I understood CommandInput is parsed from ICommandInputParser and it looks like it is made to support command line arguments parsing only. My idea is to refactor away the command line arguments parsing logic in it's own class and add another class to process Environment Variables, then ICommandInputParser could orchestrate both class to produce a valid CommandInput. Does it sound good to you ?

That's a valid point. Let's see how it goes and we can adjust the implementation later.