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

clap-rs 0.7.1: dynamic usage strings seem 'inverted' #96

Closed
Byron opened this issue May 2, 2015 · 4 comments
Closed

clap-rs 0.7.1: dynamic usage strings seem 'inverted' #96

Byron opened this issue May 2, 2015 · 4 comments
Labels
C-bug Category: Updating dependencies

Comments

@Byron
Copy link
Contributor

Byron commented May 2, 2015

With a program exhibiting the following grammar ...

$ groupsmigration1 archive insert --help`
groupsmigration1-archive-insert
Inserts a new mail into the archive of the Google group.

USAGE:
    groupsmigration1 archive insert <group-id>  [FLAGS] [OPTIONS] [ -u <mode> <file> ]

FLAGS:
    -h, --help       Prints help information
    -v, --version    Prints version information

OPTIONS:
    -m <mime>          The file's mime time, like 'application/octet-stream'
    -u <mode> <file>        Specify the upload protocol (simple|resumable) and the file to upload
    -o <out>           Specify the file into which to write the programs output
    -p <v>...          Set various fields of the request structure

POSITIONAL ARGUMENTS:
    group-id    The group ID

... I would expect a partial invocation like groupsmigration1 archive insert to yield a usage more along the lines of ...

One or more required arguments were not supplied: <group-id>, '-u <mode> <file>'
USAGE:
    groupsmigration1 archive insert <group-id> -u <mode> <file>
For more information try --help

... but what you get is just as follows:

One or more required arguments were not supplied
USAGE:
    groupsmigration1 archive insert
For more information try --help

Another example for missing required parameters is groupsmigration1 archive insert -u simple README.md where <group-id> is missing. However, this is not actually said in the resulting usage text:

$ groupsmigration1 archive insert -u simple README.md
One or more required arguments were not supplied
USAGE:
    groupsmigration1 archive insert    [ -u <mode> <file> ]
For more information try --help

What I would expect here is something like this:

One or more required arguments were not supplied: <group-id>
USAGE:
    groupsmigration1 archive insert <group-id> -u simple README.md
For more information try --help

In a call like groupsmigration1 archive insert group one will see a usage string like this:

One or more required arguments were not supplied
USAGE:
    groupsmigration1 archive insert  <group-id>
For more information try --help

and I think think it would be much better to get something like that:

One or more required arguments were not supplied: -u <mode> <file>
USAGE:
    groupsmigration1 archive insert group -u <mode> <file>
For more information try --help

Once the usage is improved, clap will finally be top-of-the-line when it comes to usability I think, and I really want it to be there :).
Thanks for taking care.

@kbknapp
Copy link
Member

kbknapp commented May 2, 2015

Thanks for the detailed report! I agree, as I was thinking about this last night, it should state which arguments were missing such as your last:

One or more required arguments were not supplied: -u <mode> <file>
USAGE:
    groupsmigration1 archive insert group -u <mode> <file>
For more information try --help

This should be a decently simple change. I'll start working on this today.

Also, as for why <group-id> is getting dropped from the requirements list, this could be a bug. Could you show me where the argument definitions for the insert subcommand are located?

One last kind of side note, the [ and ] around the -u <mode> <file> I'm on the fence about if I should take them out or not (for the usage string only, not for the new "missing required" error message). I was originally worried that if I user saw:

groupsmigration1 archive insert <group-id> -u <mode> <file>

They may mistakenly think <file> is not part of -u until they see the full help message. If a program had more than one positional argument, and some of which were optional, they could mistakenly do something like

groupsmigration1 archive insert <group-id> <file> -u <mode> <some_other_positional>

I'll post back here once I've got fixes!

@kbknapp kbknapp added the C-bug Category: Updating dependencies label May 2, 2015
@kbknapp
Copy link
Member

kbknapp commented May 2, 2015

I was able to reproduce the bug with this snippet, but if you have a link to your source I'd still like to see that to see how exactly your requirements are set up so I don't fix the bug in my test case, but not in your real case.

let u_names = ["mode", "file"];
let matches = App::new("MyApp")
                    .arg(Arg::from_usage("-f [f]... 'some value'")
                              .value_names(&u_names)
                              .required(true))
                    .arg(Arg::from_usage("<gid> 'other value'"))
                    .get_matches();

Which produces grammarr:

fake 

USAGE:
    fake <gid>  [FLAGS] [ -f <mode> <file> ] 

FLAGS:
    -h, --help       Prints help information
    -v, --version    Prints version information

OPTIONS:
    -f <mode> <file>        some value

POSITIONAL ARGUMENTS:
    gid    other value

And when run with incomplete $ fake:

One or more required arguments were not supplied
USAGE:
    fake    
For more information try --help

@kbknapp kbknapp closed this as completed in 12aea96 May 3, 2015
kbknapp added a commit that referenced this issue May 3, 2015
Bug fixes, performance changes, and some clean up

Fixes #96
@kbknapp
Copy link
Member

kbknapp commented May 3, 2015

Take a look at the latest version (0.7.2 on crates.io) or master here and let me know if that works for you. It now tells you exactly which arguments you're missing (if any), along some visual clean up (spaces between arguments). I still need to fix the tab alignment in the help info, but I may wait on that as it's minor right now.

Also, the requirements all should be working, i.e. you can make an argument required by default, and if it requires other arguments, those arguments will be listed in the default usage string as well...kind of "required by proxy"

One other visual thing I changed was the [ and ] around arguments with named values, those have been removed because with the new requirements error messages it's quite explicit about what belongs with what. And the square brackets make an argument look "optional"

@Byron
Copy link
Contributor Author

Byron commented May 5, 2015

I finally got around to testing the modifications (with v0.7.5 actually), and must say that the help and usage strings now seems perfect. I'd have nothing to add here, except for saying thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

2 participants