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

refactor: Prep for actions in derive #3789

Merged
merged 11 commits into from
Jun 4, 2022
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 10 additions & 10 deletions src/builder/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/// .arg(
/// Arg::new("special-help")
/// .short('?')
/// .action(clap::builder::ArgAction::Help)
/// .action(clap::ArgAction::Help)
/// );
///
/// // Existing help still exists
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/builder/arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions src/builder/value_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,7 +1252,7 @@ impl BoolValueParser {
["true", "false"]
.iter()
.copied()
.map(|l| crate::PossibleValue::new(l).hide(true))
.map(|l| crate::PossibleValue::new(l))
}
}

Expand Down Expand Up @@ -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"))
}
}

Expand Down Expand Up @@ -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"))
}
}

Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/output/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/parser/arg_matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
14 changes: 10 additions & 4 deletions src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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> {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
40 changes: 25 additions & 15 deletions src/parser/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
{
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/builder/action.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![allow(clippy::bool_assert_comparison)]

use clap::builder::ArgAction;
use clap::Arg;
use clap::ArgAction;
use clap::Command;

#[test]
Expand Down
Loading