Skip to content

Commit

Permalink
fix: incorrectly handle args that startswith - but are not flags/op…
Browse files Browse the repository at this point in the history
…tions (#344)
  • Loading branch information
sigoden authored Aug 16, 2024
1 parent 9b2dd0c commit e0a9167
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 36 deletions.
77 changes: 53 additions & 24 deletions src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use crate::{
ChoiceValue, DefaultValue,
};
use anyhow::Result;
use indexmap::IndexSet;

const UTIL_FNS: [(&str, &str); 6] = [
const UTIL_FNS: [(&str, &str); 7] = [
(
"_argc_take_args",
r#"
Expand All @@ -23,7 +24,7 @@ _argc_take_args() {
else
while [[ $_argc_take_index -lt $_argc_len ]]; do
_argc_take_value="${argc__args[_argc_take_index]}"
if [[ -n "$signs" ]] && [[ "$_argc_take_value" =~ ^["$signs"] ]]; then
if _argc_maybe_flag_option "$signs" "$_argc_take_value"; then
if [[ "${#_argc_take_value}" -gt 1 ]]; then
break
fi
Expand Down Expand Up @@ -180,6 +181,35 @@ _argc_check_bool() {
_argc_die "error: environment variable '$env_name' has invalid value for param '$param_name'"
fi
}
"#,
),
(
"_argc_maybe_flag_option",
r#"
_argc_maybe_flag_option() {
local signs="$1" arg="$2"
if [[ -z "$signs" ]]; then
return 1
fi
local cond=false
if [[ "$signs" == *"+"* ]]; then
if [[ "$arg" =~ ^\+[^+].* ]]; then
cond=true
fi
elif [[ "$arg" == -* ]]; then
if (( ${#arg} < 3 )) || [[ ! "$arg" =~ ^---.* ]]; then
cond=true
fi
fi
if [[ "$cond" == "false" ]]; then
return 1
fi
local value="${arg%%=*}"
if [[ "$value" =~ [[:space:]] ]]; then
return 1
fi
return 0
}
"#,
),
];
Expand Down Expand Up @@ -232,7 +262,7 @@ fn build_root(cmd: &Command, wrap_width: Option<usize>) -> String {
let after_hook = if after_hook { "\n _argc_after" } else { "" };
let mut util_fns = String::new();
for (fn_name, util_fn) in UTIL_FNS {
if command.contains(fn_name) {
if command.contains(fn_name) || util_fns.contains(fn_name) {
util_fns.push_str(util_fn);
}
}
Expand Down Expand Up @@ -369,21 +399,6 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
} else {
String::new()
};
let parse_unknown_flag_options = if !cmd.flag_option_params.is_empty() {
let unknown_flags = flag_option_signs
.chars()
.map(|v| format!("{v}?*"))
.collect::<Vec<String>>()
.join(" | ");
format!(
r#"
{unknown_flags})
_argc_die "error: unexpected argument \`$_argc_key\` found"
;;"#
)
} else {
String::new()
};
let parse_subcommands = if !cmd.subcommands.is_empty() {
let mut parses: Vec<String> = cmd
.subcommands
Expand Down Expand Up @@ -437,13 +452,24 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
String::new()
};

let handle_unknown_flag_options = if !cmd.flag_option_params.is_empty() {
let signs = flag_option_signs.iter().collect::<String>();
format!(
r#"
if _argc_maybe_flag_option "{signs}" "$_argc_item"; then
_argc_die "error: unexpected argument \`$_argc_key\` found"
fi"#,
)
} else {
String::new()
};
let parse_fallback = if !cmd.subcommands.is_empty() && cmd.positional_params.is_empty() {
let name = cmd.full_name();
if let Some(subcmd) = cmd.find_default_subcommand() {
let paths = subcmd.paths.join("_");
format!(
r#"
*)
*){handle_unknown_flag_options}
if [[ "${{#argc__positionals[@]}}" -eq 0 ]]; then
_argc_action=_argc_parse_{paths}
break
Expand All @@ -453,7 +479,7 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
} else {
format!(
r#"
*)
*){handle_unknown_flag_options}
_argc_die "error: \`{name}\` requires a subcommand but one was not provided"$'\n'" [subcommands: $_argc_subcmds]"
;;"#
)
Expand All @@ -473,7 +499,7 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
};
format!(
r#"
*)
*){handle_unknown_flag_options}
argc__positionals+=("$_argc_item")
_argc_index=$((_argc_index + 1)){terminated}
;;"#
Expand All @@ -499,7 +525,6 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
parse_dash,
parse_flag_options,
parse_subcommands,
parse_unknown_flag_options,
parse_fallback,
]
.join("");
Expand All @@ -524,7 +549,7 @@ _argc_parse{suffix}() {{
)
}

fn build_parse_flag_option(param: &FlagOptionParam, signs: &str) -> String {
fn build_parse_flag_option(param: &FlagOptionParam, signs: &IndexSet<char>) -> String {
let names = param.list_names().join(" | ");
let long_name = param.long_name();
let var_name = param.var_name();
Expand Down Expand Up @@ -555,7 +580,11 @@ fn build_parse_flag_option(param: &FlagOptionParam, signs: &str) -> String {
;;"#
)
} else {
let signs = if param.terminated() { "" } else { signs };
let signs: String = if param.terminated() {
"".into()
} else {
signs.iter().collect::<String>()
};
let delimiter = match param.delimiter() {
Some(v) => v.to_string(),
None => String::new(),
Expand Down
4 changes: 2 additions & 2 deletions src/command/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,15 @@ impl Command {
}
}

pub(crate) fn flag_option_signs(&self) -> String {
pub(crate) fn flag_option_signs(&self) -> IndexSet<char> {
let mut signs: IndexSet<char> = ['-'].into();
for param in &self.flag_option_params {
if let Some(short) = param.short() {
signs.extend(short.chars().take(1))
}
signs.extend(param.long_prefix().chars().take(1))
}
signs.into_iter().collect()
signs
}

pub(crate) fn cmd_name(&self) -> String {
Expand Down
36 changes: 27 additions & 9 deletions src/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl<'a, 'b, T: Runtime> Matcher<'a, 'b, T> {
} else {
let mut is_rest_args_positional = false; // option(e.g. -f --foo) will be treated as positional arg
if let Some(arg) = args.last() {
if arg.starts_with(|c| root_cmd.flag_option_signs().contains(c)) {
if maybe_flag_option(arg, &root_cmd.flag_option_signs()) {
arg_comp = ArgComp::FlagOrOption;
} else if !arg.is_empty() {
arg_comp = ArgComp::CommandOrPositional;
Expand Down Expand Up @@ -137,7 +137,7 @@ impl<'a, 'b, T: Runtime> Matcher<'a, 'b, T> {
&mut is_rest_args_positional,
cmd,
);
} else if arg.len() > 1 && arg.starts_with(|c| signs.contains(c)) {
} else if arg.len() > 1 && maybe_flag_option(arg, &signs) {
if let Some((k, v)) = arg.split_once('=') {
if let Some(param) = cmd.find_flag_option(k) {
add_param_choice_fn(&mut choice_fns, param);
Expand Down Expand Up @@ -1181,7 +1181,7 @@ fn take_value_args<'a>(
args: &'a [String],
start: usize,
len: usize,
signs: &str,
signs: &IndexSet<char>,
assigned: bool,
compgen: bool,
) -> Vec<&'a str> {
Expand All @@ -1192,9 +1192,7 @@ fn take_value_args<'a>(
let args_len = args.len();
let end = (start + len).min(args_len);
for (i, arg) in args.iter().enumerate().take(end).skip(start) {
if arg.starts_with(|c| signs.contains(c))
&& (arg.len() > 1 || (compgen && i == args_len - 1))
{
if maybe_flag_option(arg, signs) && (arg.len() > 1 || (compgen && i == args_len - 1)) {
break;
}
output.push(arg.as_str());
Expand Down Expand Up @@ -1243,7 +1241,7 @@ fn match_flag_option<'a, 'b>(
arg_comp: &mut ArgComp,
split_last_arg_at: &mut Option<usize>,
combine_shorts: bool,
signs: &str,
signs: &IndexSet<char>,
compgen: bool,
) {
let arg = &args[*arg_index];
Expand Down Expand Up @@ -1361,8 +1359,7 @@ fn comp_subcomands(cmd: &Command, flag: bool) -> Vec<CompItem> {
if i > 0 && v.len() < 2 {
continue;
}
if (flag && v.starts_with(|c| signs.contains(c)))
|| (!flag && !v.starts_with(|c| signs.contains(c)))
if (flag && maybe_flag_option(&v, &signs)) || (!flag && !maybe_flag_option(&v, &signs))
{
if !flag {
has_help_subcmd = true;
Expand Down Expand Up @@ -1499,3 +1496,24 @@ fn delimit_arg_values<'x, T: Param>(param: &T, values: &[&'x str]) -> Vec<&'x st
fn is_bool_value(value: &str) -> bool {
matches!(value, "true" | "false" | "0" | "1")
}

fn maybe_flag_option(arg: &str, signs: &IndexSet<char>) -> bool {
let cond = if signs.contains(&'+') && arg.starts_with('+') {
!arg.starts_with("++")
} else if arg.starts_with('-') {
arg.len() < 3 || !arg.starts_with("---")
} else {
false
};
if !cond {
return false;
}
let value = match arg.split_once('=') {
Some((v, _)) => v,
_ => arg,
};
if value.contains(|c: char| c.is_whitespace()) {
return false;
}
true
}
11 changes: 10 additions & 1 deletion src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,14 @@ fn parse_with_long_head(input: &str) -> nom::IResult<&str, (Option<&str>, &str)>
),
peek(space1),
)),
preceded(space0, alt((tag("--"), tag("-"), tag("+")))),
preceded(
space0,
alt((
terminated(tag("--"), peek(not(char('-')))),
terminated(tag("-"), peek(not(char('-')))),
terminated(tag("+"), peek(not(char('+')))),
)),
),
),)),
|(short, long_prefix)| (short.map(|_| &input[0..2]), long_prefix),
)(input)
Expand Down Expand Up @@ -1047,6 +1054,8 @@ mod tests {
Ok(("foo", (Some("+f"), "+")))
);
assert_eq!(parse_with_long_head("+foo"), Ok(("foo", (None, "+"))));
assert!(parse_with_long_head("-f ---foo").is_err());
assert!(parse_with_long_head("+f ++foo").is_err());
}

#[test]
Expand Down
38 changes: 38 additions & 0 deletions tests/snapshots/integration__spec__option_value_dash.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
source: tests/spec.rs
expression: data
---
************ RUN ************
prog --oa ---

# OUTPUT
argc_oa=---
argc__args=( prog --oa --- )
argc__positionals=( )

# RUN_OUTPUT
argc__args=([0]="prog" [1]="--oa" [2]="---")
argc__positionals=()
argc_oa=---

************ RUN ************
prog ---

# OUTPUT
argc__args=( prog --- )
argc__positionals=( --- )

# RUN_OUTPUT
argc__args=([0]="prog" [1]="---")
argc__positionals=([0]="---")

************ RUN ************
prog --a b

# OUTPUT
argc__args=( prog '--a b' )
argc__positionals=( '--a b' )

# RUN_OUTPUT
argc__args=([0]="prog" [1]="--a b")
argc__positionals=([0]="--a b")
15 changes: 15 additions & 0 deletions tests/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,18 @@ fn option_single_dash() {
[vec!["prog", "--oa", "-"], vec!["prog", "--oa", "-", ""],]
);
}

#[test]
fn option_value_dash() {
let script = r###"
# @option --oa
"###;
snapshot_multi!(
script,
[
vec!["prog", "--oa", "---"],
vec!["prog", "---"],
vec!["prog", "--a b"]
]
);
}

0 comments on commit e0a9167

Please sign in to comment.