Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(parser)!: Split on value delimiter after validating num_args #4025

Merged
merged 1 commit into from Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Expand Up @@ -23,7 +23,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Replace `Arg::number_of_values` (average across occurrences) with `Arg::num_args` (per occurrence, raw CLI args, not parsed values)
- `num_args(0)` no longer implies `takes_value(true).multiple_values(true)`
- `num_args(1)` no longer implies `multiple_values(true)`
- Does not check default values, only what the user explicitly passes in
- Does not check default or env values, only what the user explicitly passes in
- No longer terminates on delimited values
- `Arg::value_terminator` must stand on its own now rather than being in a delimited list
- Sometimes `Arg::default_missing_value` didn't require `num_args(0..=1)`, now it does
- Replace `Arg::min_values` (across all occurrences) with `Arg::num_args(N..)` (per occurrence)
- Replace `Arg::max_values` (across all occurrences) with `Arg::num_args(1..=M)` (per occurrence)
Expand Down
5 changes: 2 additions & 3 deletions src/error/kind.rs
Expand Up @@ -104,9 +104,8 @@ pub enum ErrorKind {
/// # use clap::{Command, Arg, error::ErrorKind};
/// let result = Command::new("prog")
/// .arg(Arg::new("arg")
/// .num_args(1..=2)
/// .require_value_delimiter(true))
/// .try_get_matches_from(vec!["prog", "too,many,values"]);
/// .num_args(1..=2))
/// .try_get_matches_from(vec!["prog", "too", "many", "values"]);
/// assert!(result.is_err());
/// assert_eq!(result.unwrap_err().kind(), ErrorKind::TooManyValues);
/// ```
Expand Down
20 changes: 19 additions & 1 deletion src/parser/arg_matcher.rs
Expand Up @@ -202,7 +202,9 @@ impl ArgMatcher {
"ArgMatcher::needs_more_vals: o={}, pending={}",
o.name, num_pending
);
if let Some(expected) = o.get_num_args() {
if num_pending == 1 && o.is_require_value_delimiter_set() {
false
} else if let Some(expected) = o.get_num_args() {
debug!(
"ArgMatcher::needs_more_vals: expected={}, actual={}",
expected, num_pending
Expand All @@ -221,19 +223,35 @@ impl ArgMatcher {
&mut self,
id: &Id,
ident: Option<Identifier>,
trailing_values: bool,
) -> &mut Vec<OsString> {
let pending = self.pending.get_or_insert_with(|| PendingArg {
id: id.clone(),
ident,
raw_vals: Default::default(),
trailing_idx: None,
});
debug_assert_eq!(pending.id, *id, "{}", INTERNAL_ERROR_MSG);
if ident.is_some() {
debug_assert_eq!(pending.ident, ident, "{}", INTERNAL_ERROR_MSG);
}
if trailing_values {
pending
.trailing_idx
.get_or_insert_with(|| pending.raw_vals.len());
}
&mut pending.raw_vals
}

pub(crate) fn start_trailing(&mut self) {
if let Some(pending) = &mut self.pending {
// Allow asserting its started on subsequent calls
pending
.trailing_idx
.get_or_insert_with(|| pending.raw_vals.len());
}
}

pub(crate) fn take_pending(&mut self) -> Option<PendingArg> {
self.pending.take()
}
Expand Down