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

Feature: App::arg_add_or_modify method #1730

Closed
wants to merge 1 commit into from
Closed

Feature: App::arg_add_or_modify method #1730

wants to merge 1 commit into from

Conversation

jhwgh1968
Copy link

I was having this issue in Clap 2.x, and am hoping for a backport. My apologies if 3.x has solved this problem already in a different way.

To quote from the documentation:

Suppose an application was allowed to pass arbitrary arguments to an external program. This would require enabling the AllowHyphenValues argument setting.

However, if the App was generated by another crate or by structopt, the setting could not be set on an argument that already existed. This function facilitates that use case:

let m = generate_app() // this was done automatically, so args could not set settings.
    .arg_add_or_modify("command-args", |arg| arg.setting(ArgSettings::AllowHyphenValues));
let n = m.clone();

// Rather than setting a global AllowHyphenValues, it is only on one argument.
assert!(m.try_get_matches_from(
    &["app", "--input", "in.txt", "--ext-arg", "-x", "--ext-arg", "out.txt"]).is_ok());

// This ensures putting additional arguments into a different value will be
// caught.
assert!(n.try_get_matches_from(
    &["app", "--input", "-i"]).is_err());

@pksunkara
Copy link
Member

Hmm.. I am not convinced this should be in the library. Or maybe this is not how it should be implemented. I understand the use case, it's just I am trying to think of a better way to do this.

@jhwgh1968
Copy link
Author

This was definitely a patch of frustration rather than feature design. 😄

If you have a better design or alternative way to achieve this -- specifically, when the App is generated by structopt -- then I am happy to accomodate.

@CreepySkeleton
Copy link
Contributor

@jhwgh1968 why can't you just set in in structopt? Or do you mean the app is generated by third party and you simply can't change the structopt's input?

@jhwgh1968
Copy link
Author

I will set up a small reproduction repo when I am back at the computer I originally posted from. In the mean time, here is the basic problem from memory.

The goal is to allow this syntax: cargo run -- -d a.txt b.txt --extern-flags -d -e -f
Which would cause the Rust program to execute this external command: cmd -d -e -f a.txt b.txt

The only way I could do that was a hack, shown below:

use structopt::StructOpt; // version 0.2
use clap::AppSettings; // version 2.x

#[derive(Debug, StructOpt)]
#[structopt(name = "example", about = "An example of multiple multi-value lists, which need different options.")]
struct Opt {
    /// Activate debug mode
    #[structopt(short, long)]
    debug: bool,

    /// The list of files to pass to the external command. Must not start with a dash.
    files: Vec<String>,

    /// The list of argument flags to pass to the external command. The position of these flags has an effect,
    /// So they should all be at the start.
    #[structopt(long = "extern-flags")]
    flags: Vec<String>
}

fn main() {
    // HACK: allow globally accepting leading dashes, even though we really only want it for one field. I wish we could set per-field options...
    let m = Opt::from_clap(Opt::clap().global_setting(clap::AppSettings::AllowHyphens)).get_matches();
    let (file_names, flags) = { /* decode everything here, and verify the files exist */ }
    std::process::Command::new("cmd").args(flags).args(file_names);
    // handle the child process
}

However, the App Setting is global, so the above also allows this: cargo run -- -d a.txt -e b.txt --extern-flags -d -f

That should be a syntax error, because the external command is sensitive to the position of its flags. Instead, the Rust code goes ahead with this undesired call: cmd -d -f a.txt -e b.txt

And the external command handles a.txt and b.txt differently.

I could imagine two solutions to this problem:

  1. Allow applying ArgSettings on the auto-generated structopt arguments in the procedural macro definition. I couldn't find a way to do this.
  2. Allow modification of existing arguments in an App structure without replacing them. This is what my patch does, and provides example code to solve my problem above.

Hopefully that explain it, @CreepySkeleton. If I missed something, or this is solved in Clap 3.x another way, let me know.

@CreepySkeleton
Copy link
Contributor

Allow applying ArgSettings on the auto-generated structopt arguments in the procedural macro definition. I couldn't find a way to do this.

(That feeling when you've been asked the question so many times you even can't remember how many times it's been, probably a billion. In fact, so many that you're starting to suspect that, when you die and end up in Hell, the first thing Satan will be going to tell you will not be - "Abandon hope all ye who enter here", no; the first unspeakable thing you'll hear will be - "Hey you, poor little soul! How do I call %some method from clap% with structopt?". Sound familiar?)

You can do it via raw methods:

#[derive(Debug, StructOpt)]
#[structopt(name = "example", about = "An example of multiple multi-value lists, which need different options.")]
struct Opt {
    /// Activate debug mode
    #[structopt(short, long)]
    debug: bool,

    /// The list of files to pass to the external command. Must not start with a dash.
    files: Vec<String>,

    /// The list of argument flags to pass to the external command. The position of these flags has an effect,
    /// So they should all be at the start.
    #[structopt(
        long = "extern-flags", 
        setting = clap::AppSettings::AllowHyphens // <== HERE
    )]
    flags: Vec<String>
}

That will do exactly what you want, but I don't recommend it.

The standard for "external flags" is to use the -- delimiter, just like cargo does:

cargo test --release -- --nocapture
      ^-------------    ^----------
       normal args    external args

clap supports this via Arg::raw method:

use structopt::StructOpt;

#[derive(Debug, StructOpt)]
#[structopt(name = "example", about = "An example of multiple multi-value lists, which need different options.")]
struct Opt {
    /// Activate debug mode
    #[structopt(short, long)]
    debug: bool,

    /// The list of files to pass to the external command. Must not start with a dash.
    files: Vec<String>,

    /// The list of argument flags to pass to the external command. The position of these flags has an effect,
    /// So they should all be at the start.
    #[structopt(raw = true)]
    flags: Vec<String>
}

fn main() {
    let res = Opt::from_args();
    println!("{:#?}", res);
}
$ cargo run -- -d a.txt b.txt -- -d -f
    Finished dev [unoptimized + debuginfo] target(s) in 0.17s
     Running `target\debug\probe.exe -d a.txt b.txt -- -d -f`
Opt {
    debug: true,
    files: [
        "a.txt",
        "b.txt",
    ],
    flags: [
        "-d",
        "-f",
    ],
}
$ cargo run -- --help
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `target\debug\probe.exe --help`
example 0.2.0
An example of multiple multi-value lists, which need different options.

USAGE:
    probe.exe [FLAGS] [files]... [-- <flags>...]

FLAGS:
    -d, --debug      Activate debug mode
    -h, --help       Prints help information
    -V, --version    Prints version information

ARGS:
    <files>...    The list of files to pass to the external command. Must not start with a dash
    <flags>...    The list of argument flags to pass to the external command. The position of these flags has an
                  effect, So they should all be at the start

Neat, isn't it?

@jhwgh1968
Copy link
Author

Sorry to make you feel like that, @CreepySkeleton!

I tried using -- at some point, but ended up manually parsing it out of the final arguments instead with the allow hyphen option -- not the best strategy, I agree.

I missed raw, though. After reading your explanation, I see it is documented... but it doesn't seem very obvious. The 0.2 docs have it as a foot note. The 0.3 docs are better, but I feel like the words didn't "click" that it was what I needed.

Based on your explanation, I will close this PR -- and perhaps later, write a PR with documentation improvements instead.

Your patience is appreciated -- and I have my own question like that the Devil would ask in my own technical work, so if it comes to that, I'll join you and we can have a party. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants