Open quirijnslings opened 4 years ago
Can you provide the suggested Delegate signature that you want to pass to Parser with pseudocode example.
While working out the details, I changed my mind slightly: the delegate must be a parameter to the OptionAttribute, not to the parser. After all, you may want to have a different method to get the value for different parameters, as the example below shows.
Consider a simple MyOptions class, with only 2 properties: Username and Password (both string). Both are required (because the program cannot run without them). But the user has not supplied them.
The framework must be extended with a delegate as follows:
public delegate object GetParameter(OptionAttribute optionAttribute);
The BaseAttribute is enhanced with the following property:
public GetParameter ParamGetter { get; set; }
Somewhere inside the framework (not sure where, I haven't looked in detail at your code), there is probably some code that iterates over all the properties of the specified Options object and the associated attribute:
// iterate over all OptionAttributes and associated properties
if (optionAttribute.ParamGetter != null)
{
var value = ParamGetter(optionAttribute);
if (value != null)
{
// use this value for the current property
// some type mapping must be done here because the delegate returns an object which must be converted to the
// type of the property
}
}
}
The developer using CommandLineParser would use it like this:
public class MyOptions : Options
{
[Option("username", Required = true, HelpText = "Enter your username", ParamGetter = GetUsername)]
public string Username { get; set; }
[Option("password", Required = true, HelpText = "Enter your password", ParamGetter = GetPassword)]
public string Password { get; set; }
}
// ask the user to enter the username on the commandline
public object GetUsername(optionAttribute)
{
Console.WriteLine(optionAttribute.HelpText) + ":";
var username = Console.ReadLine();
return username;
}
// ask the user to enter the password on the commandline, hiding it from sight in case someone is watching
public object GetPassword(optionAttribute)
{
Console.WriteLine(optionAttribute.HelpText) + ":";
string pass = "";
do
{
ConsoleKeyInfo key = Console.ReadKey(true);
// Backspace Should Not Work
if (key.Key != ConsoleKey.Backspace && key.Key != ConsoleKey.Enter)
{
pass += key.KeyChar;
Console.Write("*");
}
else
{
if (key.Key == ConsoleKey.Backspace && pass.Length > 0)
{
pass = pass.Substring(0, (pass.Length - 1));
Console.Write("\b \b");
}
else if (key.Key == ConsoleKey.Enter)
{
break;
}
}
} while (true);
Console.WriteLine();
return pass;
}
Note: if it would be possible to use a delegate with a generic type argument, that would be even better. In that case, the developer could write a method with a specific return type instead of the rather ugly 'object'.
I understand that you want parser can read input from keyboard at runtime for some arguments.
the delegate must be a parameter to the OptionAttribute
Delegate is not supported in Attributes, see attribute parameter types
Just a suggestion change for your design:
We can add a property to OptionAttribute
like:
public bool EnableKeyboardInput {get;set}
When EnableKeyboardInput==true
parser read the argument value.
KeyboardInput can be a Built-in component to read user keyboard based on the true value of EnableKeyboardInput.
Question: How parser know that the argument is password protected?
[ ] Add other property to OptionAttribute
like Secret : true/false
[ ] what is your suggestion?
Question: Do parser need to validate input keyboard: numeric /datetime (format) or be relaxed?
Question: When to read arguments?
It can be read at the start of parsing based on the specs of options and arguments. Question: what is the title (label) of input?
The title is the option shortname /or longname (user is not aware of the option property name).
E.g
Enter missing: -a
What is your suggestions based on the questions above?
Thank you for the feedback. I was hoping to make the system more decoupled by introducing delegates, but as this is not possible, I agree with your suggestions. Some notes:
Regarding the label of the input, I would say if there is a longname we will use that because it is more descriptive, otherwise we use the shortname. Perhaps we can also include the help text (if available) between brackets? So:
Enter missing --item-values (Complete help text)
Secret attribute: agreed
When to read arguments: only if no parameter has been supplied. It should take place when the ParseArguments command is executed, so that errors in the input will end up in the ParseError handler.
If there are multiple 'EnableKeyboardInput' params, the order in which you ask for input is not important.
Type casting: can we reuse the logic for type casting when a parameter is supplied for the command line? The only difference is the handling of multi-value (lists, enumerables, arrays) properties, in which case we will add 'comma separated' as you suggest.
For ints just do a TryParseInt and throw an exception if the value cannot be parsed. For DateTime, use DateTime.Parse(string) - this will take the current culture, so developers are able to influence the outcome by setting the culture of their choice. But I guess these type casting rules are in the project already, since they are also needed if you supply the values directly from the commandline.
I still investigate other solutions and I'll wait to listen from the community for other suggested solutions for this interactive feature.
@moh-hassan I would like to have an integrated interactive mode, too. Is that feature already available?
Regarding the detection of Password options. I don't have the usecase yet, why not identifying them by the type SecureString?
[Option("pass", Required = true, HelpText = "Enter your password")] public SecureString Password { get; set; }
I'm interested in this too, currently I use the WithParsed option to test the parameters and ask at the console.
I would rather use a Func
I would like to have a new parameter for Option: bool Interactive. When this is set to true, a delegate function will be called (which you need to pass in to the Parser somehow). This must happen BEFORE the check for required parameters is done. This will allow the developer to ask for the values interactively. The return type of the delegate is object. When the delegate returns a non-null value, the value is assigned to the parameter (if the parameter is of a primitive type, the object is first cast to a nullable and the Value is used). When the delegate returns null, nothing happens and the Parser continues normally (possibly failing when the parameter was set to required). The construction with the delegate ensures encapsulation (not an extra dependency on the console), and also gives the developer additional flexibility. But it's also fine if this is implemented as a simple method inside the parser, interacting with the console from CommandlineParser directly.
Either way, I think this would be a really helpful enhancement for many users of CommandLineParser.