Skip to content

Commit 9b985a3

Browse files
epageepage
authored
Merge pull request #5912 from epage/takes
feat(builder): Allow flags to take `num_args=0..=1`
2 parents e03d632 + 389fbe8 commit 9b985a3

File tree

8 files changed

+201
-19
lines changed

8 files changed

+201
-19
lines changed

‎clap_builder/src/builder/action.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#[cfg(debug_assertions)]
22
use crate::util::AnyValueId;
33

4+
use crate::builder::ValueRange;
5+
46
/// Behavior of arguments when they are encountered while parsing
57
///
68
/// # Examples
@@ -369,6 +371,35 @@ impl ArgAction {
369371
}
370372
}
371373

374+
#[cfg(debug_assertions)]
375+
pub(crate) fn max_num_args(&self) -> ValueRange {
376+
match self {
377+
Self::Set => ValueRange::FULL,
378+
Self::Append => ValueRange::FULL,
379+
Self::SetTrue => ValueRange::OPTIONAL,
380+
Self::SetFalse => ValueRange::OPTIONAL,
381+
Self::Count => ValueRange::EMPTY,
382+
Self::Help => ValueRange::EMPTY,
383+
Self::HelpShort => ValueRange::EMPTY,
384+
Self::HelpLong => ValueRange::EMPTY,
385+
Self::Version => ValueRange::EMPTY,
386+
}
387+
}
388+
389+
pub(crate) fn default_num_args(&self) -> ValueRange {
390+
match self {
391+
Self::Set => ValueRange::SINGLE,
392+
Self::Append => ValueRange::SINGLE,
393+
Self::SetTrue => ValueRange::EMPTY,
394+
Self::SetFalse => ValueRange::EMPTY,
395+
Self::Count => ValueRange::EMPTY,
396+
Self::Help => ValueRange::EMPTY,
397+
Self::HelpShort => ValueRange::EMPTY,
398+
Self::HelpLong => ValueRange::EMPTY,
399+
Self::Version => ValueRange::EMPTY,
400+
}
401+
}
402+
372403
pub(crate) fn default_value(&self) -> Option<&'static std::ffi::OsStr> {
373404
match self {
374405
Self::Set => None,

‎clap_builder/src/builder/arg.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4546,11 +4546,7 @@ impl Arg {
45464546
if val_names_len > 1 {
45474547
self.num_vals.get_or_insert(val_names_len.into());
45484548
} else {
4549-
let nargs = if self.get_action().takes_values() {
4550-
ValueRange::SINGLE
4551-
} else {
4552-
ValueRange::EMPTY
4553-
};
4549+
let nargs = self.get_action().default_num_args();
45544550
self.num_vals.get_or_insert(nargs);
45554551
}
45564552
}

‎clap_builder/src/builder/debug_asserts.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -690,14 +690,14 @@ fn assert_arg(arg: &Arg) {
690690
arg.get_id(),
691691
);
692692

693-
if arg.is_takes_value_set() {
694-
assert!(
695-
arg.get_action().takes_values(),
696-
"Argument `{}`'s selected action {:?} contradicts `takes_value`",
697-
arg.get_id(),
698-
arg.get_action()
699-
);
700-
}
693+
assert!(
694+
arg.get_num_args().unwrap_or(1.into()).max_values()
695+
<= arg.get_action().max_num_args().max_values(),
696+
"Argument `{}`'s action {:?} is incompatible with `num_args({:?})`",
697+
arg.get_id(),
698+
arg.get_action(),
699+
arg.get_num_args().unwrap_or(1.into())
700+
);
701701
if let Some(action_type_id) = arg.get_action().value_type_id() {
702702
assert_eq!(
703703
action_type_id,

‎clap_builder/src/builder/range.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,17 @@ impl ValueRange {
1818
end_inclusive: 1,
1919
};
2020

21+
#[cfg(debug_assertions)]
22+
pub(crate) const OPTIONAL: Self = Self {
23+
start_inclusive: 0,
24+
end_inclusive: 1,
25+
};
26+
27+
pub(crate) const FULL: Self = Self {
28+
start_inclusive: 0,
29+
end_inclusive: usize::MAX,
30+
};
31+
2132
/// Create a range
2233
///
2334
/// # Panics
@@ -135,9 +146,7 @@ impl From<std::ops::Range<usize>> for ValueRange {
135146

136147
impl From<std::ops::RangeFull> for ValueRange {
137148
fn from(_: std::ops::RangeFull) -> Self {
138-
let start_inclusive = 0;
139-
let end_inclusive = usize::MAX;
140-
Self::raw(start_inclusive, end_inclusive)
149+
Self::FULL
141150
}
142151
}
143152

@@ -176,7 +185,10 @@ impl From<std::ops::RangeToInclusive<usize>> for ValueRange {
176185
impl std::fmt::Display for ValueRange {
177186
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
178187
ok!(self.start_inclusive.fmt(f));
179-
if !self.is_fixed() {
188+
if self.is_fixed() {
189+
} else if self.end_inclusive == usize::MAX {
190+
ok!("..".fmt(f));
191+
} else {
180192
ok!("..=".fmt(f));
181193
ok!(self.end_inclusive.fmt(f));
182194
}

‎clap_complete/src/engine/complete.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ pub fn complete(
8181
});
8282

8383
if let Some(opt) = opt {
84-
if opt.get_action().takes_values() && value.is_none() {
84+
if opt.get_num_args().expect("built").takes_values() && value.is_none() {
8585
next_state = ParseState::Opt((opt, 1));
8686
};
8787
} else if pos_allows_hyphen(current_cmd, pos_index) {
@@ -581,7 +581,10 @@ fn parse_shortflags<'c, 's>(
581581
});
582582
is_find.unwrap_or(false)
583583
});
584-
if opt.map(|o| o.get_action().takes_values()).unwrap_or(false) {
584+
if opt
585+
.map(|o| o.get_num_args().expect("built").takes_values())
586+
.unwrap_or(false)
587+
{
585588
takes_value_opt = opt;
586589
break;
587590
}

‎tests/builder/action.rs

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,19 @@ fn set_true_with_required_if_eq() {
219219
assert_eq!(matches.get_flag("mammal"), true);
220220
}
221221

222+
#[test]
223+
#[should_panic = "Argument `mammal`'s action SetTrue is incompatible with `num_args(1..)`"]
224+
fn set_true_with_incompatible_num_args() {
225+
Command::new("test")
226+
.arg(
227+
Arg::new("mammal")
228+
.long("mammal")
229+
.action(ArgAction::SetTrue)
230+
.num_args(1..),
231+
)
232+
.build();
233+
}
234+
222235
#[test]
223236
fn set_false() {
224237
let cmd = Command::new("test").arg(
@@ -333,6 +346,19 @@ fn set_false_with_default_value_if_value() {
333346
assert_eq!(matches.get_flag("mammal"), false);
334347
}
335348

349+
#[test]
350+
#[should_panic = "Argument `mammal`'s action SetFalse is incompatible with `num_args(1..)`"]
351+
fn set_false_with_incompatible_num_args() {
352+
Command::new("test")
353+
.arg(
354+
Arg::new("mammal")
355+
.long("mammal")
356+
.action(ArgAction::SetFalse)
357+
.num_args(1..),
358+
)
359+
.build();
360+
}
361+
336362
#[test]
337363
fn count() {
338364
let cmd = Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::Count));
@@ -442,3 +468,69 @@ fn count_with_default_value_if_value() {
442468
assert_eq!(*matches.get_one::<u8>("dog").unwrap(), 0);
443469
assert_eq!(*matches.get_one::<u8>("mammal").unwrap(), 1);
444470
}
471+
472+
#[test]
473+
#[should_panic = "Argument `mammal`'s action Count is incompatible with `num_args(1..)`"]
474+
fn count_with_incompatible_num_args() {
475+
Command::new("test")
476+
.arg(
477+
Arg::new("mammal")
478+
.long("mammal")
479+
.action(ArgAction::Count)
480+
.num_args(1..),
481+
)
482+
.build();
483+
}
484+
485+
#[test]
486+
#[should_panic = "Argument `mammal`'s action Help is incompatible with `num_args(1..)`"]
487+
fn help_with_incompatible_num_args() {
488+
Command::new("test")
489+
.arg(
490+
Arg::new("mammal")
491+
.long("mammal")
492+
.action(ArgAction::Help)
493+
.num_args(1..),
494+
)
495+
.build();
496+
}
497+
498+
#[test]
499+
#[should_panic = "Argument `mammal`'s action HelpShort is incompatible with `num_args(1..)`"]
500+
fn help_short_with_incompatible_num_args() {
501+
Command::new("test")
502+
.arg(
503+
Arg::new("mammal")
504+
.long("mammal")
505+
.action(ArgAction::HelpShort)
506+
.num_args(1..),
507+
)
508+
.build();
509+
}
510+
511+
#[test]
512+
#[should_panic = "Argument `mammal`'s action HelpLong is incompatible with `num_args(1..)`"]
513+
fn help_long_with_incompatible_num_args() {
514+
Command::new("test")
515+
.arg(
516+
Arg::new("mammal")
517+
.long("mammal")
518+
.action(ArgAction::HelpLong)
519+
.num_args(1..),
520+
)
521+
.build();
522+
}
523+
524+
#[test]
525+
#[should_panic = "Argument `mammal`'s action Version is incompatible with `num_args(1..)`"]
526+
fn version_with_incompatible_num_args() {
527+
Command::new("test")
528+
.version("1.0.0")
529+
.arg(
530+
Arg::new("mammal")
531+
.long("mammal")
532+
.action(ArgAction::Version)
533+
.num_args(1..),
534+
)
535+
.build();
536+
}

‎tests/builder/flags.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,3 +222,28 @@ fn leading_dash_stripped() {
222222
let cmd = Command::new("mycat").arg(Arg::new("filename").long("--filename"));
223223
cmd.debug_assert();
224224
}
225+
226+
#[test]
227+
fn optional_value() {
228+
let cmd = Command::new("flag").args([arg!(-f --flag "some flag")
229+
.action(ArgAction::SetTrue)
230+
.num_args(0..=1)]);
231+
232+
let m = cmd.clone().try_get_matches_from(vec![""]).unwrap();
233+
assert!(!*m.get_one::<bool>("flag").expect("defaulted by clap"));
234+
235+
let m = cmd.clone().try_get_matches_from(vec!["", "-f"]).unwrap();
236+
assert!(*m.get_one::<bool>("flag").expect("defaulted by clap"));
237+
238+
let m = cmd
239+
.clone()
240+
.try_get_matches_from(vec!["", "-f", "false"])
241+
.unwrap();
242+
assert!(!*m.get_one::<bool>("flag").expect("defaulted by clap"));
243+
244+
let m = cmd
245+
.clone()
246+
.try_get_matches_from(vec!["", "-f", "true"])
247+
.unwrap();
248+
assert!(*m.get_one::<bool>("flag").expect("defaulted by clap"));
249+
}

‎tests/derive/flags.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,3 +329,26 @@ fn unit_for_negation() {
329329
Opt::try_parse_from(["test", "--arg", "--no-arg"]).unwrap()
330330
);
331331
}
332+
333+
#[test]
334+
fn optional_value_flag() {
335+
#[derive(Parser, PartialEq, Eq, Debug)]
336+
struct Opt {
337+
#[arg(short, long, num_args=0..=1)]
338+
alice: bool,
339+
}
340+
341+
assert_eq!(Opt { alice: false }, Opt::try_parse_from(["test"]).unwrap());
342+
assert_eq!(
343+
Opt { alice: true },
344+
Opt::try_parse_from(["test", "-a"]).unwrap()
345+
);
346+
assert_eq!(
347+
Opt { alice: true },
348+
Opt::try_parse_from(["test", "-a", "true"]).unwrap()
349+
);
350+
assert_eq!(
351+
Opt { alice: false },
352+
Opt::try_parse_from(["test", "-a", "false"]).unwrap()
353+
);
354+
}

0 commit comments

Comments
 (0)