Skip to content

Commit

Permalink
fix(derive): Move off of SubcommandRequiredElseHelp
Browse files Browse the repository at this point in the history
This also let us remove the deprecated attribute

Fixes clap-rs#3280
  • Loading branch information
epage committed Jul 22, 2022
1 parent 50019ca commit 8b064cf
Show file tree
Hide file tree
Showing 11 changed files with 22 additions and 88 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- `ErrorKind::EmptyValue` replaced with `ErrorKind::InvalidValue`
- `ErrorKind::UnrecognizedSubcommand` replaced with `ErrorKind::InvalidSubcommand`
- *(derive)* `subcommand_required(true).arg_required_else_help(true)` is set instead of `SubcommandRequiredElseHelp`

### Features

Expand Down
5 changes: 0 additions & 5 deletions clap_bench/benches/06_rustup.rs
Expand Up @@ -27,7 +27,6 @@ fn build_cli() -> Command<'static> {
.about("The Rust toolchain installer")
.after_help(RUSTUP_HELP)
.setting(AppSettings::DeriveDisplayOrder)
// .setting(AppSettings::SubcommandRequiredElseHelp)
.arg(
Arg::new("verbose")
.help("Enable verbose output")
Expand Down Expand Up @@ -69,7 +68,6 @@ fn build_cli() -> Command<'static> {
.about("Modify or query the installed toolchains")
.after_help(TOOLCHAIN_HELP)
.setting(AppSettings::DeriveDisplayOrder)
// .setting(AppSettings::SubcommandRequiredElseHelp)
.subcommand(Command::new("list").about("List installed toolchains"))
.subcommand(
Command::new("install")
Expand Down Expand Up @@ -107,7 +105,6 @@ fn build_cli() -> Command<'static> {
Command::new("target")
.about("Modify a toolchain's supported targets")
.setting(AppSettings::DeriveDisplayOrder)
// .setting(AppSettings::SubcommandRequiredElseHelp)
.subcommand(
Command::new("list")
.about("List installed and available targets")
Expand Down Expand Up @@ -142,7 +139,6 @@ fn build_cli() -> Command<'static> {
Command::new("component")
.about("Modify a toolchain's installed components")
.setting(AppSettings::DeriveDisplayOrder)
// .setting(AppSettings::SubcommandRequiredElseHelp)
.subcommand(
Command::new("list")
.about("List installed and available components")
Expand All @@ -168,7 +164,6 @@ fn build_cli() -> Command<'static> {
.about("Modify directory toolchain overrides")
.after_help(OVERRIDE_HELP)
.setting(AppSettings::DeriveDisplayOrder)
// .setting(AppSettings::SubcommandRequiredElseHelp)
.subcommand(Command::new("list").about("List directory toolchain overrides"))
.subcommand(
Command::new("set")
Expand Down
7 changes: 3 additions & 4 deletions clap_derive/src/derives/args.rs
Expand Up @@ -179,10 +179,9 @@ pub fn gen_augment(
quote!()
} else {
quote_spanned! { kind.span()=>
#[allow(deprecated)]
let #app_var = #app_var.setting(
clap::AppSettings::SubcommandRequiredElseHelp
);
let #app_var = #app_var
.subcommand_required(true)
.arg_required_else_help(true);
}
};

Expand Down
4 changes: 2 additions & 2 deletions clap_derive/src/derives/into_app.rs
Expand Up @@ -103,9 +103,9 @@ pub fn gen_for_enum(enum_name: &Ident, generics: &Generics, attrs: &[Attribute])
#[deny(clippy::correctness)]
impl #impl_generics clap::CommandFactory for #enum_name #ty_generics #where_clause {
fn command<'b>() -> clap::Command<'b> {
#[allow(deprecated)]
let #app_var = clap::Command::new(#name)
.setting(clap::AppSettings::SubcommandRequiredElseHelp);
.subcommand_required(true)
.arg_required_else_help(true);
<Self as clap::Subcommand>::augment_subcommands(#app_var)
}

Expand Down
5 changes: 3 additions & 2 deletions clap_derive/src/derives/subcommand.rs
Expand Up @@ -256,8 +256,9 @@ fn gen_augment(
let #subcommand_var = clap::Command::new(#name);
let #subcommand_var = #subcommand_var #initial_app_methods;
let #subcommand_var = #arg_block;
#[allow(deprecated)]
let #subcommand_var = #subcommand_var.setting(clap::AppSettings::SubcommandRequiredElseHelp);
let #subcommand_var = #subcommand_var
.subcommand_required(true)
.arg_required_else_help(true);
#subcommand_var #final_from_attrs
});
};
Expand Down
4 changes: 1 addition & 3 deletions clap_mangen/src/render.rs
@@ -1,4 +1,3 @@
use clap::AppSettings;
use roff::{bold, italic, roman, Inline, Roff};

pub(crate) fn subcommand_heading(cmd: &clap::Command) -> String {
Expand Down Expand Up @@ -188,8 +187,7 @@ pub(crate) fn after_help(roff: &mut Roff, cmd: &clap::Command) {
}

fn subcommand_markers(cmd: &clap::Command) -> (&'static str, &'static str) {
#[allow(deprecated)]
markers(cmd.is_subcommand_required_set() || cmd.is_set(AppSettings::SubcommandRequiredElseHelp))
markers(cmd.is_subcommand_required_set())
}

fn option_markers(opt: &clap::Arg) -> (&'static str, &'static str) {
Expand Down
11 changes: 9 additions & 2 deletions examples/derive_ref/interop_tests.md
Expand Up @@ -101,12 +101,19 @@ For more information try --help
```console
$ interop_hand_subcommand
? failed
error: 'interop_hand_subcommand[EXE]' requires a subcommand but one was not provided
clap

USAGE:
interop_hand_subcommand[EXE] [OPTIONS] <SUBCOMMAND>

For more information try --help
OPTIONS:
-h, --help Print help information
-t, --top-level

SUBCOMMANDS:
add
help Print this message or the help of the given subcommand(s)
remove

```

Expand Down
14 changes: 0 additions & 14 deletions src/builder/app_settings.rs
Expand Up @@ -116,17 +116,6 @@ pub enum AppSettings {
)]
SubcommandRequired,

/// Deprecated, replaced with [`Command::subcommand_required`] combined with
/// [`Command::arg_required_else_help`].
#[cfg_attr(
feature = "deprecated",
deprecated(
since = "3.1.0",
note = "Replaced with `Command::subcommand_required` combined with `Command::arg_required_else_help`"
)
)]
SubcommandRequiredElseHelp,

/// Deprecated, replaced with [`Command::allow_external_subcommands`] and
/// [`Command::is_allow_external_subcommands_set`]
#[cfg_attr(
Expand Down Expand Up @@ -392,7 +381,6 @@ bitflags! {
const PROPAGATE_VERSION = 1 << 3;
const DISABLE_VERSION_FOR_SC = 1 << 4;
const WAIT_ON_ERROR = 1 << 6;
const SC_REQUIRED_ELSE_HELP = 1 << 7;
const NO_AUTO_HELP = 1 << 8;
const NO_AUTO_VERSION = 1 << 9;
const DISABLE_VERSION_FLAG = 1 << 10;
Expand Down Expand Up @@ -490,8 +478,6 @@ impl_settings! { AppSettings, AppFlags,
=> Flags::SC_NEGATE_REQS,
SubcommandRequired
=> Flags::SC_REQUIRED,
SubcommandRequiredElseHelp
=> Flags::SC_REQUIRED_ELSE_HELP,
UseLongFormatForHelpSubcommand
=> Flags::USE_LONG_FORMAT_FOR_HELP_SC,
TrailingVarArg
Expand Down
5 changes: 1 addition & 4 deletions src/output/usage.rs
@@ -1,7 +1,6 @@
use indexmap::IndexSet;

// Internal
use crate::builder::AppSettings as AS;
use crate::builder::{Arg, ArgPredicate, Command};
use crate::parser::ArgMatcher;
use crate::util::ChildGraph;
Expand Down Expand Up @@ -142,9 +141,7 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
usage.push_str(" <");
usage.push_str(placeholder);
usage.push('>');
} else if self.cmd.is_subcommand_required_set()
|| self.cmd.is_set(AS::SubcommandRequiredElseHelp)
{
} else if self.cmd.is_subcommand_required_set() {
usage.push_str(" <");
usage.push_str(placeholder);
usage.push('>');
Expand Down
7 changes: 1 addition & 6 deletions src/parser/validator.rs
@@ -1,5 +1,5 @@
// Internal
use crate::builder::{AppSettings, Arg, ArgPredicate, Command, PossibleValue};
use crate::builder::{Arg, ArgPredicate, Command, PossibleValue};
use crate::error::{Error, Result as ClapResult};
use crate::output::fmt::Stream;
use crate::output::Usage;
Expand Down Expand Up @@ -60,7 +60,6 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
return Err(Error::display_help_error(self.cmd, message));
}
}
#[allow(deprecated)]
if !has_subcmd && self.cmd.is_subcommand_required_set() {
let bn = self
.cmd
Expand All @@ -73,10 +72,6 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
.required(&self.required)
.create_usage_with_title(&[]),
));
} else if !has_subcmd && self.cmd.is_set(AppSettings::SubcommandRequiredElseHelp) {
debug!("Validator::new::get_matches_with: SubcommandRequiredElseHelp=true");
let message = self.cmd.write_help_err(false, Stream::Stderr)?;
return Err(Error::display_help_error(self.cmd, message));
}

self.validate_conflicts(matcher, &mut conflicts)?;
Expand Down
47 changes: 1 addition & 46 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, ArgAction, Command};
use clap::{arg, error::ErrorKind, Arg, ArgAction, Command};

static ALLOW_EXT_SC: &str = "clap-test v1.4.8
Expand Down Expand Up @@ -67,20 +67,6 @@ OPTIONS:
-V, --version Print version information
";

static SUBCOMMAND_REQUIRED_ELSE_HELP: &str = "test 1.0
USAGE:
test <SUBCOMMAND>
OPTIONS:
-h, --help Print help information
-V, --version Print version information
SUBCOMMANDS:
help Print this message or the help of the given subcommand(s)
info
";

#[test]
fn sub_command_negate_required() {
Command::new("sub_command_negate")
Expand Down Expand Up @@ -194,37 +180,6 @@ fn arg_required_else_help_error_message() {
);
}

#[test]
fn subcommand_required_else_help() {
#![allow(deprecated)]
let result = Command::new("test")
.setting(AppSettings::SubcommandRequiredElseHelp)
.subcommand(Command::new("info"))
.try_get_matches_from(&[""]);

assert!(result.is_err());
let err = result.err().unwrap();
assert_eq!(
err.kind(),
ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand
);
}

#[test]
fn subcommand_required_else_help_error_message() {
#![allow(deprecated)]
let cmd = Command::new("test")
.setting(AppSettings::SubcommandRequiredElseHelp)
.version("1.0")
.subcommand(Command::new("info").arg(Arg::new("filename")));
utils::assert_output(
cmd,
"test",
SUBCOMMAND_REQUIRED_ELSE_HELP,
true, // Unlike normal displaying of help, we should provide a fatal exit code
);
}

#[cfg(not(feature = "suggestions"))]
#[test]
fn infer_subcommands_fail_no_args() {
Expand Down

0 comments on commit 8b064cf

Please sign in to comment.