dotnet / command-line-api

Command line parsing, invocation, and rendering of terminal output.
https://github.com/dotnet/command-line-api/wiki
MIT License
3.38k stars 380 forks source link

Allow a command to accept multiple distinct arguments #310

Closed tmds closed 5 years ago

tmds commented 5 years ago

Some commands take multiple distinct arguments:

git remote add [<options>] <name> <url>
ln <target> <link_name>

This is typically used for required parameters. The user doesn't have to specify the name when providing these values.

System.CommandLine currently only supports a single argument. This argument may be of arity 1+. The above UX is not possible.

That single argument is made available as a string[]. This is different from Options, where options are bound based on their name to handler argument parameters (related https://github.com/dotnet/System.CommandLine/issues/297). This makes it much less convenient to consume these arguments, than it is to consume Options.

Suggested feature:

jonsequitur commented 5 years ago

This also relates to the positional options feature (#173, #28), which can be enabled (but is not currently enabled by default) using EnablePositionalOptions. Positional options could be used to provide this syntax, modeling name and url as different options each with an arity of 1 and having different argument types if appropriate.

tmds commented 5 years ago

Consider Arguments to be positional Options.

git remote add [<options>] <name> <url>
               -----------
               ^- random order (Options)
                           ------------
                           ^- fixed order (Arguments)

Positional matching interferes with completions

Maybe these things help:

Assumptions:

Completion:

@jonsequitur would that help? Or can you give some examples of where things go bad?

jonsequitur commented 5 years ago

Only suggest an option name when the user has typed the first character of an option name (e.g. -). So as long as this character isn't typed, the suggestions are subcommand names and the next argument value suggestions.

The reason we do completion the way we do (substring match, with candidates a mix of commands, options, or argument values) is a deliberate choice to improve discoverability for the end user, rather than making them think "I need an option" or "I need a command".

The assumptions look intuitively correct. I can't recall if the implementation matches them.

tmds commented 5 years ago

The reason we do completion the way we do (substring match, with candidates a mix of commands, options, or argument values) is a deliberate choice to improve discoverability for the end user

Trying some things on the command line, show me tools also do the opposite. When starting with a - they no longer consider argument values as suggestions. Only when adding a -- on it's own, words that start with a - are treated as arguments.

jonsequitur commented 5 years ago

I believe that's a bug with the implementation, not intentional. Does this look correct? (Related: #10)

fruit ⇥ should show both --apple and apple. fruit a⇥ should show both --apple and apple. fruit -⇥ should show only --apple.

tmds commented 5 years ago

Trying some things on the command line

I meant that other tools have this behavior on splitting between suggesting option names/argument values depending on whether the first char is a - and whether they are past a -- (double dash on it's own).

So it may limit discoverability, but may improve usability.

jonsequitur commented 5 years ago

Understood. The reasoning for this break from convention follows changes that have occurred in Intellisense / autocomplete in editors over the years. In the last few years, substring matching has generally replaced "starts-with" matching.

tmds commented 5 years ago

I'm not sure if the same is true on the command line.

If desired, substring matching is still possible, after using the first character to distinguish between option names and other.

jonsequitur commented 5 years ago

Keep in mind that we are not enforcing POSIX syntax. Cross-platform, the option prefix could be variously - (POSIX convention) or / (Windows convention). Additionally, options are in no way constrained to begin with a prefix, and commands are allowed to have prefixes.

When interacting with a new tool, the end user may be unaware of these specifics. They may also be unaware of the semantic distinctions between commands, options, and arguments. This is why we match more broadly to improve discoverability and learnability.

All of that said, I don't think either opinion will be proved "correct" without a usability study. We may find that these fall into an end-user preferences configuration, since they are design decisions geared toward the end user, not the app developer.

tmds commented 5 years ago

To solve the completion issue for arguments(/positional options) we need to change something about completions.

jonsequitur commented 5 years ago

Possibly. Validation messages were also misleading with positional options enabled. I suspect both of these can be fixed by making the core parser a bit smarter. Specific test cases that demonstrate behaviors we'd like to change would be a great first step toward making this work.

tmds commented 5 years ago

I've updated the top comment to be more clear what the requested feature is and to include some of the things that were discussed so far.

tmds commented 5 years ago

@KathleenDollard this is a feature request to extend argument support.

jnm2 commented 5 years ago

Would there be such a thing as commands having required and optional arguments?

This is what it looks like today:

Usage:
  Foo upgrade [options]

Options:
  --instance    The SQL Server instance containing the database to upgrade.
  --database    The name of the SQL Server database to upgrade.
  --login       A SQL Server login with owner access of the database to upgrade. If none is specified, Windows authentication will be used.
  --dry-run     This is the only thing I'd actually want in the Options section.

With required and optional arguments, could it be something like this?

Usage:
  Foo upgrade <instance> <database> [ <login> ] [options]

Arguments:
  <instance>    The SQL Server instance containing the database to upgrade.
  <database>    The name of the SQL Server database to upgrade.
  <login>       A SQL Server login with owner access of the database to upgrade. If none is specified, Windows authentication will be used.

Options:
  --dry-run     This is the only thing I'd actually want in the Options section.

Syntax that would be nice to use:

new Command(...)
{
    new Argument("instance", "The SQL Server instance containing the database to upgrade."),
    new Argument("database", "The name of the SQL Server database to upgrade."),
    new Argument("login", "A SQL Server login with owner access of the database to upgrade. " +
        "If none is specified, Windows authentication will be used.", defaultValue: null),
    new Option("--dry-run", "This is the only thing I'd actually want in the Options section.", new Argument<bool>())
}

The login argument would be denoted as optional ([ <login> ]) because the C# specifies a default value for that argument.

jonsequitur commented 5 years ago

Yes, commands and options can both currently specify how many arguments are required, via argument arity, and a default value satisfies the requirement if the user doesn't specify an argument. (Help output doesn't reflect this currently.) I would expect this behavior to extend to this scenario.

One consideration for the API design is we don't vary behavior based on the order in which child objects are added to their parents. Since the parsing of these arguments will necessarily be positional, the API will need to be explicit about their position.

KathleenDollard commented 5 years ago

@jnm2

It seems to me that the difference between options and arguments is whether the argument name (--database) is included. I assume in the example you posted, you'd have an opinion on which is better for end users. When there are multiple arguments, they need to be positional.

I think this would be a valuable addition.

And when @jonsequitur is back, I think we need a separate conversation about where/how required values are supported.

jnm2 commented 5 years ago

@KathleenDollard Yes, that is exactly how I was thinking of it. Thanks!

jonsequitur commented 5 years ago

@jnm2 Your API suggestion was a good one. Thoughts? https://github.com/dotnet/command-line-api/pull/542/files#diff-fb1621b467c13fb6006c42c063e47bc2.

jonsequitur commented 5 years ago

I've opened a separate issue to cover related help improvements: #543

jnm2 commented 5 years ago

@jonsequitur Wow... that's a lot to get my head around, but I'm glad my suggestion was useful and I'll look forward to future versions!

gitfool commented 5 years ago

@jonsequitur Awesome! I’m very interested in these changes but reluctant to switch to nightly builds. Will there be another prerelease soonish? (I only ask because the last alpha was a couple of months ago.)

jonsequitur commented 5 years ago

There will be, and there are a few minor breaking changes which I'd like to document before publishing new packages on nuget.org.