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

Usage string broken for ArgGroup of option and argument #498

Closed
joshtriplett opened this issue May 7, 2016 · 6 comments
Closed

Usage string broken for ArgGroup of option and argument #498

joshtriplett opened this issue May 7, 2016 · 6 comments
Assignees
Labels
C-bug Category: Updating dependencies C-enhancement Category: Raise on the bar on expectations

Comments

@joshtriplett
Copy link
Contributor

Consider the following subcommand definition:

    SubCommand::with_name("set-base")
        .about("Set the base commit for the patch series")
        .arg(Arg::with_name("base").help("Base commit").required(false))
        .arg_from_usage("-d, --delete 'Remove the base commit information'")
        .group(ArgGroup::with_name("base_or_delete")
            .args(&["base", "delete"])
            .required(true)),

(The same issue applies if I use arg_from_usage for base.)

The usage string looks like this:

    set-base [FLAGS] [[base]|--delete] [ARGS]
...
    [base]    Base commit

Note the extra square brackets around base; that part should look like base|--delete.

Also, the alternative between base and --delete should probably have something other than square brackets around it, since one of those two is required.

@joshtriplett
Copy link
Contributor Author

joshtriplett commented May 7, 2016

An interesting further complication: if I add a .conflicts_with("delete") to base, and pass both, I get the following error:

error: The argument '--delete' cannot be used with '[base]'

USAGE:
    git series set-base [base] --delete [[base]|--delete] [[base]|--delete]

That seems like two too many of each argument.

@joshtriplett
Copy link
Contributor Author

joshtriplett commented May 7, 2016

Using .required_unless("delete") instead of .conflicts_with("delete") produces slightly better results, but still a bit wrong:

USAGE:
    set-base [FLAGS] <base> [<base>|--delete]

<base> shouldn't appear twice.

@kbknapp
Copy link
Member

kbknapp commented May 7, 2016

Thanks for taking the time to file these! I agree with all of the above, so it's actually like 3 bugs in 1, or a formating one for the first comment which is an easy fix that I agree with too.

I'll post back here as they're completed then put out 2.4.1.

@kbknapp kbknapp added C-bug Category: Updating dependencies C-enhancement Category: Raise on the bar on expectations P2: need to have labels May 9, 2016
@kbknapp
Copy link
Member

kbknapp commented May 9, 2016

Tracking for issues:

  • Duplicated args in usage string
  • formatting for required arg groups
  • formatting for positional arguments

@kbknapp kbknapp self-assigned this May 9, 2016
kbknapp added a commit that referenced this issue May 9, 2016
For example, if an arg is part of a required group, it will only appear
in the group usage string, and not in both the group as well as the arg
by itself.

Imagine a group containing two args, `arg1` and `--arg2`

OLD:
    `myprog <arg1> <arg1|--arg2>`

NEW:
    `myprog <arg1|--arg2>`

Closes #498
@kbknapp
Copy link
Member

kbknapp commented May 9, 2016

@joshtriplett All tasks done, this should be merged with #499 at which point I'll put out v2.4.1

kbknapp added a commit that referenced this issue May 9, 2016
For example, if an arg is part of a required group, it will only appear
in the group usage string, and not in both the group as well as the arg
by itself.

Imagine a group containing two args, `arg1` and `--arg2`

OLD:
    `myprog <arg1> <arg1|--arg2>`

NEW:
    `myprog <arg1|--arg2>`

Closes #498
kbknapp added a commit that referenced this issue May 9, 2016
For example, if an arg is part of a required group, it will only appear
in the group usage string, and not in both the group as well as the arg
by itself.

Imagine a group containing two args, `arg1` and `--arg2`

OLD:
    `myprog <arg1> <arg1|--arg2>`

NEW:
    `myprog <arg1|--arg2>`

Closes #498
@homu homu closed this as completed in 3ca0947 May 10, 2016
@kbknapp
Copy link
Member

kbknapp commented May 10, 2016

due to some crazy test issues and refactoring woes, the next version is actually v2.4.3 and is out on crates.io. This is the version that has the changes from this issue 😉 Sorry for the confusion!

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

No branches or pull requests

3 participants