Skip to content

Commit

Permalink
fix(parser): Don't double-increment index on flags
Browse files Browse the repository at this point in the history
  • Loading branch information
epage committed Jun 1, 2022
1 parent 06ea572 commit 4afd1aa
Show file tree
Hide file tree
Showing 2 changed files with 266 additions and 21 deletions.
21 changes: 0 additions & 21 deletions src/parser/parser.rs
Expand Up @@ -1183,13 +1183,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
Ok(ParseResult::ValuesDone)
}
ArgAction::SetTrue => {
if source == ValueSource::CommandLine
&& matches!(ident, Some(Identifier::Short) | Some(Identifier::Long))
{
// Record flag's index
self.cur_idx.set(self.cur_idx.get() + 1);
debug!("Parser::react: cur_idx:={}", self.cur_idx.get());
}
let raw_vals = match raw_vals.len() {
0 => {
vec![OsString::from("true")]
Expand All @@ -1209,13 +1202,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
Ok(ParseResult::ValuesDone)
}
ArgAction::SetFalse => {
if source == ValueSource::CommandLine
&& matches!(ident, Some(Identifier::Short) | Some(Identifier::Long))
{
// Record flag's index
self.cur_idx.set(self.cur_idx.get() + 1);
debug!("Parser::react: cur_idx:={}", self.cur_idx.get());
}
let raw_vals = match raw_vals.len() {
0 => {
vec![OsString::from("false")]
Expand All @@ -1235,13 +1221,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
Ok(ParseResult::ValuesDone)
}
ArgAction::Count => {
if source == ValueSource::CommandLine
&& matches!(ident, Some(Identifier::Short) | Some(Identifier::Long))
{
// Record flag's index
self.cur_idx.set(self.cur_idx.get() + 1);
debug!("Parser::react: cur_idx:={}", self.cur_idx.get());
}
let raw_vals = match raw_vals.len() {
0 => {
let existing_value = *matcher
Expand Down
266 changes: 266 additions & 0 deletions tests/builder/action.rs
Expand Up @@ -2,6 +2,61 @@ use clap::builder::ArgAction;
use clap::Arg;
use clap::Command;

#[test]
fn set_true() {
let cmd =
Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::SetTrue));

let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
assert_eq!(matches.get_one::<bool>("mammal"), None);
assert_eq!(matches.is_present("mammal"), false);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), None);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(1));

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal", "--mammal"])
.unwrap();
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(2));
}

#[test]
fn set_true_with_default_value() {
let cmd = Command::new("test").arg(
Arg::new("mammal")
.long("mammal")
.action(ArgAction::SetTrue)
.default_value("false"),
);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(1));

let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(1));
}

#[test]
fn set_true_with_default_value_if_present() {
let cmd = Command::new("test")
Expand Down Expand Up @@ -47,3 +102,214 @@ fn set_true_with_default_value_if_value() {
assert_eq!(matches.get_one::<bool>("dog"), None);
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
}

#[test]
fn set_false() {
let cmd = Command::new("test").arg(
Arg::new("mammal")
.long("mammal")
.action(ArgAction::SetFalse),
);

let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
assert_eq!(matches.get_one::<bool>("mammal"), None);
assert_eq!(matches.is_present("mammal"), false);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), None);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(1));

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal", "--mammal"])
.unwrap();
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(2));
}

#[test]
fn set_false_with_default_value() {
let cmd = Command::new("test").arg(
Arg::new("mammal")
.long("mammal")
.action(ArgAction::SetFalse)
.default_value("true"),
);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(1));

let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(1));
}

#[test]
fn set_false_with_default_value_if_present() {
let cmd = Command::new("test")
.arg(
Arg::new("mammal")
.long("mammal")
.action(ArgAction::SetFalse)
.default_value_if("dog", None, Some("false")),
)
.arg(Arg::new("dog").long("dog").action(ArgAction::SetFalse));

let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(matches.get_one::<bool>("dog"), None);
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
}

#[test]
fn set_false_with_default_value_if_value() {
let cmd = Command::new("test")
.arg(
Arg::new("mammal")
.long("mammal")
.action(ArgAction::SetFalse)
.default_value_if("dog", Some("false"), Some("false")),
)
.arg(Arg::new("dog").long("dog").action(ArgAction::SetFalse));

let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(matches.get_one::<bool>("dog"), None);
assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
}

#[test]
fn count() {
let cmd = Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::Count));

let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
assert_eq!(matches.get_one::<u64>("mammal"), None);
assert_eq!(matches.is_present("mammal"), false);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), None);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 1);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(1));

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal", "--mammal"])
.unwrap();
assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 2);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(2));
}

#[test]
fn count_with_default_value() {
let cmd = Command::new("test").arg(
Arg::new("mammal")
.long("mammal")
.action(ArgAction::Count)
.default_value("10"),
);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 1);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(1));

let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 10);
assert_eq!(matches.is_present("mammal"), true);
assert_eq!(matches.occurrences_of("mammal"), 0);
assert_eq!(matches.index_of("mammal"), Some(1));
}

#[test]
fn count_with_default_value_if_present() {
let cmd = Command::new("test")
.arg(
Arg::new("mammal")
.long("mammal")
.action(ArgAction::Count)
.default_value_if("dog", None, Some("10")),
)
.arg(Arg::new("dog").long("dog").action(ArgAction::Count));

let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
assert_eq!(*matches.get_one::<u64>("dog").unwrap(), 1);
assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 10);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(matches.get_one::<u64>("dog"), None);
assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 1);
}

#[test]
fn count_with_default_value_if_value() {
let cmd = Command::new("test")
.arg(
Arg::new("mammal")
.long("mammal")
.action(ArgAction::Count)
.default_value_if("dog", Some("2"), Some("10")),
)
.arg(Arg::new("dog").long("dog").action(ArgAction::Count));

let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
assert_eq!(*matches.get_one::<u64>("dog").unwrap(), 1);
assert_eq!(matches.get_one::<u64>("mammal"), None);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--dog", "--dog"])
.unwrap();
assert_eq!(*matches.get_one::<u64>("dog").unwrap(), 2);
assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 10);

let matches = cmd
.clone()
.try_get_matches_from(["test", "--mammal"])
.unwrap();
assert_eq!(matches.get_one::<u64>("dog"), None);
assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 1);
}

0 comments on commit 4afd1aa

Please sign in to comment.