Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't collapse optional positional arguments into [ARGS] in the usage string #769

Closed
dotdash opened this issue Dec 7, 2016 · 5 comments · Fixed by #4151
Closed

Don't collapse optional positional arguments into [ARGS] in the usage string #769

dotdash opened this issue Dec 7, 2016 · 5 comments · Fixed by #4151
Assignees
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc...
Milestone

Comments

@dotdash
Copy link
Contributor

dotdash commented Dec 7, 2016

Rust Version

rustc 1.15.0-nightly (daf8c1dfc 2016-12-05)

Affected Version of clap

2.19.1

Expected Behavior Summary

If I have one required and two optional positional arguments, I expect the usage string to say:

USAGE:
    clap-test <FILE> [<WIDTH> [<HEIGHT>]]

to properly represent the order in which the arguments need to be given.

Actual Behavior Summary

The optional positional arguments are collapsed into a single [ARGS] placeholder, and I have to look at the ARGS block and know it tells me the required order.

USAGE:
    clap-test <FILE> [ARGS]

FLAGS:
        --help       Prints help information
    -V, --version    Prints version information

ARGS:
    <FILE>      the configuration file
    <WIDTH>     texture width [default: 512]
    <HEIGHT>    texture height [default: 512]

Steps to Reproduce the issue

Run the program with --help.

Sample Code or Link to Sample Code

extern crate clap;
use clap::{Arg, App};

fn main() {
    let matches = App::new("My  app")
        .version("0.1")
        .author("Me <me@ma.il>")
        .about("Does stuff")
        .arg(Arg::with_name("config")
             .value_name("FILE")
             .help("the configuration file")
             .required(true)
             .takes_value(true)
             .index(1))
        .arg(Arg::with_name("width")
             .value_name("WIDTH")
             .help("texture width")
             .takes_value(true)
             .default_value("512")
             .index(2))
        .arg(Arg::with_name("height")
             .value_name("HEIGHT")
             .help("texture height")
             .takes_value(true)
             .default_value("512")
             .index(3))
        .get_matches();
}
@kbknapp
Copy link
Member

kbknapp commented Dec 7, 2016

I do agree, but at the same time I worry about applications that have lots of positional arguments. I'd like to get some others opinions on this issue before merging.

One point I'd like to change from your proposal though is the $ prog <required> [<optional1> <optional2>]

I'd prefer that go to $ prog <required> [optional1] [optional2]. This is personal preference but I dislike the CLIs that give me a usage strings like $ prog <opt> [[<req>|[-e <some>]] | [other]] It gests difficult to parse (mentally) very quickly.

@dotdash
Copy link
Contributor Author

dotdash commented Dec 8, 2016

Oh, I meant to mention that I'd be fine with not nesting the square brackets. Not sure what the best option is for handling lots of optional positional arguments, but the same problem already exists for required positional arguments, so a common solution would be nice.

@kbknapp
Copy link
Member

kbknapp commented Dec 8, 2016

So we already don't collapse them down if it's only 2 positional arguments (I believe...I can't remember 😜 ). So it's entirely possible that we only collapse them after only after a [higher] certain number (perhaps up the number to...5?).

@dotdash
Copy link
Contributor Author

dotdash commented Dec 9, 2016

They're collapsed to [ARGS] if there are two or more. I guess I could live with collapsing only if there are more than X items, or even just an option that allows me to just disable it altogether.

@kbknapp
Copy link
Member

kbknapp commented Dec 9, 2016

Adding an option to just disable the collapsing is easy, and I think the route I'll go since that won't mess with current usage strings (i.e. it's an opt-in feature).

@kbknapp kbknapp added this to the 2.20.0 milestone Dec 10, 2016
@kbknapp kbknapp added A-help Area: documentation, including docs.rs, readme, examples, etc... D: easy C-enhancement Category: Raise on the bar on expectations and removed T: RFC / question labels Dec 10, 2016
@kbknapp kbknapp added T: new setting and removed C-enhancement Category: Raise on the bar on expectations labels Dec 31, 2016
@kbknapp kbknapp self-assigned this Dec 31, 2016
@homu homu closed this as completed in c2978af Dec 31, 2016
epage added a commit to epage/clap that referenced this issue Aug 30, 2022
The setting was added to resolve clap-rs#769.  The reason it was optional is out
of concern for applications with a lot of positional arguments.  I think
those cases are rare enough that we should just push people to override
the usage.  Positional arguments are generally important enough, even if
optional, to show.

As a side effect, this fixed some bugs with
`dont_collapse_args_in_usage` where it would repeat an argument in a
smart usage.

As a side effect, smart usage now shows `--` when it should
epage added a commit to epage/clap that referenced this issue Aug 30, 2022
The setting was added to resolve clap-rs#769.  The reason it was optional is out
of concern for applications with a lot of positional arguments.  I think
those cases are rare enough that we should just push people to override
the usage.  Positional arguments are generally important enough, even if
optional, to show.

As a side effect, this fixed some bugs with
`dont_collapse_args_in_usage` where it would repeat an argument in a
smart usage.

As a side effect, smart usage now shows `--` when it should
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc...
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants