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(help): Deprecate NoAutoVersion/NoAutoHelp #3800

Merged
merged 3 commits into from Jun 8, 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
43 changes: 4 additions & 39 deletions src/builder/app_settings.rs
Expand Up @@ -293,47 +293,12 @@ pub enum AppSettings {
#[deprecated(since = "3.1.0", note = "Replaced with `Command::no_binary_name`")]
NoBinaryName,

/// Treat the auto-generated `-h, --help` flags like any other flag, and *not* print the help
/// message.
///
/// This allows one to handle printing of the help message manually.
///
/// ```rust
/// # use clap::{Command, AppSettings};
/// let result = Command::new("myprog")
/// .setting(AppSettings::NoAutoHelp)
/// .try_get_matches_from("myprog --help".split(" "));
///
/// // Normally, if `--help` is used clap prints the help message and returns an
/// // ErrorKind::DisplayHelp
/// //
/// // However, `--help` was treated like a normal flag
///
/// assert!(result.is_ok());
/// assert!(result.unwrap().is_present("help"));
/// ```
/// Deprecated, replaced with [`Arg::action`][super::Arg::action]
#[deprecated(since = "3.2.0", note = "Replaced with `Arg::action`")]
NoAutoHelp,

/// Treat the auto-generated `-V, --version` flags like any other flag, and
/// *not* print the version message.
///
/// This allows one to handle printing of the version message manually.
///
/// ```rust
/// # use clap::{Command, AppSettings};
/// let result = Command::new("myprog")
/// .version("3.0")
/// .setting(AppSettings::NoAutoVersion)
/// .try_get_matches_from("myprog --version".split(" "));
///
/// // Normally, if `--version` is used clap prints the version message and returns an
/// // ErrorKind::DisplayVersion
/// //
/// // However, `--version` was treated like a normal flag
///
/// assert!(result.is_ok());
/// assert!(result.unwrap().is_present("version"));
/// ```
/// Deprecated, replaced with [`Arg::action`][super::Arg::action]
#[deprecated(since = "3.2.0", note = "Replaced with `Arg::action`")]
NoAutoVersion,

/// Deprecated, replaced with [`AppSettings::AllowHyphenValues`]
Expand Down
2 changes: 1 addition & 1 deletion src/builder/command.rs
Expand Up @@ -4373,7 +4373,7 @@ impl<'help> App<'help> {
.collect();

assert!(args_missing_help.is_empty(),
"AppSettings::HelpExpected is enabled for the Command {}, but at least one of its arguments does not have either `help` or `long_help` set. List of such arguments: {}",
"Command::help_expected is enabled for the Command {}, but at least one of its arguments does not have either `help` or `long_help` set. List of such arguments: {}",
self.name,
args_missing_help.join(", ")
);
Expand Down
23 changes: 14 additions & 9 deletions src/builder/debug_asserts.rs
Expand Up @@ -4,8 +4,8 @@ use clap_lex::RawOsStr;

use crate::builder::arg::ArgProvider;
use crate::mkeymap::KeyType;
use crate::util::Id;
use crate::{AppSettings, Arg, Command, ValueHint};
use crate::ArgAction;
use crate::{Arg, Command, ValueHint};

pub(crate) fn assert_app(cmd: &Command) {
debug!("Command::_debug_asserts");
Expand All @@ -23,16 +23,21 @@ pub(crate) fn assert_app(cmd: &Command) {
);

// Used `Command::mut_arg("version", ..) but did not provide any version information to display
let has_mutated_version = cmd
let version_needed = cmd
.get_arguments()
.any(|x| x.id == Id::version_hash() && x.provider == ArgProvider::GeneratedMutated);
.filter(|x| {
matches!(x.get_action(), ArgAction::Version)
&& matches!(
x.provider,
ArgProvider::User | ArgProvider::GeneratedMutated
)
})
.map(|x| x.get_id())
.collect::<Vec<_>>();

if has_mutated_version {
assert!(cmd.is_set(AppSettings::NoAutoVersion),
"Command {}: Used Command::mut_arg(\"version\", ..) without providing Command::version, Command::long_version or using AppSettings::NoAutoVersion"
assert_eq!(version_needed, Vec::<&str>::new(), "Command {}: `ArgAction::Version` used without providing Command::version or Command::long_version"
,cmd.get_name()
);
}
);
}

for sc in cmd.get_subcommands() {
Expand Down
1 change: 1 addition & 0 deletions src/parser/parser.rs
Expand Up @@ -178,6 +178,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
let sc_name = self.possible_subcommand(arg_os.to_value(), valid_arg_found);
debug!("Parser::get_matches_with: sc={:?}", sc_name);
if let Some(sc_name) = sc_name {
#[allow(deprecated)]
if sc_name == "help"
&& !self.is_set(AS::NoAutoHelp)
&& !self.cmd.is_disable_help_subcommand_set()
Expand Down
44 changes: 25 additions & 19 deletions tests/builder/app_settings.rs
Expand Up @@ -2,7 +2,7 @@ use std::ffi::OsString;

use super::utils;

use clap::{arg, error::ErrorKind, AppSettings, Arg, Command};
use clap::{arg, error::ErrorKind, AppSettings, Arg, ArgAction, Command};

static ALLOW_EXT_SC: &str = "clap-test v1.4.8

Expand Down Expand Up @@ -1345,62 +1345,68 @@ fn aaos_option_use_delim_false() {
#[test]
fn no_auto_help() {
let cmd = Command::new("myprog")
.setting(AppSettings::NoAutoHelp)
.subcommand(Command::new("foo"));
.subcommand(Command::new("foo"))
.mut_arg("help", |v| v.action(ArgAction::SetTrue));

let result = cmd.clone().try_get_matches_from("myprog --help".split(' '));

assert!(result.is_ok(), "{}", result.unwrap_err());
assert!(result.unwrap().is_present("help"));
assert_eq!(result.unwrap().get_one::<bool>("help").copied(), Some(true));

let result = cmd.clone().try_get_matches_from("myprog -h".split(' '));

assert!(result.is_ok(), "{}", result.unwrap_err());
assert!(result.unwrap().is_present("help"));

let result = cmd.clone().try_get_matches_from("myprog help".split(' '));

assert!(result.is_ok(), "{}", result.unwrap_err());
assert_eq!(result.unwrap().subcommand_name(), Some("help"));
assert_eq!(result.unwrap().get_one::<bool>("help").copied(), Some(true));
}

#[test]
fn no_auto_version() {
let cmd = Command::new("myprog")
.version("3.0")
.setting(AppSettings::NoAutoVersion);
.mut_arg("version", |v| v.action(ArgAction::SetTrue));

let result = cmd
.clone()
.try_get_matches_from("myprog --version".split(' '));

assert!(result.is_ok(), "{}", result.unwrap_err());
assert!(result.unwrap().is_present("version"));
assert_eq!(
result.unwrap().get_one::<bool>("version").copied(),
Some(true)
);

let result = cmd.clone().try_get_matches_from("myprog -V".split(' '));

assert!(result.is_ok(), "{}", result.unwrap_err());
assert!(result.unwrap().is_present("version"));
assert_eq!(
result.unwrap().get_one::<bool>("version").copied(),
Some(true)
);
}

#[test]
fn no_auto_version_mut_arg() {
let cmd = Command::new("myprog")
.version("3.0")
.mut_arg("version", |v| v.help("custom help"))
.setting(AppSettings::NoAutoVersion);
.mut_arg("version", |v| {
v.action(ArgAction::SetTrue).help("custom help")
});

let result = cmd
.clone()
.try_get_matches_from("myprog --version".split(' '));

assert!(result.is_ok(), "{}", result.unwrap_err());
assert!(result.unwrap().is_present("version"));
assert_eq!(
result.unwrap().get_one::<bool>("version").copied(),
Some(true)
);

let result = cmd.clone().try_get_matches_from("myprog -V".split(' '));

assert!(result.is_ok(), "{}", result.unwrap_err());
assert!(result.unwrap().is_present("version"));
assert_eq!(
result.unwrap().get_one::<bool>("version").copied(),
Some(true)
);
}

#[test]
Expand Down
8 changes: 4 additions & 4 deletions tests/builder/help.rs
Expand Up @@ -1997,7 +1997,7 @@ fn issue_1487() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "AppSettings::HelpExpected is enabled for the Command"]
#[should_panic = "Command::help_expected is enabled for the Command"]
fn help_required_but_not_given() {
Command::new("myapp")
.help_expected(true)
Expand All @@ -2008,7 +2008,7 @@ fn help_required_but_not_given() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "AppSettings::HelpExpected is enabled for the Command"]
#[should_panic = "Command::help_expected is enabled for the Command"]
fn help_required_but_not_given_settings_after_args() {
Command::new("myapp")
.arg(Arg::new("foo"))
Expand All @@ -2019,7 +2019,7 @@ fn help_required_but_not_given_settings_after_args() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "AppSettings::HelpExpected is enabled for the Command"]
#[should_panic = "Command::help_expected is enabled for the Command"]
fn help_required_but_not_given_for_one_of_two_arguments() {
Command::new("myapp")
.help_expected(true)
Expand All @@ -2046,7 +2046,7 @@ fn help_required_globally() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "AppSettings::HelpExpected is enabled for the Command"]
#[should_panic = "Command::help_expected is enabled for the Command"]
fn help_required_globally_but_not_given_for_subcommand() {
Command::new("myapp")
.help_expected(true)
Expand Down
8 changes: 4 additions & 4 deletions tests/builder/legacy/help.rs
Expand Up @@ -1991,7 +1991,7 @@ fn issue_1487() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "AppSettings::HelpExpected is enabled for the Command"]
#[should_panic = "Command::help_expected is enabled for the Command"]
fn help_required_but_not_given() {
Command::new("myapp")
.help_expected(true)
Expand All @@ -2002,7 +2002,7 @@ fn help_required_but_not_given() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "AppSettings::HelpExpected is enabled for the Command"]
#[should_panic = "Command::help_expected is enabled for the Command"]
fn help_required_but_not_given_settings_after_args() {
Command::new("myapp")
.arg(Arg::new("foo"))
Expand All @@ -2013,7 +2013,7 @@ fn help_required_but_not_given_settings_after_args() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "AppSettings::HelpExpected is enabled for the Command"]
#[should_panic = "Command::help_expected is enabled for the Command"]
fn help_required_but_not_given_for_one_of_two_arguments() {
Command::new("myapp")
.help_expected(true)
Expand All @@ -2040,7 +2040,7 @@ fn help_required_globally() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "AppSettings::HelpExpected is enabled for the Command"]
#[should_panic = "Command::help_expected is enabled for the Command"]
fn help_required_globally_but_not_given_for_subcommand() {
Command::new("myapp")
.help_expected(true)
Expand Down
2 changes: 1 addition & 1 deletion tests/builder/legacy/version.rs
Expand Up @@ -206,7 +206,7 @@ fn propagate_version_short() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "Used Command::mut_arg(\"version\", ..) without providing Command::version, Command::long_version or using AppSettings::NoAutoVersion"]
#[should_panic = "`ArgAction::Version` used without providing Command::version or Command::long_version"]
fn mut_arg_version_panic() {
let _res = common()
.mut_arg("version", |v| v.short('z'))
Expand Down
9 changes: 4 additions & 5 deletions tests/builder/version.rs
Expand Up @@ -2,7 +2,7 @@ use super::utils;

use std::str;

use clap::{error::ErrorKind, AppSettings, Arg, Command};
use clap::{error::ErrorKind, Arg, ArgAction, Command};

fn common() -> Command<'static> {
Command::new("foo")
Expand Down Expand Up @@ -206,7 +206,7 @@ fn propagate_version_short() {

#[cfg(debug_assertions)]
#[test]
#[should_panic = "Used Command::mut_arg(\"version\", ..) without providing Command::version, Command::long_version or using AppSettings::NoAutoVersion"]
#[should_panic = "`ArgAction::Version` used without providing Command::version or Command::long_version"]
fn mut_arg_version_panic() {
let _res = common()
.mut_arg("version", |v| v.short('z'))
Expand All @@ -216,12 +216,11 @@ fn mut_arg_version_panic() {
#[test]
fn mut_arg_version_no_auto_version() {
let res = common()
.mut_arg("version", |v| v.short('z'))
.setting(AppSettings::NoAutoVersion)
.mut_arg("version", |v| v.short('z').action(ArgAction::SetTrue))
.try_get_matches_from("foo -z".split(' '));

assert!(res.is_ok(), "{}", res.unwrap_err());
assert!(res.unwrap().is_present("version"));
assert_eq!(res.unwrap().get_one::<bool>("version").copied(), Some(true));
}

#[cfg(debug_assertions)]
Expand Down