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

requires_ifs does not appear to work #3059

Closed
2 tasks done
FrankC01 opened this issue Dec 4, 2021 · 9 comments
Closed
2 tasks done

requires_ifs does not appear to work #3059

FrankC01 opened this issue Dec 4, 2021 · 9 comments
Labels
C-bug Category: Updating dependencies

Comments

@FrankC01
Copy link

FrankC01 commented Dec 4, 2021

Please complete the following tasks

  • I have searched the discussions
  • I have searched the existing issues

Rust Version

rustc 1.55.0 (c8dfcfe04 2021-09-06)

Clap Version

clap = "2.33.3"

Minimal reproducible code

    #[test]
    fn test_output() {
        let res = App::new("prog")
            .arg(
                Arg::with_name("output")
                    .long("output")
                    .short("o")
                    .takes_value(true)
                    .possible_values(&["csv", "excel", "stdout"])
                    .default_value("stdout")
                    .help("Direct output to file"),
            )
            .arg(
                Arg::with_name("filename")
                    .long("filename")
                    .short("f")
                    .takes_value(true)
                    // .requires_if("excel", "output") 
                    // .requires_ifs(&[("output", "excel"), ("output", "csv")])
                    .requires_ifs(&[("excel", "output"), ("csv", "output")])
                    .help("Filename for '-o excel' or '-o csv' output"),
            )
            .get_matches_from_safe(vec!["prog", "-o", "excel"]);
        println!("{:?}", res);
        // assert!(res.is_err()); // We  used -o excel so -f <filename> is required
        // assert_eq!(res.unwrap_err().kind, ErrorKind::MissingRequiredArgument);
    }

Steps to reproduce the bug with the above code

cargo test

Actual Behaviour

No error occurred

Expected Behaviour

Expected error as the -f <val> is required if -o excel or -o csv is present

Additional Context

No response

Debug Output

No response

@FrankC01 FrankC01 added the C-bug Category: Updating dependencies label Dec 4, 2021
@FrankC01
Copy link
Author

FrankC01 commented Dec 4, 2021

This works if I change

.requires_ifs(&[("output", "excel"), ("output", "csv")])

to

.required_ifs(&[("output", "excel"), ("output", "csv")])

@epage
Copy link
Member

epage commented Dec 4, 2021

Yes

  • a "requires" relationship says filename cannot be present unless X conditions are met (prog -f file will fail without -o excel)
  • a "required" relationship says filename must be present if X conditions are met (prog -o excel will fail without -f file)

@epage
Copy link
Member

epage commented Dec 4, 2021

And I do feel like we could do a better job clarifying that distinction

@FrankC01
Copy link
Author

FrankC01 commented Dec 5, 2021

However, while required_ifs fails if -f <arg> is not supplied with -o <arg>, it does not fail if I do prog -f somefile

Basically I'm looking for a way to:

  1. If -f <arg> appears without -o <arg then fail
  2. if -o <arg> appears without -f <arg> then fail

Am I missing some magic combo?

@epage
Copy link
Member

epage commented Dec 6, 2021

Could you provide code with those cases so we know which APIs you are using in which ways to try to get the behavior you are wanting that isn't working as expected?

@FrankC01
Copy link
Author

FrankC01 commented Dec 6, 2021

Basically I want the two options to be required of each other -o <arg> -f<arg>
I will go through iterations of what I've tried and add them here

@FrankC01
Copy link
Author

FrankC01 commented Dec 8, 2021

@epage Here are the four combo's (2-required_ifs and 2-requires_ifs). I expect all to fail but only the first does:

    #[test]
    fn test_requiredifs_options_without_file_fail() {
        let res = App::new("prog")
            .arg(
                Arg::with_name("output")
                    .long("output")
                    .short("o")
                    .takes_value(true)
                    .possible_values(&["csv", "excel", "stdout"])
                    .default_value("stdout")
                    .help("Direct output to file"),
            )
            .arg(
                Arg::with_name("filename")
                    .long("filename")
                    .short("f")
                    .takes_value(true)
                    .required_ifs(&[("output", "excel"), ("output", "csv")])
                    .help("Filename for '-o excel' or '-o csv' output"),
            )
            .get_matches_from_safe(vec!["prog", "-o", "excel"]);
        assert!(res.is_err()); // We  used -o excel so -f <filename> is required
        assert_eq!(res.unwrap_err().kind, ErrorKind::MissingRequiredArgument);
    }
    #[test]
    fn test_requiredifs_options_without_output_should_fail() {
        let res = App::new("prog")
            .arg(
                Arg::with_name("output")
                    .long("output")
                    .short("o")
                    .takes_value(true)
                    .possible_values(&["csv", "excel", "stdout"])
                    .default_value("stdout")
                    .help("Direct output to file"),
            )
            .arg(
                Arg::with_name("filename")
                    .long("filename")
                    .short("f")
                    .takes_value(true)
                    .required_ifs(&[("output", "excel"), ("output", "csv")])
                    .help("Filename for '-o excel' or '-o csv' output"),
            )
            .get_matches_from_safe(vec!["prog", "-f", "filename"]);
        assert!(res.is_err()); // We  used -o excel so -f <filename> is required
        assert_eq!(res.unwrap_err().kind, ErrorKind::MissingRequiredArgument);
    }
    #[test]
    fn test_requiresif_options_without_file_should_fail() {
        let res = App::new("prog")
            .arg(
                Arg::with_name("output")
                    .long("output")
                    .short("o")
                    .takes_value(true)
                    .possible_values(&["csv", "excel", "stdout"])
                    .default_value("stdout")
                    .help("Direct output to file"),
            )
            .arg(
                Arg::with_name("filename")
                    .long("filename")
                    .short("f")
                    .takes_value(true)
                    .requires_ifs(&[("output", "excel"), ("output", "csv")])
                    .help("Filename for '-o excel' or '-o csv' output"),
            )
            .get_matches_from_safe(vec!["prog", "-o", "excel"]);
        assert!(res.is_err()); // We  used -o excel so -f <filename> is required
        assert_eq!(res.unwrap_err().kind, ErrorKind::MissingRequiredArgument);
    }
    #[test]
    fn test_requiresif_options_without_output_should_fail() {
        let res = App::new("prog")
            .arg(
                Arg::with_name("output")
                    .long("output")
                    .short("o")
                    .takes_value(true)
                    .possible_values(&["csv", "excel", "stdout"])
                    .default_value("stdout")
                    .help("Direct output to file"),
            )
            .arg(
                Arg::with_name("filename")
                    .long("filename")
                    .short("f")
                    .takes_value(true)
                    .requires_ifs(&[("output", "excel"), ("output", "csv")])
                    .help("Filename for '-o excel' or '-o csv' output"),
            )
            .get_matches_from_safe(vec!["prog", "-f", "somefile"]);
        assert!(res.is_err()); // We  used -o excel so -f <filename> is required
        assert_eq!(res.unwrap_err().kind, ErrorKind::MissingRequiredArgument);
    }

@epage
Copy link
Member

epage commented Dec 10, 2021

What you are looking for is something like

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

fn main() {}

#[test]
fn test_requiresif_options_without_output_should_fail() {
    let mut app = App::new("prog")
        .arg(
            Arg::new("output")
                .long("output")
                .short('o')
                .takes_value(true)
                .possible_values(&["csv", "excel", "stdout"])
                .requires_ifs(&[("excel", "filename"), ("csv", "filename")])
                .help("Direct output to file"),
        )
        .arg(
            Arg::new("filename")
                .long("filename")
                .short('f')
                .takes_value(true)
                .requires("output")
                .help("Filename for '-o excel' or '-o csv' output"),
        );

    let res = app.try_get_matches_from_mut(vec!["prog", "-f", "somefile"]);
    assert!(res.is_err()); // We  used -o excel so -f <filename> is required
    assert_eq!(res.unwrap_err().kind, ErrorKind::MissingRequiredArgument);

    let res = app.try_get_matches_from_mut(vec!["prog", "-o", "excel"]);
    assert!(res.is_err()); // We  used -o excel so -f <filename> is required
    assert_eq!(res.unwrap_err().kind, ErrorKind::MissingRequiredArgument);

    let res = app.try_get_matches_from_mut(vec!["prog", "-o", "stdout"]);
    assert!(res.is_ok(), "{:?}", res);

    let res = app.try_get_matches_from_mut(vec!["prog", "-o", "excel", "-f", "file.xls"]);
    assert!(res.is_ok(), "{:?}", res);
}

This is written with clap3. I've not checked to see if there are any gotchas backporting to clap2.

Even with clap3, I had to remove the default_value. That can be added back in when we fix #3076

@FrankC01
Copy link
Author

Ok, so I had it bass-ackwards to begin with. I can work with your solution if you want to close this

And thanks for your help!

@epage epage closed this as completed Dec 10, 2021
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