Skip to content

Commit

Permalink
fix(parser): Always put in arg "" for external subcommands (unstable)
Browse files Browse the repository at this point in the history
This allows distinguishing external subcommands from built-in
subcommands which can especially be confusing when escaping subcommands.

Fixes clap-rs#3263
  • Loading branch information
epage committed May 6, 2022
1 parent 630dde7 commit 03f1321
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 7 deletions.
14 changes: 14 additions & 0 deletions src/parse/arg_matcher.rs
Expand Up @@ -85,6 +85,7 @@ impl ArgMatcher {
}
}

#[cfg(not(feature = "unstable-v4"))]
pub(crate) fn get_mut(&mut self, arg: &Id) -> Option<&mut MatchedArg> {
self.0.args.get_mut(arg)
}
Expand Down Expand Up @@ -142,6 +143,19 @@ impl ArgMatcher {
ma.inc_occurrences();
}

#[cfg(feature = "unstable-v4")]
pub(crate) fn inc_occurrence_of_external(&mut self, allow_invalid_utf8: bool) {
let id = &Id::empty_hash();
debug!(
"ArgMatcher::inc_occurrence_of_external: id={:?}, allow_invalid_utf8={}",
id, allow_invalid_utf8
);
let ma = self.entry(id).or_insert(MatchedArg::new());
ma.update_ty(ValueSource::CommandLine);
ma.invalid_utf8_allowed(allow_invalid_utf8);
ma.inc_occurrences();
}

pub(crate) fn add_val_to(&mut self, arg: &Id, val: OsString, ty: ValueSource, append: bool) {
if append {
self.append_val_to(arg, val, ty);
Expand Down
22 changes: 15 additions & 7 deletions src/parse/parser.rs
Expand Up @@ -428,26 +428,34 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {

// Collect the external subcommand args
let mut sc_m = ArgMatcher::new(self.cmd);
let allow_invalid_utf8 = self
.cmd
.is_allow_invalid_utf8_for_external_subcommands_set();
#[cfg(feature = "unstable-v4")]
{
sc_m.inc_occurrence_of_external(allow_invalid_utf8);
}

for v in raw_args.remaining(&mut args_cursor) {
let allow_invalid_utf8 = self
.cmd
.is_allow_invalid_utf8_for_external_subcommands_set();
if !allow_invalid_utf8 && v.to_str().is_none() {
return Err(ClapError::invalid_utf8(
self.cmd,
Usage::new(self.cmd).create_usage_with_title(&[]),
));
}
let external_id = &Id::empty_hash();
sc_m.add_val_to(
&Id::empty_hash(),
external_id,
v.to_os_string(),
ValueSource::CommandLine,
false,
);
sc_m.get_mut(&Id::empty_hash())
.expect("just inserted")
.invalid_utf8_allowed(allow_invalid_utf8);
#[cfg(not(feature = "unstable-v4"))]
{
sc_m.get_mut(external_id)
.expect("just inserted")
.invalid_utf8_allowed(allow_invalid_utf8);
}
}

matcher.subcommand(SubCommand {
Expand Down
40 changes: 40 additions & 0 deletions tests/builder/app_settings.rs
Expand Up @@ -851,6 +851,46 @@ fn issue_1093_allow_ext_sc() {
utils::assert_output(cmd, "clap-test --help", ALLOW_EXT_SC, false);
}

#[test]
#[cfg(not(feature = "unstable-v4"))]
fn allow_ext_sc_empty_args() {
let res = Command::new("clap-test")
.version("v1.4.8")
.allow_external_subcommands(true)
.allow_invalid_utf8_for_external_subcommands(true)
.try_get_matches_from(vec!["clap-test", "external-cmd"]);

assert!(res.is_ok(), "{}", res.unwrap_err());

match res.unwrap().subcommand() {
Some((name, args)) => {
assert_eq!(name, "external-cmd");
assert_eq!(args.values_of_lossy(""), None);
}
_ => unreachable!(),
}
}

#[test]
#[cfg(feature = "unstable-v4")]
fn allow_ext_sc_empty_args() {
let res = Command::new("clap-test")
.version("v1.4.8")
.allow_external_subcommands(true)
.allow_invalid_utf8_for_external_subcommands(true)
.try_get_matches_from(vec!["clap-test", "external-cmd"]);

assert!(res.is_ok(), "{}", res.unwrap_err());

match res.unwrap().subcommand() {
Some((name, args)) => {
assert_eq!(name, "external-cmd");
assert_eq!(args.values_of_lossy(""), Some(vec![]));
}
_ => unreachable!(),
}
}

#[test]
fn allow_ext_sc_when_sc_required() {
let res = Command::new("clap-test")
Expand Down

0 comments on commit 03f1321

Please sign in to comment.