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.

Sign up for GitHub

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

Jump to bottom

feat(builder): Expose ArgAction #3774

Merged
merged 9 commits into from Jun 1, 2022
Merged

feat(builder): Expose ArgAction #3774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fb99d2c
refactor(parser): Allow adding more actions in the future
epage May 31, 2022
eeca653
refactor(parser): Loosen Action trait requirements
epage May 31, 2022
7264bf2
refactor(parser): Clarify Action::Flag's behavior
epage May 31, 2022
2b95985
refactor(parser): Ensure action and are are in-sync
epage May 31, 2022
b1b5820
refactor(asserts): Ensure we always check positionals take values
epage May 31, 2022
91480de
feat(builder): Expose ArgAction
epage May 31, 2022
e53dd93
feat(builder): Infer takes_vaue from action
epage May 31, 2022
7163f8d
refactor(parser): Clarify intent of split_arg_values
epage May 31, 2022
3668059
refactor(parser): Clarify intent of push_arg_values
epage May 31, 2022
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
133 changes: 130 additions & 3 deletions src/builder/action.rs
View file
Open in desktop
@@ -1,8 +1,135 @@
/// Behavior of arguments when they are encountered while parsing
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) enum ArgAction {
///
/// # Examples
///
/// ```rust
/// # use clap::Command;
/// # use clap::Arg;
/// let cmd = Command::new("mycmd")
/// .arg(
/// Arg::new("special-help")
/// .short('?')
/// .action(clap::builder::ArgAction::Help)
/// );
///
/// // Existing help still exists
/// let err = cmd.clone().try_get_matches_from(["mycmd", "-h"]).unwrap_err();
/// assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp);
///
/// // New help available
/// let err = cmd.try_get_matches_from(["mycmd", "-?"]).unwrap_err();
/// assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp);
/// ```
#[derive(Clone, Debug)]
#[non_exhaustive]
#[allow(missing_copy_implementations)] // In the future, we may accept `Box<dyn ...>`
pub enum ArgAction {
/// When encountered, store the associated value(s) in [`ArgMatches`][crate::ArgMatches]
///
/// # Examples
///
/// ```rust
/// # use clap::Command;
/// # use clap::Arg;
/// let cmd = Command::new("mycmd")
/// .arg(
/// Arg::new("flag")
/// .long("flag")
/// .action(clap::builder::ArgAction::StoreValue)
/// );
///
/// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "value"]).unwrap();
/// assert!(matches.is_present("flag"));
/// assert_eq!(matches.occurrences_of("flag"), 1);
/// assert_eq!(
/// matches.get_many::<String>("flag").unwrap_or_default().map(|v| v.as_str()).collect::<Vec<_>>(),
/// vec!["value"]
/// );
/// ```
StoreValue,
Flag,
/// When encountered, increment [`ArgMatches::occurrences_of`][crate::ArgMatches::occurrences_of]
///
/// No value is allowed
///
/// # Examples
///
/// ```rust
/// # use clap::Command;
/// # use clap::Arg;
/// let cmd = Command::new("mycmd")
/// .arg(
/// Arg::new("flag")
/// .long("flag")
/// .multiple_occurrences(true)
/// .action(clap::builder::ArgAction::IncOccurrence)
/// );
///
/// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
/// assert!(matches.is_present("flag"));
/// assert_eq!(matches.occurrences_of("flag"), 2);
/// assert_eq!(matches.get_many::<String>("flag").unwrap_or_default().count(), 0);
/// ```
IncOccurrence,
/// When encountered, display [`Command::print_help`][super::App::print_help]
///
/// Depending on the flag, [`Command::print_long_help`][super::App::print_long_help] may be shown
///
/// # Examples
///
/// ```rust
/// # use clap::Command;
/// # use clap::Arg;
/// let cmd = Command::new("mycmd")
/// .arg(
/// Arg::new("special-help")
/// .short('?')
/// .action(clap::builder::ArgAction::Help)
/// );
///
/// // Existing help still exists
/// let err = cmd.clone().try_get_matches_from(["mycmd", "-h"]).unwrap_err();
/// assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp);
///
/// // New help available
/// let err = cmd.try_get_matches_from(["mycmd", "-?"]).unwrap_err();
/// assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp);
/// ```
Help,
/// When encountered, display [`Command::version`][super::App::version]
///
/// Depending on the flag, [`Command::long_version`][super::App::long_version] may be shown
///
/// # Examples
///
/// ```rust
/// # use clap::Command;
/// # use clap::Arg;
/// let cmd = Command::new("mycmd")
/// .version("1.0.0")
/// .arg(
/// Arg::new("special-version")
/// .long("special-version")
/// .action(clap::builder::ArgAction::Version)
/// );
///
/// // Existing help still exists
/// let err = cmd.clone().try_get_matches_from(["mycmd", "--version"]).unwrap_err();
/// assert_eq!(err.kind(), clap::error::ErrorKind::DisplayVersion);
///
/// // New help available
/// let err = cmd.try_get_matches_from(["mycmd", "--special-version"]).unwrap_err();
/// assert_eq!(err.kind(), clap::error::ErrorKind::DisplayVersion);
/// ```
Version,
}

impl ArgAction {
pub(crate) fn takes_value(&self) -> bool {
match self {
Self::StoreValue => true,
Self::IncOccurrence => false,
Self::Help => false,
Self::Version => false,
}
}
}
38 changes: 37 additions & 1 deletion src/builder/arg.rs
View file
Open in desktop
Expand Up @@ -1003,6 +1003,35 @@ impl<'help> Arg<'help> {
}
}

/// Specify the behavior when parsing an argument
///
/// # Examples
///
/// ```rust
/// # use clap::Command;
/// # use clap::Arg;
/// let cmd = Command::new("mycmd")
/// .arg(
/// Arg::new("flag")
/// .long("flag")
/// .action(clap::builder::ArgAction::StoreValue)
/// );
///
/// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "value"]).unwrap();
/// assert!(matches.is_present("flag"));
/// assert_eq!(matches.occurrences_of("flag"), 1);
/// assert_eq!(
/// matches.get_many::<String>("flag").unwrap_or_default().map(|v| v.as_str()).collect::<Vec<_>>(),
/// vec!["value"]
/// );
/// ```
#[inline]
#[must_use]
pub fn action(mut self, action: ArgAction) -> Self {
self.action = Some(action);
self
}

/// Specify the type of the argument.
///
/// This allows parsing and validating a value before storing it into
Expand Down Expand Up @@ -4531,7 +4560,7 @@ impl<'help> Arg<'help> {
}

/// Behavior when parsing the argument
pub(crate) fn get_action(&self) -> &super::ArgAction {
pub fn get_action(&self) -> &super::ArgAction {
const DEFAULT: super::ArgAction = super::ArgAction::StoreValue;
self.action.as_ref().unwrap_or(&DEFAULT)
}
Expand Down Expand Up @@ -4862,6 +4891,13 @@ impl<'help> Arg<'help> {
if self.is_positional() {
self.settings.set(ArgSettings::TakesValue);
}
if let Some(action) = self.action.as_ref() {
if action.takes_value() {
self.settings.set(ArgSettings::TakesValue);
} else {
self.settings.unset(ArgSettings::TakesValue);
}
}

if self.value_parser.is_none() {
if self.is_allow_invalid_utf8_set() {
Expand Down
2 changes: 1 addition & 1 deletion src/builder/command.rs
View file
Open in desktop
Expand Up @@ -4159,7 +4159,7 @@ impl<'help> App<'help> {
let action = super::ArgAction::StoreValue;
a.action = Some(action);
} else {
let action = super::ArgAction::Flag;
let action = super::ArgAction::IncOccurrence;
a.action = Some(action);
}
}
Expand Down
13 changes: 13 additions & 0 deletions src/builder/debug_asserts.rs
View file
Open in desktop
Expand Up @@ -642,6 +642,14 @@ fn assert_arg(arg: &Arg) {
arg.name,
);

assert_eq!(
arg.get_action().takes_value(),
arg.is_takes_value_set(),
"Argument `{}`'s selected action {:?} contradicts `takes_value`",
arg.name,
arg.get_action()
);

if arg.get_value_hint() != ValueHint::Unknown {
assert!(
arg.is_takes_value_set(),
Expand All @@ -664,6 +672,11 @@ fn assert_arg(arg: &Arg) {
"Argument '{}' is a positional argument and can't have short or long name versions",
arg.name
);
assert!(
arg.is_takes_value_set(),
"Argument '{}` is positional, it must take a value",
arg.name
);
}

if arg.is_required_set() {
Expand Down
2 changes: 1 addition & 1 deletion src/builder/mod.rs
View file
Open in desktop
Expand Up @@ -24,6 +24,7 @@ mod debug_asserts;
#[cfg(test)]
mod tests;

pub use action::ArgAction;
pub use app_settings::{AppFlags, AppSettings};
pub use arg::Arg;
pub use arg_group::ArgGroup;
Expand Down Expand Up @@ -54,7 +55,6 @@ pub use command::App;
#[cfg(feature = "regex")]
pub use self::regex::RegexRef;

pub(crate) use action::ArgAction;
pub(crate) use arg::display_arg_val;
pub(crate) use arg_predicate::ArgPredicate;
pub(crate) use value_parser::ValueParserInner;
34 changes: 17 additions & 17 deletions src/parser/parser.rs
View file
Open in desktop
Expand Up @@ -347,7 +347,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
// get the option so we can check the settings
let arg_values = matcher.pending_values_mut(id, None);
let arg = &self.cmd[id];
let parse_result = self.push_arg_values(
let parse_result = self.split_arg_values(
arg,
arg_os.to_value_os(),
trailing_values,
Expand Down Expand Up @@ -389,7 +389,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
}
let arg_values = matcher.pending_values_mut(&arg.id, Some(Identifier::Index));
let _parse_result =
self.push_arg_values(arg, arg_os.to_value_os(), trailing_values, arg_values);
self.split_arg_values(arg, arg_os.to_value_os(), trailing_values, arg_values);
if let Some(_parse_result) = _parse_result {
if _parse_result != ParseResult::ValuesDone {
debug!(
Expand Down Expand Up @@ -963,7 +963,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
debug!("Parser::parse_opt_value: has default_missing_vals");
for v in arg.default_missing_vals.iter() {
let trailing_values = false; // CLI should not be affecting default_missing_values
let _parse_result = self.push_arg_values(
let _parse_result = self.split_arg_values(
arg,
&RawOsStr::new(v),
trailing_values,
Expand Down Expand Up @@ -997,7 +997,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
}
} else if let Some(v) = attached_value {
let mut arg_values = Vec::new();
let parse_result = self.push_arg_values(arg, v, trailing_values, &mut arg_values);
let parse_result = self.split_arg_values(arg, v, trailing_values, &mut arg_values);
let react_result = self.react(
Some(ident),
ValueSource::CommandLine,
Expand Down Expand Up @@ -1026,16 +1026,16 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
}
}

fn push_arg_values(
fn split_arg_values(
&self,
arg: &Arg<'help>,
val: &RawOsStr,
trailing_values: bool,
output: &mut Vec<OsString>,
) -> Option<ParseResult> {
debug!("Parser::push_arg_values; arg={}, val={:?}", arg.name, val);
debug!("Parser::split_arg_values; arg={}, val={:?}", arg.name, val);
debug!(
"Parser::push_arg_values; trailing_values={:?}, DontDelimTrailingVals={:?}",
"Parser::split_arg_values; trailing_values={:?}, DontDelimTrailingVals={:?}",
trailing_values,
self.cmd.is_dont_delimit_trailing_values_set()
);
Expand Down Expand Up @@ -1070,13 +1070,13 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
}
}

fn store_arg_values(
fn push_arg_values(
&self,
arg: &Arg<'help>,
raw_vals: Vec<OsString>,
matcher: &mut ArgMatcher,
) -> ClapResult<()> {
debug!("Parser::store_arg_values: {:?}", raw_vals);
debug!("Parser::push_arg_values: {:?}", raw_vals);

for raw_val in raw_vals {
// update the current index because each value is a distinct index to clap
Expand Down Expand Up @@ -1154,7 +1154,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
} else {
self.start_custom_arg(matcher, arg, source);
}
self.store_arg_values(arg, raw_vals, matcher)?;
self.push_arg_values(arg, raw_vals, matcher)?;
if ident == Some(Identifier::Index) && arg.is_multiple_values_set() {
// HACK: Maintain existing occurrence behavior
let matched = matcher.get_mut(&arg.id).unwrap();
Expand All @@ -1167,7 +1167,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
}
Ok(ParseResult::ValuesDone)
}
ArgAction::Flag => {
ArgAction::IncOccurrence => {
debug_assert_eq!(raw_vals, Vec::<OsString>::new());
if source == ValueSource::CommandLine {
if matches!(ident, Some(Identifier::Short) | Some(Identifier::Long)) {
Expand Down Expand Up @@ -1254,7 +1254,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
);
let mut arg_values = Vec::new();
let _parse_result =
self.push_arg_values(arg, &val, trailing_values, &mut arg_values);
self.split_arg_values(arg, &val, trailing_values, &mut arg_values);
let _ = self.react(None, ValueSource::EnvVariable, arg, arg_values, matcher)?;
if let Some(_parse_result) = _parse_result {
if _parse_result != ParseResult::ValuesDone {
Expand All @@ -1264,7 +1264,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
} else {
match arg.get_action() {
ArgAction::StoreValue => unreachable!("{:?} is not a flag", arg.get_id()),
ArgAction::Flag => {
ArgAction::IncOccurrence => {
debug!("Parser::add_env: Found a flag with value `{:?}`", val);
let predicate = str_to_bool(val.to_str_lossy());
debug!("Parser::add_env: Found boolean literal `{:?}`", predicate);
Expand Down Expand Up @@ -1324,7 +1324,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
// The flag occurred, we just want to add the val groups
let mut arg_values = Vec::new();
for v in arg.default_missing_vals.iter() {
let _parse_result = self.push_arg_values(
let _parse_result = self.split_arg_values(
arg,
&RawOsStr::new(v),
trailing_values,
Expand All @@ -1337,7 +1337,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
}
}
self.start_custom_arg(matcher, arg, ValueSource::CommandLine);
self.store_arg_values(arg, arg_values, matcher)?;
self.push_arg_values(arg, arg_values, matcher)?;
}
None => {
debug!("Parser::add_default_value:iter:{}: wasn't used", arg.name);
Expand Down Expand Up @@ -1377,7 +1377,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
if add {
if let Some(default) = default {
let mut arg_values = Vec::new();
let _parse_result = self.push_arg_values(
let _parse_result = self.split_arg_values(
arg,
&RawOsStr::new(default),
trailing_values,
Expand Down Expand Up @@ -1416,7 +1416,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
debug!("Parser::add_default_value:iter:{}: wasn't used", arg.name);
let mut arg_values = Vec::new();
for v in arg.default_vals.iter() {
let _parse_result = self.push_arg_values(
let _parse_result = self.split_arg_values(
arg,
&RawOsStr::new(v),
trailing_values,
Expand Down