Merge pull request #3789 from epage/refactor

refactor: Prep for actions in derive
This commit is contained in:
Ed Page 2022-06-04 13:46:39 -05:00 committed by GitHub
commit 44e1095166
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 207 additions and 43 deletions

View file

@ -144,7 +144,7 @@ jobs:
- name: Check documentation
env:
RUSTDOCFLAGS: -D warnings
run: cargo doc --workspace --all-features --no-deps --document-private-items
run: make doc
rustfmt:
name: rustfmt
runs-on: ubuntu-latest

View file

@ -35,3 +35,6 @@ clippy-%:
test-ui-%:
cargo +${MSRV} test --test derive_ui --features derive ${_FEATURES_${@:test-ui-%=%}}
doc:
cargo doc --workspace --all-features --no-deps --document-private-items

View file

@ -9,7 +9,7 @@
/// .arg(
/// Arg::new("special-help")
/// .short('?')
/// .action(clap::builder::ArgAction::Help)
/// .action(clap::ArgAction::Help)
/// );
///
/// // Existing help still exists
@ -35,7 +35,7 @@ pub enum ArgAction {
/// .arg(
/// Arg::new("flag")
/// .long("flag")
/// .action(clap::builder::ArgAction::Set)
/// .action(clap::ArgAction::Set)
/// );
///
/// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "value"]).unwrap();
@ -59,7 +59,7 @@ pub enum ArgAction {
/// Arg::new("flag")
/// .long("flag")
/// .multiple_occurrences(true)
/// .action(clap::builder::ArgAction::Append)
/// .action(clap::ArgAction::Append)
/// );
///
/// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "value1", "--flag", "value2"]).unwrap();
@ -82,7 +82,7 @@ pub enum ArgAction {
/// .arg(
/// Arg::new("flag")
/// .long("flag")
/// .action(clap::builder::ArgAction::StoreValue)
/// .action(clap::ArgAction::StoreValue)
/// );
///
/// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "value"]).unwrap();
@ -108,7 +108,7 @@ pub enum ArgAction {
/// Arg::new("flag")
/// .long("flag")
/// .multiple_occurrences(true)
/// .action(clap::builder::ArgAction::IncOccurrence)
/// .action(clap::ArgAction::IncOccurrence)
/// );
///
/// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
@ -132,7 +132,7 @@ pub enum ArgAction {
/// .arg(
/// Arg::new("flag")
/// .long("flag")
/// .action(clap::builder::ArgAction::SetTrue)
/// .action(clap::ArgAction::SetTrue)
/// );
///
/// let matches = cmd.clone().try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
@ -167,7 +167,7 @@ pub enum ArgAction {
/// .arg(
/// Arg::new("flag")
/// .long("flag")
/// .action(clap::builder::ArgAction::SetFalse)
/// .action(clap::ArgAction::SetFalse)
/// );
///
/// let matches = cmd.clone().try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
@ -202,7 +202,7 @@ pub enum ArgAction {
/// .arg(
/// Arg::new("flag")
/// .long("flag")
/// .action(clap::builder::ArgAction::Count)
/// .action(clap::ArgAction::Count)
/// );
///
/// let matches = cmd.clone().try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
@ -235,7 +235,7 @@ pub enum ArgAction {
/// .arg(
/// Arg::new("special-help")
/// .short('?')
/// .action(clap::builder::ArgAction::Help)
/// .action(clap::ArgAction::Help)
/// );
///
/// // Existing help still exists
@ -261,7 +261,7 @@ pub enum ArgAction {
/// .arg(
/// Arg::new("special-version")
/// .long("special-version")
/// .action(clap::builder::ArgAction::Version)
/// .action(clap::ArgAction::Version)
/// );
///
/// // Existing help still exists

View file

@ -18,9 +18,9 @@ use yaml_rust::Yaml;
// Internal
use crate::builder::usage_parser::UsageParser;
use crate::builder::ArgAction;
use crate::builder::ArgPredicate;
use crate::util::{Id, Key};
use crate::ArgAction;
use crate::PossibleValue;
use crate::ValueHint;
use crate::INTERNAL_ERROR_MSG;
@ -1014,7 +1014,7 @@ impl<'help> Arg<'help> {
/// .arg(
/// Arg::new("flag")
/// .long("flag")
/// .action(clap::builder::ArgAction::StoreValue)
/// .action(clap::ArgAction::StoreValue)
/// );
///
/// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "value"]).unwrap();

View file

@ -1252,7 +1252,7 @@ impl BoolValueParser {
["true", "false"]
.iter()
.copied()
.map(|l| crate::PossibleValue::new(l).hide(true))
.map(|l| crate::PossibleValue::new(l))
}
}
@ -1352,7 +1352,7 @@ impl FalseyValueParser {
.iter()
.chain(crate::util::FALSE_LITERALS.iter())
.copied()
.map(|l| crate::PossibleValue::new(l).hide(true))
.map(|l| crate::PossibleValue::new(l).hide(l != "true" && l != "false"))
}
}
@ -1449,7 +1449,7 @@ impl BoolishValueParser {
.iter()
.chain(crate::util::FALSE_LITERALS.iter())
.copied()
.map(|l| crate::PossibleValue::new(l).hide(true))
.map(|l| crate::PossibleValue::new(l).hide(l != "true" && l != "false"))
}
}

View file

@ -26,6 +26,7 @@
#[cfg(not(feature = "std"))]
compile_error!("`std` feature is currently required to build `clap`");
pub use crate::builder::ArgAction;
pub use crate::builder::Command;
pub use crate::builder::{Arg, ArgGroup};
pub use crate::error::Error;

View file

@ -601,7 +601,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
spec_vals.push(env_info);
}
}
if !a.is_hide_default_value_set() && !a.default_vals.is_empty() {
if a.is_takes_value_set() && !a.is_hide_default_value_set() && !a.default_vals.is_empty() {
debug!(
"Help::spec_vals: Found default value...[{:?}]",
a.default_vals

View file

@ -116,7 +116,7 @@ impl ArgMatcher {
self.matches.args.contains_key(arg)
}
pub(crate) fn arg_names(&self) -> indexmap::map::Keys<Id, MatchedArg> {
pub(crate) fn arg_ids(&self) -> indexmap::map::Keys<Id, MatchedArg> {
self.matches.args.keys()
}

View file

@ -9,7 +9,6 @@ use clap_lex::RawOsStr;
// Internal
use crate::builder::AppSettings as AS;
use crate::builder::ArgAction;
use crate::builder::{Arg, Command};
use crate::error::Error as ClapError;
use crate::error::Result as ClapResult;
@ -20,6 +19,7 @@ use crate::parser::features::suggestions;
use crate::parser::{ArgMatcher, SubCommand};
use crate::parser::{Validator, ValueSource};
use crate::util::Id;
use crate::ArgAction;
use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
pub(crate) struct Parser<'help, 'cmd> {
@ -771,7 +771,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
long_arg, rest
);
let used: Vec<Id> = matcher
.arg_names()
.arg_ids()
.filter(|arg_id| {
matcher.check_explicit(arg_id, crate::builder::ArgPredicate::IsPresent)
})
.filter(|&n| {
self.cmd
.find(n)
@ -1312,7 +1315,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
// Override anything that can override us
let mut transitive = Vec::new();
for arg_id in matcher.arg_names() {
for arg_id in matcher.arg_ids() {
if let Some(overrider) = self.cmd.find(arg_id) {
if overrider.overrides.contains(&arg.id) {
transitive.push(&overrider.id);
@ -1612,7 +1615,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
let required = self.cmd.required_graph();
let used: Vec<Id> = matcher
.arg_names()
.arg_ids()
.filter(|arg_id| {
matcher.check_explicit(arg_id, crate::builder::ArgPredicate::IsPresent)
})
.filter(|n| self.cmd.find(n).map_or(true, |a| !a.is_hide_set()))
.cloned()
.collect();

View file

@ -52,7 +52,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
if !has_subcmd && self.cmd.is_arg_required_else_help_set() {
let num_user_values = matcher
.arg_names()
.arg_ids()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.count();
if num_user_values == 0 {
@ -179,7 +179,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
self.validate_exclusive(matcher)?;
for arg_id in matcher
.arg_names()
.arg_ids()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|arg_id| self.cmd.find(arg_id).is_some())
{
@ -193,15 +193,22 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
fn validate_exclusive(&self, matcher: &ArgMatcher) -> ClapResult<()> {
debug!("Validator::validate_exclusive");
// Not bothering to filter for `check_explicit` since defaults shouldn't play into this
let args_count = matcher.arg_names().count();
let args_count = matcher
.arg_ids()
.filter(|arg_id| {
matcher.check_explicit(arg_id, crate::builder::ArgPredicate::IsPresent)
})
.count();
if args_count <= 1 {
// Nothing present to conflict with
return Ok(());
}
matcher
.arg_names()
.arg_ids()
.filter(|arg_id| {
matcher.check_explicit(arg_id, crate::builder::ArgPredicate::IsPresent)
})
.filter_map(|name| {
debug!("Validator::validate_exclusive:iter:{:?}", name);
self.cmd
@ -263,7 +270,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
fn build_conflict_err_usage(&self, matcher: &ArgMatcher, conflicting_keys: &[Id]) -> String {
let used_filtered: Vec<Id> = matcher
.arg_names()
.arg_ids()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|n| {
// Filter out the args we don't want to specify.
@ -288,7 +295,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
fn gather_requires(&mut self, matcher: &ArgMatcher) {
debug!("Validator::gather_requires");
for name in matcher
.arg_names()
.arg_ids()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
{
debug!("Validator::gather_requires:iter:{:?}", name);
@ -455,12 +462,15 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
debug!("Validator::validate_required: required={:?}", self.required);
self.gather_requires(matcher);
let is_exclusive_present = matcher.arg_names().any(|name| {
self.cmd
.find(name)
.map(|arg| arg.is_exclusive_set())
.unwrap_or_default()
});
let is_exclusive_present = matcher
.arg_ids()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.any(|id| {
self.cmd
.find(id)
.map(|arg| arg.is_exclusive_set())
.unwrap_or_default()
});
debug!(
"Validator::validate_required: is_exclusive_present={}",
is_exclusive_present
@ -569,7 +579,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
);
let used: Vec<Id> = matcher
.arg_names()
.arg_ids()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|n| {
// Filter out the args we don't want to specify.
@ -601,7 +611,7 @@ impl Conflicts {
debug!("Conflicts::gather_conflicts: arg={:?}", arg_id);
let mut conflicts = Vec::new();
for other_arg_id in matcher
.arg_names()
.arg_ids()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
{
if arg_id == other_arg_id {

View file

@ -1,7 +1,7 @@
#![allow(clippy::bool_assert_comparison)]
use clap::builder::ArgAction;
use clap::Arg;
use clap::ArgAction;
use clap::Command;
#[test]

View file

@ -62,7 +62,7 @@ fn flag_conflict_with_all() {
}
#[test]
fn flag_conflict_with_everything() {
fn exclusive_flag() {
let result = Command::new("flag_conflict")
.arg(arg!(-f --flag "some flag").exclusive(true))
.arg(arg!(-o --other "some flag"))
@ -72,6 +72,56 @@ fn flag_conflict_with_everything() {
assert_eq!(err.kind(), ErrorKind::ArgumentConflict);
}
#[test]
fn exclusive_option() {
let result = Command::new("flag_conflict")
.arg(
arg!(-f --flag <VALUE> "some flag")
.required(false)
.exclusive(true),
)
.arg(arg!(-o --other <VALUE> "some flag").required(false))
.try_get_matches_from(vec!["myprog", "-o=val1", "-f=val2"]);
assert!(result.is_err());
let err = result.err().unwrap();
assert_eq!(err.kind(), ErrorKind::ArgumentConflict);
}
#[test]
fn not_exclusive_with_defaults() {
let result = Command::new("flag_conflict")
.arg(
arg!(-f --flag <VALUE> "some flag")
.required(false)
.exclusive(true),
)
.arg(
arg!(-o --other <VALUE> "some flag")
.required(false)
.default_value("val1"),
)
.try_get_matches_from(vec!["myprog", "-f=val2"]);
assert!(result.is_ok(), "{}", result.unwrap_err());
}
#[test]
fn default_doesnt_activate_exclusive() {
let result = Command::new("flag_conflict")
.arg(
arg!(-f --flag <VALUE> "some flag")
.required(false)
.exclusive(true)
.default_value("val2"),
)
.arg(
arg!(-o --other <VALUE> "some flag")
.required(false)
.default_value("val1"),
)
.try_get_matches_from(vec!["myprog"]);
assert!(result.is_ok(), "{}", result.unwrap_err());
}
#[test]
fn arg_conflicts_with_group() {
let mut cmd = Command::new("group_conflict")

View file

@ -24,6 +24,7 @@ fn opt_missing() {
m.value_source("color").unwrap(),
clap::ValueSource::DefaultValue
);
assert_eq!(m.index_of("color"), Some(1));
}
#[test]
@ -50,6 +51,7 @@ fn opt_present_with_missing_value() {
m.value_source("color").unwrap(),
clap::ValueSource::CommandLine
);
assert_eq!(m.index_of("color"), Some(2));
}
#[test]
@ -76,6 +78,7 @@ fn opt_present_with_value() {
m.value_source("color").unwrap(),
clap::ValueSource::CommandLine
);
assert_eq!(m.index_of("color"), Some(2));
}
#[test]
@ -101,6 +104,7 @@ fn opt_present_with_empty_value() {
m.value_source("color").unwrap(),
clap::ValueSource::CommandLine
);
assert_eq!(m.index_of("color"), Some(2));
}
//## `default_value`/`default_missing_value` non-interaction checks
@ -271,3 +275,32 @@ fn default_missing_values_are_valid() {
)
.try_get_matches();
}
#[test]
fn valid_index() {
let m = Command::new("df")
.arg(
Arg::new("color")
.long("color")
.default_value("auto")
.min_values(0)
.require_equals(true)
.default_missing_value("always"),
)
.arg(Arg::new("sync").long("sync"))
.try_get_matches_from(vec!["df", "--color", "--sync"])
.unwrap();
assert!(m.is_present("color"));
assert_eq!(
m.get_one::<String>("color").map(|v| v.as_str()).unwrap(),
"always"
);
assert_eq!(m.occurrences_of("color"), 1);
assert_eq!(
m.value_source("color").unwrap(),
clap::ValueSource::CommandLine
);
// Make sure the index reflects `--color`s position and not something else
assert_eq!(m.index_of("color"), Some(2));
}

View file

@ -19,6 +19,20 @@ fn opts() {
);
}
#[test]
fn default_has_index() {
let r = Command::new("df")
.arg(
arg!(o: -o <opt> "some opt")
.required(false)
.default_value("default"),
)
.try_get_matches_from(vec![""]);
assert!(r.is_ok(), "{}", r.unwrap_err());
let m = r.unwrap();
assert_eq!(m.index_of("o"), Some(1));
}
#[test]
fn opt_without_value_fail() {
let r = Command::new("df")

View file

@ -363,7 +363,7 @@ OPTIONS:
let mut buffer: Vec<u8> = Default::default();
cmd.write_help(&mut buffer).unwrap();
let help = String::from_utf8(buffer).unwrap();
assert_eq!(help, HELP);
snapbox::assert_eq(HELP, help);
}
#[test]
@ -420,7 +420,7 @@ OPTIONS:
let mut buffer: Vec<u8> = Default::default();
cmd.write_help(&mut buffer).unwrap();
let help = String::from_utf8(buffer).unwrap();
assert_eq!(help, HELP);
snapbox::assert_eq(HELP, help);
}
#[test]
@ -476,5 +476,52 @@ OPTIONS:
let mut buffer: Vec<u8> = Default::default();
cmd.write_help(&mut buffer).unwrap();
let help = String::from_utf8(buffer).unwrap();
assert_eq!(help, HELP);
snapbox::assert_eq(HELP, help);
}
#[test]
#[cfg(feature = "unstable-v4")]
fn derive_possible_value_help() {
static HELP: &str = "clap
Application help
USAGE:
clap <ARG>
ARGS:
<ARG>
Argument help
Possible values:
- foo: Foo help
- bar: Bar help
OPTIONS:
-h, --help
Print help information
";
/// Application help
#[derive(Parser, PartialEq, Debug)]
struct Args {
/// Argument help
#[clap(arg_enum, value_parser)]
arg: ArgChoice,
}
#[derive(clap::ArgEnum, PartialEq, Debug, Clone)]
enum ArgChoice {
/// Foo help
Foo,
/// Bar help
Bar,
}
use clap::CommandFactory;
let mut cmd = Args::command();
let mut buffer: Vec<u8> = Default::default();
cmd.write_long_help(&mut buffer).unwrap();
let help = String::from_utf8(buffer).unwrap();
snapbox::assert_eq(HELP, help);
}

View file

@ -17,6 +17,6 @@ error[E0599]: no function or associated item named `parse` found for struct `Opt
|
= help: items from traits can only be used if the trait is implemented and in scope
= note: the following traits define an item `parse`, perhaps you need to implement one of them:
candidate #1: `Parser`
candidate #1: `StructOpt`
candidate #2: `AnyValueParser`
candidate #3: `TypedValueParser`