Skip to content

Arity for command arguments are not validated when there are child commands #1151

Open
@iravanchi

Description

@iravanchi

When I specify a command argument as required (using arg.Arity = ArgumentArity.ExactlyOnce) on a command that has sub-commands, the arity is not validated when invoking any of the subcommands. They are only validated when the command is being invoked directly.

Activity

changed the title [-]Arity for command arguments are not validated[/-] [+]Arity for command arguments are not validated when there are child commands[/+] on Jan 3, 2021
jonsequitur

jonsequitur commented on Jan 4, 2021

@jonsequitur
Contributor

This is by design. There's no need for subcommands to share parameters on account of their hierarchy.

For example:

image

But you can add an argument at multiple positions in the hierarchy to get the behavior I think you're looking for. Note the uncommented line from the previous screen shot:

image

iravanchi

iravanchi commented on Jan 8, 2021

@iravanchi
Author

That perfectly makes sense, if the "middle" verbs are not meant to be executed.

I was trying to use the library in a usecase that might not be the original intention but is still nice to have. I'm using some verbs to wrap parts of the logic and make it reusable across other verbs. That's similar to what can be achieved by "piping" in the command line, but this allows me to do it in-process and pass objects instead of strings. Here's an example usage:

Suppose I'm creating a CLI for a CRM application, and want to allow a couple of actions of Client records via CLI. I need a bunch of different queries for clients, so one way is to do it like this:

cli notify-clients --email 'someone@somewhere.com' --message 'message to be sent to clients'
cli notify-clients --minimum-client-age 21 --message 'message to be sent to clients'
cli notify-clients --department 'department name' --message 'message to be sent to clients'

And there can be multiple actions meant for a "set of clients" (say disable-client, delete-client, update-client, ...)
So the intention is to reuse the "client query options" (such as --email, --minimum-age, --department, etc.) across all these commands. One way is to reuse the instances as you mentioned above.

What I'm going about to implement this, is to have a middle verb that's responsible for querying and passing the query result for the next command to run the action on it (exactly as one would expect the piping to work in the command prompt). Like this:

cli clients --email 'someone@somewhere.com' notify --message 'message to be sent to clients'
cli clients --min-client-age 21 notify --message 'message to be sent to clients'
cli clients --department 'department name' notify --message 'message to be sent to clients'

With this, just be adding a child verb, I can use the same query logic for a new action:

cli clients --department 'department name' update <update-options>

I've implemented this by child commands looking up the instance of their "parent" command and using its methods / properties to reuse that logic, but I had to use my own code to initialize and validate the options and arguments for that. In this type of usage, it can be useful to apply the same set of, for example, Arity validations on the middle command as well.

I understand that this is not the intention of the library, and that the behavior is by design. I personally find this way if re-use (and also organizing the commands for the CLI user) to be more productive (maybe only limited to my use case) and I just wanted to paint a picture on why validation of middle-command arity is something that can be helpful.

williamb1024

williamb1024 commented on Feb 28, 2021

@williamb1024

I am also of the opinion that inherited arguments and options should provide validation. The options validation was requested in #901.

From my experimentation, the help system code properly supports generation of arguments that exist within a parent command and the parser will properly assign the argument result. The missing support appears to be only in the validation code during the parser symbol visiting.

Would the project be open to accepting a pull request that provides support for inherited command, argument and option validation controlled by an additional property defined on the Command class?

The general idea would be that during the ParseResultVisitor.ValidateCommandResult if the current command has enabled inherited validation, the parent command would also be validated. Inherited validation would occur prior to the current command's validation.

jonsequitur

jonsequitur commented on Mar 2, 2021

@jonsequitur
Contributor

Can this behavior be accomplished using Command.AddGlobalOption on the command whose child commands you'd like to share options with?

williamb1024

williamb1024 commented on Mar 2, 2021

@williamb1024

In my case, I'm looking to share arguments rather than options.

An example of a possible CLI is a tool that uses an HTTP endpoint to perform some form of encoding or encryption:

tool.exe <serviceUrl> <action> <input>

This could be defined as (please forgive syntax errors, I'm entering this directly):

new RootCommand() {
  new Argument<string>("serviceUrl"),
  new Command("base64") {
    new Argument<FileInfo>("input")
  }.WithHandler(MyHandler),
  new Command("ascii85") {
    new Argument<FileInfo>("input")
  }.WithHandler(MyOtherHandler)
}

Even in this scenario there is a good bit of repetition, specifically with the <Input> argument, but the general structure of the command line seems more logical to me than implementing the same with options.

Ultimately, I suppose my desire is to support a CLI similar to what one would commonly find on networking hardware.

jonsequitur

jonsequitur commented on Mar 4, 2021

@jonsequitur
Contributor

With either arguments or options you can also add them to multiple positions within the tree:

var inputArg = new Argument<FileInfo>("input");

var cmd = new RootCommand 
{
    new Argument<string>("serviceUrl"),
    new Command("base64") {
        inputArg
    }.WithHandler(MyHandler),
    new Command("ascii85") {
        inputArg
    }.WithHandler(MyOtherHandler)
};
williamb1024

williamb1024 commented on Mar 9, 2021

@williamb1024
var inputArg = new Argument<FileInfo>("input");

var cmd = new RootCommand 
{
    new Argument<string>("serviceUrl"),
    new Command("base64") {
        inputArg
    }.WithHandler(MyHandler),
    new Command("ascii85") {
        inputArg
    }.WithHandler(MyOtherHandler)
};

In your example, the inputArg is replicated between the base64 and ascii85 verbs as expected. What isn't possible is validating that the serviceUrl argument has actually been provided. My described changes would allow the serviceUrl argument to be required and validated.

Currently these command lines are both considered valid:

application.exe "something-for-serviceUrl" base64 c:\file.txt
application.exe base64 c:\file.txt

The desired outcome would be that serviceUrl would be required and validated. This could be achieved by duplicating the serviceUrl argument within the commands, but that changes the CLI order and also affects the output of the help.

jonsequitur

jonsequitur commented on Mar 9, 2021

@jonsequitur
Contributor

To specify that the child commands require this argument, while also allowing it in the position immediately following the root command / arg0, you can do this:

image

williamb1024

williamb1024 commented on Mar 11, 2021

@williamb1024

Using this example, the serviceUrl argument appears twice in the help. Once before both the base64 and ascii85 verbs and again after each of the verbs.

I originally tried something similar, using two separate instances of the serviceUrl argument and hiding the one that belongs with each of the verbs. This resulted in a duplicate alias error. In your example, I don't believe it's possible to hide the either of the arguments given that all of the arguments are a reference to the same instance.

jonsequitur

jonsequitur commented on Mar 11, 2021

@jonsequitur
Contributor

Agreed. I think there are two different concerns here that are unfortunately coupled.

  • The parser configuration. (Is the one I've suggested behaviorally correct?)
  • The help output. (Clearly not ideal.)

Our help API's don't yet allow enough customization but we're working on that. Even then, I think it's challenging to convey the different inputs that are valid for a given parser setup. Some CLIs use a list of examples to make this clearer. The traditional synopsis isn't rich enough to convey positional arguments that are valid at more than one position but can only be provided once. Suggestions are welcome.

@Keboo @KathleenDollard Thoughts?

(Also, the help discussion is a but off topic for this issue but related: #1174.)

danielcweber

danielcweber commented on Sep 7, 2022

@danielcweber

What's especially misleading is that, while outerCommand --help would print

outerCommand <arg> innerCommand

suggesting that arg is required even when invoking innerCommand, it's not how the library behaves.

@jonsequitur wrote "There's no need for subcommands to share parameters on account of their hierarchy" but I haven't understood why exactly that is - I have a use case where I planned to layout my CLI to allow innerCommand to work on whatever was specified by arg. But maybe that's just not how modern CLIs are meant to work?

I was able to re-enable argument checking by moving this check into the loop and let it work on currentCommand and _rootCommandResult. I am completely oblivious to the consequences beyond that, however. I'll happily craft a PR if considered useful.

jonsequitur

jonsequitur commented on Sep 7, 2022

@jonsequitur
Contributor

What's especially misleading is that, while outerCommand --help would print

outerCommand <arg> innerCommand

This help is quite misleading. We should fix it as part of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @iravanchi@jonsequitur@danielcweber@williamb1024

        Issue actions

          Arity for command arguments are not validated when there are child commands · Issue #1151 · dotnet/command-line-api