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

Groups within subcommands not working as expected #151

Closed
j-browne opened this issue Nov 26, 2018 · 7 comments · Fixed by #290
Closed

Groups within subcommands not working as expected #151

j-browne opened this issue Nov 26, 2018 · 7 comments · Fixed by #290
Labels
bug This is a BUG. The fix may be released in a patch version even if considered breaking help wanted Your help is needed! Feel free to participate in the discussion or send a PR

Comments

@j-browne
Copy link

I am trying to get a program with subcommands, one of which requires at least one flag. So, for example, I would want the following behavior:

  • cargo run -- a fails
  • cargo run -- a -c succeeds
  • cargo run -- a -d succeeds
  • cargo run -- a -cd succeeds

I can get it working using a clap::ArgGroup without subcommands, but not with subcommands.
For example, given the following

use structopt::clap::ArgGroup;
use structopt::StructOpt;

fn arg_group() -> ArgGroup<'static> {
    ArgGroup::with_name("group").required(true).multiple(true)
}

#[derive(StructOpt, Debug)]
#[structopt(raw(group = "arg_group()"))]
struct Sub {
    #[structopt(short = "c", group = "group")]
    c: bool,
    #[structopt(short = "d", group = "group")]
    d: bool,
}

#[derive(StructOpt, Debug)]
enum Opt {
    #[structopt(name = "a", raw(group = "arg_group()"))]
    A (Sub),
    #[structopt(name = "b")]
    B {},
}

this works (though not as a subcommand)

fn main() {
    let opt = Sub::from_args();
    println!("{:?}", opt);
}

but this does not

fn main() {
    let opt = Opt::from_args();
    println!("{:?}", opt);
}

Instead, I get the following behavior:

  • cargo run -- a fails
  • cargo run -- a -c succeeds
  • cargo run -- a -d succeeds
  • cargo run -- a -cd fails

So I'm not sure if I'm doing something wrong or if perhaps its a bug in structopt.

I'm using structopt 0.2.13 and rust nightly (6acbb5b65 2018-11-25).

@TeXitoi
Copy link
Owner

TeXitoi commented Nov 27, 2018

I have to inverstigate a bit, but I think that's a clap bug. I'll try to reproduce it in pure clap when I have the time.

@TeXitoi TeXitoi added the bug This is a BUG. The fix may be released in a patch version even if considered breaking label Nov 27, 2018
@TeXitoi TeXitoi added the help wanted Your help is needed! Feel free to participate in the discussion or send a PR label Apr 9, 2019
@Veetaha
Copy link

Veetaha commented Nov 30, 2019

I do have a similar bug to report.
The following works as expected:

use structopt::{clap::ArgGroup, StructOpt};

#[derive(StructOpt, Debug)]
#[structopt(group = ArgGroup::with_name("verb").required(true).multiple(true))]
struct Opt {
    #[structopt(long, group = "verb")] foo: bool,   
    #[structopt(long, group = "verb")] bar: bool,
}
pub fn main() {
    Opt::from_args();
}

It reports an error when none of for or bar were passed.
But when I flatten it into other StructOpt it starts working weirdly:

#[derive(Debug, StructOpt)]
struct Cli {
    #[structopt(flatten)]
    a: Opt
}

pub fn main() {
    Cli::from_args();
}

It doesn't fail when none of for or bar were passed, but it does fail when both foo and barare passed (which is not what was expected, since by this code I intended to declare that my cli application accepts multiple arguments, but minimum one).

@CreepySkeleton
Copy link
Collaborator

I gave a bit of investigation and there's a working variant:

use structopt::{clap::ArgGroup, StructOpt};

#[derive(StructOpt, Debug)]
struct Opt {
    #[structopt(long, group = "verb")] foo: bool,
    #[structopt(long, group = "verb")] bar: bool,
}

#[derive(Debug, StructOpt)]
#[structopt(group = ArgGroup::with_name("verb").required(true).multiple(true))]
struct Cli {
    #[structopt(flatten)]
    a: Opt
}

pub fn main() {
    Cli::from_args();
}

I.e you must put #[structopt(group = ...] on the top level of your app. In the first example, top level was Opt, in the second it's Cli.

I'm not sure whether it's a bug or intentional behavior, I need to take a closer look at the machinery. I promise I'll get to it by the end of the week even if I'll have to fake my death to my boss (I know you're reading this, you bastard).

Also could someone kindly lend me a time machine?

@CreepySkeleton
Copy link
Collaborator

OK, it turns out that flatten does not carry App::* calls (#[structopt(...)] attributes on top of a struct/enum) over the macro-call boundary (remember, each #[derive(StructOpt)] is a separate macro call). The example in the first comment doesn't work for a similar reason.

We cannot make this work, I don't think so. It would require an access to internal clap's machinery and I really don't like the idea of using #[doc(hidden)] pub stuff.

I suggest to simply document this pitfall.

@Veetaha
Copy link

Veetaha commented Dec 1, 2019

@CreepySkeleton So the sole workaround is to move this invocation

#[structopt(group = ArgGroup::with_name("verb").required(true).multiple(true))]

to the struct arg group is flattened to, isn't it?

@CreepySkeleton
Copy link
Collaborator

@Veetaha Yes. More specifically, all "top level" #[structopt(method...)] attributes (i.e ones placed on top of a struct/enum) must be placed on top of the struct you call from_args with.

Good news: I managed to fix it! And the fix is ridiculously simple! Will be released in the next patch, few days from now.

@Veetaha
Copy link

Veetaha commented Dec 2, 2019

@CreepySkeleton Great news, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a BUG. The fix may be released in a patch version even if considered breaking help wanted Your help is needed! Feel free to participate in the discussion or send a PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants