Tyrrrz / CliFx

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

Allow the non scalar parameter to be optional or have default value #99

Closed kamran65536 closed 3 years ago

kamran65536 commented 3 years ago

Suppose I have an command like this:

[Command("run")]
public class RunThis : ICommand
{
    [CommandParameter(0, ...]
     public string[] NonScalarParameters {get;set;}

    public ValueTask Execute(IConsole console)
    {
         if (NonScalarParameters == null)
             console.Output.WriteLine("I set it to something.");
         else
             console.Output.WriteLine("using parameter values...");
}

As command parameters are mandatory, currently it is impossible to say:

util run p1 p2 p3
util run

But it is a common usage for such commands to have default value for the last parameter.

I suggest the following change in CommandBinder

            //CommandBinder.BindParameters
            if (remainingParameterSchemas.Any() && 
               ((remainingParameterSchemas.Count != 1) || remainingParameterSchemas[0].Property.IsScalar()))
            {
                throw CliFxException.UserError(
                    "Missing parameter(s):" +
                    Environment.NewLine +
                    remainingParameterSchemas
                        .Select(o => o.GetFormattedIdentifier())
                        .JoinToString(" ")
                );
            }
Tyrrrz commented 3 years ago

Can you show a more concrete example/use case of what you're trying to achieve? Making parameters mandatory was a design decision because they are not as explicit as options. Generally, if you want to make something optional, you should be using options.

alirezanet commented 2 years ago

Can you show a more concrete example/use case

Commands:
husky install [dir] (default: .husky)    Install Husky hooks

you asked for an example.

I also think the CommandParameter need a default property.

Tyrrrz commented 2 years ago

I think optional parameters will play poorly in cases where there are multiple of them. I recommend using options for optional properties instead.

alirezanet commented 2 years ago

I suggest supporting optional, just for the last parameter. or adding default option feature (a single option could be used without --name). It solves a lot of issues and it is really helpful.

Recently I migrated my custom CLI to clifx. everything is working fine except clifx caused a lot of unnecessary complexity to my CLI. The above install example is one of them.

husky install <SomeOptionalPath>

the other one is really annoying: before people could just pass custom arguments directly to their scripts: e.g

husky exec script1.csx
husky exec script1.csx arg1 arg2 arg3

now they have to add --args, (this is really counterintuitive)

husky exec script1.csx --args arg1 arg2 arg3

this may not seem a big deal but it is (especially here) because these commands can be preconfigured in a JSON/YAML file.

      {
         "command": "dotnet",
         "args": ["husky", "exec", "script1.csx", "--args", "${args}"]
      },

look at the args part ☹️, it is really confusing after I've added --args.

This clifx limitation is my only regret after migration.

In the end in my opinion you really should consider supporting this. I think you don't want to add this feature because of possible conflicts with the cmd routing feature that shouldn't be too hard to solve.

Let me know if you want to support this or not please, I need to decide what to do. thanks