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(derive): Move off of SubcommandRequiredElseHelp #3969

Merged
merged 1 commit into from Jul 22, 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
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