From f082eb6d4aded355ac70566a252a9e97a653292e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 27 Jun 2022 22:26:26 -0500 Subject: [PATCH 1/2] test(parser): Verify global/default interaction --- tests/builder/global_args.rs | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/builder/global_args.rs b/tests/builder/global_args.rs index 2e233b0d457..f3419426bb5 100644 --- a/tests/builder/global_args.rs +++ b/tests/builder/global_args.rs @@ -126,3 +126,40 @@ fn deeply_nested_discovery() { let m = m.subcommand_matches("c").unwrap(); assert!(*m.get_one::("long-c").expect("defaulted by clap")); } + +#[test] +fn global_overrides_default() { + let cmd = Command::new("test") + .arg( + Arg::new("name") + .long("name") + .global(true) + .takes_value(true) + .default_value("from_default"), + ) + .subcommand(Command::new("sub")); + + let m = cmd.clone().try_get_matches_from(["test"]).unwrap(); + assert_eq!( + m.get_one::("name").unwrap().as_str(), + "from_default" + ); + + let m = cmd + .clone() + .try_get_matches_from(["test", "--name", "from_arg"]) + .unwrap(); + assert_eq!(m.get_one::("name").unwrap().as_str(), "from_arg"); + + let m = cmd + .clone() + .try_get_matches_from(["test", "--name", "from_arg", "sub"]) + .unwrap(); + assert_eq!(m.get_one::("name").unwrap().as_str(), "from_arg"); + + let m = cmd + .clone() + .try_get_matches_from(["test", "sub", "--name", "from_arg"]) + .unwrap(); + assert_eq!(m.get_one::("name").unwrap().as_str(), "from_arg"); +} From 72d206e4d90e39e106965c8cf0c10e1e6a8f88c7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 27 Jun 2022 22:36:14 -0500 Subject: [PATCH 2/2] fix(parser): Ensure globals override env vars This fixes a bug introduced in 4a694f3592b702f86c7a6846b867b61d9feff5fe when we were trying to move away from presence checks via occurrences. I switched it to the common type of presence check but really what we want is a highest-precedence check. Fixes #3872 --- src/parser/arg_matcher.rs | 4 +--- tests/builder/global_args.rs | 37 ++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/parser/arg_matcher.rs b/src/parser/arg_matcher.rs index 8d15c5799f4..22087e72201 100644 --- a/src/parser/arg_matcher.rs +++ b/src/parser/arg_matcher.rs @@ -74,9 +74,7 @@ impl ArgMatcher { // a default value of `other` myprog would have an existing MatchedArg for // `--global-arg` where the value is `other` let to_update = if let Some(parent_ma) = vals_map.get(global_arg) { - if parent_ma.check_explicit(ArgPredicate::IsPresent) - && !ma.check_explicit(ArgPredicate::IsPresent) - { + if parent_ma.source() > ma.source() { parent_ma } else { ma diff --git a/tests/builder/global_args.rs b/tests/builder/global_args.rs index f3419426bb5..2f9833bb911 100644 --- a/tests/builder/global_args.rs +++ b/tests/builder/global_args.rs @@ -163,3 +163,40 @@ fn global_overrides_default() { .unwrap(); assert_eq!(m.get_one::("name").unwrap().as_str(), "from_arg"); } + +#[test] +#[cfg(feature = "env")] +fn global_overrides_env() { + std::env::set_var("GLOBAL_OVERRIDES_ENV", "from_env"); + + let cmd = Command::new("test") + .arg( + Arg::new("name") + .long("name") + .global(true) + .takes_value(true) + .env("GLOBAL_OVERRIDES_ENV"), + ) + .subcommand(Command::new("sub")); + + let m = cmd.clone().try_get_matches_from(["test"]).unwrap(); + assert_eq!(m.get_one::("name").unwrap().as_str(), "from_env"); + + let m = cmd + .clone() + .try_get_matches_from(["test", "--name", "from_arg"]) + .unwrap(); + assert_eq!(m.get_one::("name").unwrap().as_str(), "from_arg"); + + let m = cmd + .clone() + .try_get_matches_from(["test", "--name", "from_arg", "sub"]) + .unwrap(); + assert_eq!(m.get_one::("name").unwrap().as_str(), "from_arg"); + + let m = cmd + .clone() + .try_get_matches_from(["test", "sub", "--name", "from_arg"]) + .unwrap(); + assert_eq!(m.get_one::("name").unwrap().as_str(), "from_arg"); +}