Skip to content

Commit e0a9167

Browse files
sigodensigoden
authored
fix: incorrectly handle args that startswith - but are not flags/options (#344)
1 parent 9b2dd0c commit e0a9167

File tree

6 files changed

+145
-36
lines changed

6 files changed

+145
-36
lines changed

‎src/build.rs‎

Lines changed: 53 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ use crate::{
55
ChoiceValue, DefaultValue,
66
};
77
use anyhow::Result;
8+
use indexmap::IndexSet;
89

9-
const UTIL_FNS: [(&str, &str); 6] = [
10+
const UTIL_FNS: [(&str, &str); 7] = [
1011
(
1112
"_argc_take_args",
1213
r#"
@@ -23,7 +24,7 @@ _argc_take_args() {
2324
else
2425
while [[ $_argc_take_index -lt $_argc_len ]]; do
2526
_argc_take_value="${argc__args[_argc_take_index]}"
26-
if [[ -n "$signs" ]] && [[ "$_argc_take_value" =~ ^["$signs"] ]]; then
27+
if _argc_maybe_flag_option "$signs" "$_argc_take_value"; then
2728
if [[ "${#_argc_take_value}" -gt 1 ]]; then
2829
break
2930
fi
@@ -180,6 +181,35 @@ _argc_check_bool() {
180181
_argc_die "error: environment variable '$env_name' has invalid value for param '$param_name'"
181182
fi
182183
}
184+
"#,
185+
),
186+
(
187+
"_argc_maybe_flag_option",
188+
r#"
189+
_argc_maybe_flag_option() {
190+
local signs="$1" arg="$2"
191+
if [[ -z "$signs" ]]; then
192+
return 1
193+
fi
194+
local cond=false
195+
if [[ "$signs" == *"+"* ]]; then
196+
if [[ "$arg" =~ ^\+[^+].* ]]; then
197+
cond=true
198+
fi
199+
elif [[ "$arg" == -* ]]; then
200+
if (( ${#arg} < 3 )) || [[ ! "$arg" =~ ^---.* ]]; then
201+
cond=true
202+
fi
203+
fi
204+
if [[ "$cond" == "false" ]]; then
205+
return 1
206+
fi
207+
local value="${arg%%=*}"
208+
if [[ "$value" =~ [[:space:]] ]]; then
209+
return 1
210+
fi
211+
return 0
212+
}
183213
"#,
184214
),
185215
];
@@ -232,7 +262,7 @@ fn build_root(cmd: &Command, wrap_width: Option<usize>) -> String {
232262
let after_hook = if after_hook { "\n _argc_after" } else { "" };
233263
let mut util_fns = String::new();
234264
for (fn_name, util_fn) in UTIL_FNS {
235-
if command.contains(fn_name) {
265+
if command.contains(fn_name) || util_fns.contains(fn_name) {
236266
util_fns.push_str(util_fn);
237267
}
238268
}
@@ -369,21 +399,6 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
369399
} else {
370400
String::new()
371401
};
372-
let parse_unknown_flag_options = if !cmd.flag_option_params.is_empty() {
373-
let unknown_flags = flag_option_signs
374-
.chars()
375-
.map(|v| format!("{v}?*"))
376-
.collect::<Vec<String>>()
377-
.join(" | ");
378-
format!(
379-
r#"
380-
{unknown_flags})
381-
_argc_die "error: unexpected argument \`$_argc_key\` found"
382-
;;"#
383-
)
384-
} else {
385-
String::new()
386-
};
387402
let parse_subcommands = if !cmd.subcommands.is_empty() {
388403
let mut parses: Vec<String> = cmd
389404
.subcommands
@@ -437,13 +452,24 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
437452
String::new()
438453
};
439454

455+
let handle_unknown_flag_options = if !cmd.flag_option_params.is_empty() {
456+
let signs = flag_option_signs.iter().collect::<String>();
457+
format!(
458+
r#"
459+
if _argc_maybe_flag_option "{signs}" "$_argc_item"; then
460+
_argc_die "error: unexpected argument \`$_argc_key\` found"
461+
fi"#,
462+
)
463+
} else {
464+
String::new()
465+
};
440466
let parse_fallback = if !cmd.subcommands.is_empty() && cmd.positional_params.is_empty() {
441467
let name = cmd.full_name();
442468
if let Some(subcmd) = cmd.find_default_subcommand() {
443469
let paths = subcmd.paths.join("_");
444470
format!(
445471
r#"
446-
*)
472+
*){handle_unknown_flag_options}
447473
if [[ "${{#argc__positionals[@]}}" -eq 0 ]]; then
448474
_argc_action=_argc_parse_{paths}
449475
break
@@ -453,7 +479,7 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
453479
} else {
454480
format!(
455481
r#"
456-
*)
482+
*){handle_unknown_flag_options}
457483
_argc_die "error: \`{name}\` requires a subcommand but one was not provided"$'\n'" [subcommands: $_argc_subcmds]"
458484
;;"#
459485
)
@@ -473,7 +499,7 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
473499
};
474500
format!(
475501
r#"
476-
*)
502+
*){handle_unknown_flag_options}
477503
argc__positionals+=("$_argc_item")
478504
_argc_index=$((_argc_index + 1)){terminated}
479505
;;"#
@@ -499,7 +525,6 @@ fn build_parse(cmd: &Command, suffix: &str) -> String {
499525
parse_dash,
500526
parse_flag_options,
501527
parse_subcommands,
502-
parse_unknown_flag_options,
503528
parse_fallback,
504529
]
505530
.join("");
@@ -524,7 +549,7 @@ _argc_parse{suffix}() {{
524549
)
525550
}
526551

527-
fn build_parse_flag_option(param: &FlagOptionParam, signs: &str) -> String {
552+
fn build_parse_flag_option(param: &FlagOptionParam, signs: &IndexSet<char>) -> String {
528553
let names = param.list_names().join(" | ");
529554
let long_name = param.long_name();
530555
let var_name = param.var_name();
@@ -555,7 +580,11 @@ fn build_parse_flag_option(param: &FlagOptionParam, signs: &str) -> String {
555580
;;"#
556581
)
557582
} else {
558-
let signs = if param.terminated() { "" } else { signs };
583+
let signs: String = if param.terminated() {
584+
"".into()
585+
} else {
586+
signs.iter().collect::<String>()
587+
};
559588
let delimiter = match param.delimiter() {
560589
Some(v) => v.to_string(),
561590
None => String::new(),

‎src/command/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,15 +370,15 @@ impl Command {
370370
}
371371
}
372372

373-
pub(crate) fn flag_option_signs(&self) -> String {
373+
pub(crate) fn flag_option_signs(&self) -> IndexSet<char> {
374374
let mut signs: IndexSet<char> = ['-'].into();
375375
for param in &self.flag_option_params {
376376
if let Some(short) = param.short() {
377377
signs.extend(short.chars().take(1))
378378
}
379379
signs.extend(param.long_prefix().chars().take(1))
380380
}
381-
signs.into_iter().collect()
381+
signs
382382
}
383383

384384
pub(crate) fn cmd_name(&self) -> String {

‎src/matcher.rs‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ impl<'a, 'b, T: Runtime> Matcher<'a, 'b, T> {
9999
} else {
100100
let mut is_rest_args_positional = false; // option(e.g. -f --foo) will be treated as positional arg
101101
if let Some(arg) = args.last() {
102-
if arg.starts_with(|c| root_cmd.flag_option_signs().contains(c)) {
102+
if maybe_flag_option(arg, &root_cmd.flag_option_signs()) {
103103
arg_comp = ArgComp::FlagOrOption;
104104
} else if !arg.is_empty() {
105105
arg_comp = ArgComp::CommandOrPositional;
@@ -137,7 +137,7 @@ impl<'a, 'b, T: Runtime> Matcher<'a, 'b, T> {
137137
&mut is_rest_args_positional,
138138
cmd,
139139
);
140-
} else if arg.len() > 1 && arg.starts_with(|c| signs.contains(c)) {
140+
} else if arg.len() > 1 && maybe_flag_option(arg, &signs) {
141141
if let Some((k, v)) = arg.split_once('=') {
142142
if let Some(param) = cmd.find_flag_option(k) {
143143
add_param_choice_fn(&mut choice_fns, param);
@@ -1181,7 +1181,7 @@ fn take_value_args<'a>(
11811181
args: &'a [String],
11821182
start: usize,
11831183
len: usize,
1184-
signs: &str,
1184+
signs: &IndexSet<char>,
11851185
assigned: bool,
11861186
compgen: bool,
11871187
) -> Vec<&'a str> {
@@ -1192,9 +1192,7 @@ fn take_value_args<'a>(
11921192
let args_len = args.len();
11931193
let end = (start + len).min(args_len);
11941194
for (i, arg) in args.iter().enumerate().take(end).skip(start) {
1195-
if arg.starts_with(|c| signs.contains(c))
1196-
&& (arg.len() > 1 || (compgen && i == args_len - 1))
1197-
{
1195+
if maybe_flag_option(arg, signs) && (arg.len() > 1 || (compgen && i == args_len - 1)) {
11981196
break;
11991197
}
12001198
output.push(arg.as_str());
@@ -1243,7 +1241,7 @@ fn match_flag_option<'a, 'b>(
12431241
arg_comp: &mut ArgComp,
12441242
split_last_arg_at: &mut Option<usize>,
12451243
combine_shorts: bool,
1246-
signs: &str,
1244+
signs: &IndexSet<char>,
12471245
compgen: bool,
12481246
) {
12491247
let arg = &args[*arg_index];
@@ -1361,8 +1359,7 @@ fn comp_subcomands(cmd: &Command, flag: bool) -> Vec<CompItem> {
13611359
if i > 0 && v.len() < 2 {
13621360
continue;
13631361
}
1364-
if (flag && v.starts_with(|c| signs.contains(c)))
1365-
|| (!flag && !v.starts_with(|c| signs.contains(c)))
1362+
if (flag && maybe_flag_option(&v, &signs)) || (!flag && !maybe_flag_option(&v, &signs))
13661363
{
13671364
if !flag {
13681365
has_help_subcmd = true;
@@ -1499,3 +1496,24 @@ fn delimit_arg_values<'x, T: Param>(param: &T, values: &[&'x str]) -> Vec<&'x st
14991496
fn is_bool_value(value: &str) -> bool {
15001497
matches!(value, "true" | "false" | "0" | "1")
15011498
}
1499+
1500+
fn maybe_flag_option(arg: &str, signs: &IndexSet<char>) -> bool {
1501+
let cond = if signs.contains(&'+') && arg.starts_with('+') {
1502+
!arg.starts_with("++")
1503+
} else if arg.starts_with('-') {
1504+
arg.len() < 3 || !arg.starts_with("---")
1505+
} else {
1506+
false
1507+
};
1508+
if !cond {
1509+
return false;
1510+
}
1511+
let value = match arg.split_once('=') {
1512+
Some((v, _)) => v,
1513+
_ => arg,
1514+
};
1515+
if value.contains(|c: char| c.is_whitespace()) {
1516+
return false;
1517+
}
1518+
true
1519+
}

‎src/parser.rs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,14 @@ fn parse_with_long_head(input: &str) -> nom::IResult<&str, (Option<&str>, &str)>
398398
),
399399
peek(space1),
400400
)),
401-
preceded(space0, alt((tag("--"), tag("-"), tag("+")))),
401+
preceded(
402+
space0,
403+
alt((
404+
terminated(tag("--"), peek(not(char('-')))),
405+
terminated(tag("-"), peek(not(char('-')))),
406+
terminated(tag("+"), peek(not(char('+')))),
407+
)),
408+
),
402409
),)),
403410
|(short, long_prefix)| (short.map(|_| &input[0..2]), long_prefix),
404411
)(input)
@@ -1047,6 +1054,8 @@ mod tests {
10471054
Ok(("foo", (Some("+f"), "+")))
10481055
);
10491056
assert_eq!(parse_with_long_head("+foo"), Ok(("foo", (None, "+"))));
1057+
assert!(parse_with_long_head("-f ---foo").is_err());
1058+
assert!(parse_with_long_head("+f ++foo").is_err());
10501059
}
10511060

10521061
#[test]

‎tests/snapshots/integration__spec__option_value_dash.snap‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
source: tests/spec.rs
3+
expression: data
4+
---
5+
************ RUN ************
6+
prog --oa ---
7+
8+
# OUTPUT
9+
argc_oa=---
10+
argc__args=( prog --oa --- )
11+
argc__positionals=( )
12+
13+
# RUN_OUTPUT
14+
argc__args=([0]="prog" [1]="--oa" [2]="---")
15+
argc__positionals=()
16+
argc_oa=---
17+
18+
************ RUN ************
19+
prog ---
20+
21+
# OUTPUT
22+
argc__args=( prog --- )
23+
argc__positionals=( --- )
24+
25+
# RUN_OUTPUT
26+
argc__args=([0]="prog" [1]="---")
27+
argc__positionals=([0]="---")
28+
29+
************ RUN ************
30+
prog --a b
31+
32+
# OUTPUT
33+
argc__args=( prog '--a b' )
34+
argc__positionals=( '--a b' )
35+
36+
# RUN_OUTPUT
37+
argc__args=([0]="prog" [1]="--a b")
38+
argc__positionals=([0]="--a b")

‎tests/spec.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,3 +417,18 @@ fn option_single_dash() {
417417
[vec!["prog", "--oa", "-"], vec!["prog", "--oa", "-", ""],]
418418
);
419419
}
420+
421+
#[test]
422+
fn option_value_dash() {
423+
let script = r###"
424+
# @option --oa
425+
"###;
426+
snapshot_multi!(
427+
script,
428+
[
429+
vec!["prog", "--oa", "---"],
430+
vec!["prog", "---"],
431+
vec!["prog", "--a b"]
432+
]
433+
);
434+
}

0 commit comments

Comments
 (0)