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

Avoid new deprecation warnings from clap 3.1.0 #10396

Merged
merged 4 commits into from
Feb 17, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use anyhow::anyhow;
use cargo::core::{features, CliUnstable};
use cargo::{self, drop_print, drop_println, CliResult, Config};
use clap::{AppSettings, Arg, ArgMatches};
use clap::{
error::{ContextKind, ContextValue},
AppSettings, Arg, ArgMatches,
};
use itertools::Itertools;
use std::collections::HashMap;
use std::fmt::Write;
Expand Down Expand Up @@ -33,9 +36,18 @@ pub fn main(config: &mut Config) -> CliResult {
let args = match cli().try_get_matches() {
Ok(args) => args,
Err(e) => {
if e.kind == clap::ErrorKind::UnrecognizedSubcommand {
if e.kind() == clap::ErrorKind::UnrecognizedSubcommand {
// An unrecognized subcommand might be an external subcommand.
let cmd = e.info[0].clone();
let cmd = e
.context()
.find_map(|c| match c {
(ContextKind::InvalidSubcommand, &ContextValue::String(ref cmd)) => {
Some(cmd)
}
(ContextKind::InvalidSubcommand, _) => unreachable!(),
_ => None,
})
.unwrap();
return super::execute_external_subcommand(config, &cmd, &[&cmd, "--help"])
.map_err(|_| e.into());
} else {
Expand Down Expand Up @@ -286,9 +298,7 @@ For more information, see issue #10049 <https://github.com/rust-lang/cargo/issue
// Note that an alias to an external command will not receive
// these arguments. That may be confusing, but such is life.
let global_args = GlobalArgs::new(args);
let new_args = cli()
.setting(AppSettings::NoBinaryName)
.try_get_matches_from(alias)?;
let new_args = cli().no_binary_name(true).try_get_matches_from(alias)?;

let new_cmd = new_args.subcommand_name().expect("subcommand is required");
already_expanded.push(cmd.to_string());
Expand Down Expand Up @@ -406,14 +416,11 @@ fn cli() -> App {
"cargo [OPTIONS] [SUBCOMMAND]"
};
App::new("cargo")
.setting(
AppSettings::DeriveDisplayOrder
| AppSettings::AllowExternalSubcommands
| AppSettings::NoAutoVersion,
)
.allow_external_subcommands(true)
.setting(AppSettings::DeriveDisplayOrder | AppSettings::NoAutoVersion)
// Doesn't mix well with our list of common cargo commands. See clap-rs/clap#3108 for
// opening clap up to allow us to style our help template
.global_setting(AppSettings::DisableColoredHelp)
.disable_colored_help(true)
jonhoo marked this conversation as resolved.
Show resolved Hide resolved
.override_usage(usage)
.help_template(
"\
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use cargo::ops::{self, TestOptions};

pub fn cli() -> App {
subcommand("bench")
.setting(AppSettings::TrailingVarArg)
.trailing_var_arg(true)
.about("Execute all benchmarks of a local package")
.arg_quiet()
.arg(
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pub fn cli() -> App {
subcommand("config")
.about("Inspect configuration values")
.after_help("Run `cargo help config` for more detailed information.\n")
.setting(clap::AppSettings::SubcommandRequiredElseHelp)
.subcommand_required(true)
jonhoo marked this conversation as resolved.
Show resolved Hide resolved
.subcommand(
subcommand("get")
.arg(Arg::new("key").help("The config key to display"))
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/git_checkout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const REMOVED: &str = "The `git-checkout` subcommand has been removed.";
pub fn cli() -> App {
subcommand("git-checkout")
.about("This subcommand has been removed")
.setting(AppSettings::Hidden)
.hide(true)
.override_help(REMOVED)
}

Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub fn cli() -> App {
subcommand("report")
.about("Generate and display various kinds of reports")
.after_help("Run `cargo help report` for more detailed information.\n")
.setting(clap::AppSettings::SubcommandRequiredElseHelp)
.subcommand_required(true)
jonhoo marked this conversation as resolved.
Show resolved Hide resolved
.subcommand(
subcommand("future-incompatibilities")
.alias("future-incompat")
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub fn cli() -> App {
subcommand("run")
// subcommand aliases are handled in aliased_command()
// .alias("r")
.setting(AppSettings::TrailingVarArg)
.trailing_var_arg(true)
.about("Run a binary or example of the local package")
.arg_quiet()
.arg(
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const CRATE_TYPE_ARG_NAME: &str = "crate-type";

pub fn cli() -> App {
subcommand("rustc")
.setting(AppSettings::TrailingVarArg)
.trailing_var_arg(true)
.about("Compile a package, and pass extra options to the compiler")
.arg_quiet()
.arg(Arg::new("args").multiple_values(true).help("Rustc flags"))
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::command_prelude::*;

pub fn cli() -> App {
subcommand("rustdoc")
.setting(AppSettings::TrailingVarArg)
.trailing_var_arg(true)
.about("Build a package's documentation, using specified custom flags.")
.arg_quiet()
.arg(Arg::new("args").multiple_values(true))
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub fn cli() -> App {
subcommand("test")
// Subcommand aliases are handled in `aliased_command()`.
// .alias("t")
.setting(AppSettings::TrailingVarArg)
.trailing_var_arg(true)
.about("Execute all unit and integration tests and build examples of a local package")
.arg(
Arg::new("TESTNAME")
Expand Down
6 changes: 4 additions & 2 deletions src/cargo/util/command_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub use crate::core::compiler::CompileMode;
pub use crate::{CliError, CliResult, Config};
pub use clap::{AppSettings, Arg, ArgMatches};

pub type App = clap::App<'static>;
pub type App = clap::Command<'static>;

pub trait AppExt: Sized {
fn _arg(self, arg: Arg<'static>) -> Self;
Expand Down Expand Up @@ -281,7 +281,9 @@ pub fn multi_opt(name: &'static str, value_name: &'static str, help: &'static st
}

pub fn subcommand(name: &'static str) -> App {
App::new(name).setting(AppSettings::DeriveDisplayOrder | AppSettings::DontCollapseArgsInUsage)
App::new(name)
.dont_collapse_args_in_usage(true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI this became a global_setting.

If that is a concern, you can still use the AppSetting until clap v4. In clap v4, my expectation is we'll support subcommands overriding global settings (I think that doesn't work today)

When we made this global, it was under the assumption people would want a uniform look. If this assumption is incorrect, we can open an issue and re-visit this in clap v4.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, interesting. I honestly couldn't tell you whether that change is okay or not. I feel like it probably is since we're seemingly applying it to all subcommands, but others who know the CLI interface better may need to weigh in here.

.setting(AppSettings::DeriveDisplayOrder)
}

/// Determines whether or not to gate `--profile` as unstable when resolving it.
Expand Down