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

Allow positional arguments from ArgGroup take value separately #1761

Closed
EverlastingBugstopper opened this issue Mar 25, 2020 · 8 comments
Closed
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-enhancement Category: Raise on the bar on expectations S-wont-fix Status: Closed as there is no plan to fix this

Comments

@EverlastingBugstopper
Copy link

EverlastingBugstopper commented Mar 25, 2020

What are you trying to do?

I'm trying to create positional command line arguments that are exclusive, you can either set one or the other. I'd like to be able to use clap_derive with this so that they are boiled down to a single expiration: std::time::Duration field in an opt struct.

set expiration to 60 seconds

$ app set key value EX 60

set expiration to 6000 milliseconds

$ app set key value PX 6000

What have you tried already?

I haven't been sure what to try or if this is even supported :)

@pksunkara
Copy link
Member

You can use conflicts function.

@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Mar 26, 2020

I'm under impression that conflicts is not what he was asking about.

This is pretty interesting question. What he's asking for (or so I get it) is:

  • Allow one of the given set of options: PX, EX. Only one of them can be present at a time.

    These two conditions can be implemented via ArgGroup.

  • Each of them takes value. This cannot be implemented via ArgGroup.

    use clap::{App, ArgGroup, Arg};
    
    fn main() {
        let m = App::new("app")
            .arg(Arg::with_name("PX").takes_value(true))
            .arg(Arg::with_name("EX").takes_value(true))
            .group(ArgGroup::with_name("expiration").args(&["PX", "EX"]))
            .get_matches();
    
        println!("{:?}", m.value_of("EX"));
    }
    $ cargo run -- EX 666
       Compiling probe v0.2.0 (D:\workspace\probe)
        Finished dev [unoptimized + debuginfo] target(s) in 10.46s
         Running `target\debug\probe.exe EX 666`
    error: The argument '<EX>' cannot be used with one or more of the other specified arguments
    
    USAGE:
        probe.exe <PX|EX>
    
    For more information try --help
    error: process didn't exit successfully: `target\debug\probe.exe EX 666` (exit code: 1)
    

    This could be implemented if EX and PX were non-positional options:

    use clap::{App, ArgGroup, Arg};
    
    fn main() {
        let m = App::new("app")
            .arg(Arg::with_name("PX").takes_value(true).long("PX"))
            .arg(Arg::with_name("EX").takes_value(true).long("EX"))
            .group(ArgGroup::with_name("expiration").args(&["PX", "EX"]))
            .get_matches();
    
        println!("{:?}", m.value_of("EX"));
    }
    $ cargo run -- --EX 600
        Finished dev [unoptimized + debuginfo] target(s) in 0.10s
         Running `target\debug\probe.exe --EX 600`
    Some("600")
    

But non-positional options must start with -.

I'll take liberty to rename this issue, transforming it into feature request.

Until the, @EverlastingBugstopper , you can make use of subcommands:

use clap::{App, Arg, SubCommand};

fn main() {
    let m = App::new("app")
        .subcommand(SubCommand::with_name("PX")
          .arg(Arg::with_name("value").takes_value(true).required(true)))
        .subcommand(SubCommand::with_name("EX")
          .arg(Arg::with_name("value").takes_value(true).required(true)))
        .get_matches();

    println!("{:#?}", m);
}
$ cargo run -- EX 600
   Compiling probe v0.2.0 (D:\workspace\probe)
    Finished dev [unoptimized + debuginfo] target(s) in 1.77s
     Running `target\debug\probe.exe EX 600`
ArgMatches {
    args: {},
    subcommand: Some(
        SubCommand {
            name: "EX",
            matches: ArgMatches {
                args: {
                    "value": MatchedArg {
                        occurs: 1,
                        indices: [
                            1,
                        ],
                        vals: [
                            "600",
                        ],
                    },
                },
                subcommand: None,
                usage: Some(
                    "USAGE:\n    probe.exe EX <value>",
                ),
            },
        },
    ),
    usage: Some(
        "USAGE:\n    probe.exe [SUBCOMMAND]",
    ),
}

The only thing is that the error message might not appear what you want:

$ cargo run -- --help
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
     Running `target\debug\probe.exe --help`
app

USAGE:
    probe.exe [SUBCOMMAND]

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

SUBCOMMANDS:
    EX
    PX
    help    Prints this message or the help of the given subcommand(s)

@CreepySkeleton CreepySkeleton changed the title Exclusive or argument? usage: [expiration EX seconds|PX milliseconds] Allow positional arguments from ArgGroup take value separately Mar 26, 2020
@CreepySkeleton CreepySkeleton added this to the 3.1 milestone Mar 26, 2020
@EverlastingBugstopper
Copy link
Author

Hey @CreepySkeleton - thanks for the thoughtful response!

When y'all implement new features like this is it automatically provided in clap_derive or does that require extra work as well? My mental model of how clap_derive actually works is a bit shaky, it seems like most things are supported but the patterns aren't always obvious to me (how would I go about creating an ArgGroup in clap_derive?

@CreepySkeleton
Copy link
Contributor

(how would I go about creating an ArgGroup in clap_derive?

Example

When y'all implement new features like this is it automatically provided in clap_derive or does that require extra work as well?

Most things are immediately available through #[clap(method = value)] attributes. Some things, like subcommands, require dedicated features.

I am inclined to remind you that clap_derive is still in alpha stage and API may change. You can play with it, but expect sudden breakages. For production, use structopt.

@EverlastingBugstopper
Copy link
Author

Awesome! Yeah - this is something I'm working on the side, not released or intended to be released soon so it's ok that it's still in alpha. Thanks for the help :)

@pksunkara pksunkara removed the W: 3.x label Aug 13, 2021
@epage epage added C-enhancement Category: Raise on the bar on expectations S-triage Status: New; needs maintainer attention. and removed T: new feature labels Dec 8, 2021
@epage epage removed this from the 3.1 milestone Dec 13, 2021
@epage epage added the A-parsing Area: Parser's logic and needs it changed somehow. label Dec 13, 2021
@epage
Copy link
Member

epage commented Nov 8, 2022

#1334 would be one way to improve the subcommand help output.

@EverlastingBugstopper The thing that isn't too clear to me is what in the example commands are meant to be literals vs place holders.

$ app set key value EX 60

One workaround discussed earlier was subcommands which means EX and PX are literals. A proposed way of this working was using a EX and PX positional argument but that would have different behavior unless we also limited them with possible values to being just the literals EX and PX. At that point, someone can define a single positional argument with EX and PX possible values. The value name could be set to EX|PX to make it display nicely.

If this is about skipping them based on values, this reminds me of

@EverlastingBugstopper
Copy link
Author

EverlastingBugstopper commented Nov 8, 2022

I opened this issue a while back when I was working on mini-redis. Upon looking at this again a few years later, I don't think I would ever want to design a CLI with usage like this 😆

It looks like when they implemented support for setting expiration that it's just a number and there's no way to specify a difference between seconds and milliseconds. (I'd probably implement this today using something like humantime to parse the length of time rather than archaic EX/PX literals that the actual redis implementation uses)

@epage epage added S-wont-fix Status: Closed as there is no plan to fix this and removed S-triage Status: New; needs maintainer attention. labels Nov 8, 2022
@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Nov 8, 2022
@epage
Copy link
Member

epage commented Nov 8, 2022

Thanks for the reply!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-enhancement Category: Raise on the bar on expectations S-wont-fix Status: Closed as there is no plan to fix this
Projects
None yet
Development

No branches or pull requests

4 participants