From 9f6fb9717268f8afc3d1c4dc2da7cf97a2b9c3d1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 28 Mar 2023 00:26:18 -0500 Subject: [PATCH] fix!: Remove `unstable-replace` feature flag This has been implemented for 3 years without much traction for finishing it up. The subcommand use case can be worked around by creating `Command`s that just include the relevant logic, very similar to the default subcommand examples in `git` / `git-derive`. Using this for flags is covered by #4793. Without `unstable-replace` being enabled, this still cut 5 KiB from `cargo bloat --release --example git`. Closes #2011 --- Cargo.toml | 1 - Makefile | 4 +- clap_builder/Cargo.toml | 3 +- clap_builder/src/builder/command.rs | 130 ---------------------------- clap_builder/src/parser/parser.rs | 14 --- src/_features.rs | 1 - tests/builder/subcommands.rs | 18 ---- tests/ui.rs | 2 - 8 files changed, 3 insertions(+), 170 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 113620f1e75..62a67d45b43 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -92,7 +92,6 @@ unicode = ["clap_builder/unicode"] # Support for unicode characters in argument string = ["clap_builder/string"] # Allow runtime generated strings # In-work features -unstable-replace = ["clap_builder/unstable-replace"] unstable-v5 = ["clap_builder/unstable-v5", "clap_derive?/unstable-v5", "deprecated"] [lib] diff --git a/Makefile b/Makefile index e97ca9040d8..6f54f577464 100644 --- a/Makefile +++ b/Makefile @@ -15,8 +15,8 @@ MSRV?=1.64.0 _FEATURES = minimal default wasm full debug release _FEATURES_minimal = --no-default-features --features "std" _FEATURES_default = -_FEATURES_wasm = --no-default-features --features "std help usage error-context suggestions" --features "deprecated derive cargo env unicode string unstable-replace" -_FEATURES_full = --features "deprecated derive cargo env unicode string unstable-replace wrap_help" +_FEATURES_wasm = --no-default-features --features "std help usage error-context suggestions" --features "deprecated derive cargo env unicode string" +_FEATURES_full = --features "deprecated derive cargo env unicode string wrap_help" _FEATURES_next = ${_FEATURES_full} --features unstable-v5 _FEATURES_debug = ${_FEATURES_full} --features debug --features clap_complete/debug _FEATURES_release = ${_FEATURES_full} --release diff --git a/clap_builder/Cargo.toml b/clap_builder/Cargo.toml index 907e62e346a..655306e3571 100644 --- a/clap_builder/Cargo.toml +++ b/clap_builder/Cargo.toml @@ -32,7 +32,7 @@ tag-name = "v{{version}}" [features] default = ["std", "color", "help", "usage", "error-context", "suggestions"] debug = ["dep:backtrace"] # Enables debug messages -unstable-doc = ["cargo", "wrap_help", "env", "unicode", "string", "unstable-replace"] # for docs.rs +unstable-doc = ["cargo", "wrap_help", "env", "unicode", "string"] # for docs.rs # Used in default std = [] # support for no_std in a backwards-compatible way @@ -51,7 +51,6 @@ unicode = ["dep:unicode-width", "dep:unicase"] # Support for unicode characters string = [] # Allow runtime generated strings # In-work features -unstable-replace = [] unstable-v5 = ["deprecated"] [lib] diff --git a/clap_builder/src/builder/command.rs b/clap_builder/src/builder/command.rs index 8f62c2c1f0b..34c04b4250e 100644 --- a/clap_builder/src/builder/command.rs +++ b/clap_builder/src/builder/command.rs @@ -24,7 +24,6 @@ use crate::output::fmt::Stream; use crate::output::{fmt::Colorizer, write_help, Usage}; use crate::parser::{ArgMatcher, ArgMatches, Parser}; use crate::util::ChildGraph; -use crate::util::FlatMap; use crate::util::{color::ColorChoice, Id}; use crate::{Error, INTERNAL_ERROR_MSG}; @@ -99,7 +98,6 @@ pub struct Command { g_settings: AppFlags, args: MKeyMap, subcommands: Vec, - replacers: FlatMap>, groups: Vec, current_help_heading: Option, current_disp_ord: Option, @@ -1924,129 +1922,6 @@ impl Command { self } - /// Replaces an argument or subcommand used on the CLI at runtime with other arguments or subcommands. - /// - /// **Note:** This is gated behind [`unstable-replace`](https://github.com/clap-rs/clap/issues/2836) - /// - /// When this method is used, `name` is removed from the CLI, and `target` - /// is inserted in its place. Parsing continues as if the user typed - /// `target` instead of `name`. - /// - /// This can be used to create "shortcuts" for subcommands, or if a - /// particular argument has the semantic meaning of several other specific - /// arguments and values. - /// - /// # Examples - /// - /// We'll start with the "subcommand short" example. In this example, let's - /// assume we have a program with a subcommand `module` which can be invoked - /// via `cmd module`. Now let's also assume `module` also has a subcommand - /// called `install` which can be invoked `cmd module install`. If for some - /// reason users needed to be able to reach `cmd module install` via the - /// short-hand `cmd install`, we'd have several options. - /// - /// We *could* create another sibling subcommand to `module` called - /// `install`, but then we would need to manage another subcommand and manually - /// dispatch to `cmd module install` handling code. This is error prone and - /// tedious. - /// - /// We could instead use [`Command::replace`] so that, when the user types `cmd - /// install`, `clap` will replace `install` with `module install` which will - /// end up getting parsed as if the user typed the entire incantation. - /// - /// ```rust - /// # use clap_builder as clap; - /// # use clap::Command; - /// let m = Command::new("cmd") - /// .subcommand(Command::new("module") - /// .subcommand(Command::new("install"))) - /// .replace("install", &["module", "install"]) - /// .get_matches_from(vec!["cmd", "install"]); - /// - /// assert!(m.subcommand_matches("module").is_some()); - /// assert!(m.subcommand_matches("module").unwrap().subcommand_matches("install").is_some()); - /// ``` - /// - /// Now let's show an argument example! - /// - /// Let's assume we have an application with two flags `--save-context` and - /// `--save-runtime`. But often users end up needing to do *both* at the - /// same time. We can add a third flag `--save-all` which semantically means - /// the same thing as `cmd --save-context --save-runtime`. To implement that, - /// we have several options. - /// - /// We could create this third argument and manually check if that argument - /// and in our own consumer code handle the fact that both `--save-context` - /// and `--save-runtime` *should* have been used. But again this is error - /// prone and tedious. If we had code relying on checking `--save-context` - /// and we forgot to update that code to *also* check `--save-all` it'd mean - /// an error! - /// - /// Luckily we can use [`Command::replace`] so that when the user types - /// `--save-all`, `clap` will replace that argument with `--save-context - /// --save-runtime`, and parsing will continue like normal. Now all our code - /// that was originally checking for things like `--save-context` doesn't - /// need to change! - /// - /// ```rust - /// # use clap_builder as clap; - /// # use clap::{Command, Arg, ArgAction}; - /// let m = Command::new("cmd") - /// .arg(Arg::new("save-context") - /// .long("save-context") - /// .action(ArgAction::SetTrue)) - /// .arg(Arg::new("save-runtime") - /// .long("save-runtime") - /// .action(ArgAction::SetTrue)) - /// .replace("--save-all", &["--save-context", "--save-runtime"]) - /// .get_matches_from(vec!["cmd", "--save-all"]); - /// - /// assert!(m.get_flag("save-context")); - /// assert!(m.get_flag("save-runtime")); - /// ``` - /// - /// This can also be used with options, for example if our application with - /// `--save-*` above also had a `--format=TYPE` option. Let's say it - /// accepted `txt` or `json` values. However, when `--save-all` is used, - /// only `--format=json` is allowed, or valid. We could change the example - /// above to enforce this: - /// - /// ```rust - /// # use clap_builder as clap; - /// # use clap::{Command, Arg, ArgAction}; - /// let m = Command::new("cmd") - /// .arg(Arg::new("save-context") - /// .long("save-context") - /// .action(ArgAction::SetTrue)) - /// .arg(Arg::new("save-runtime") - /// .long("save-runtime") - /// .action(ArgAction::SetTrue)) - /// .arg(Arg::new("format") - /// .long("format") - /// .action(ArgAction::Set) - /// .value_parser(["txt", "json"])) - /// .replace("--save-all", &["--save-context", "--save-runtime", "--format=json"]) - /// .get_matches_from(vec!["cmd", "--save-all"]); - /// - /// assert!(m.get_flag("save-context")); - /// assert!(m.get_flag("save-runtime")); - /// assert_eq!(m.get_one::("format").unwrap(), "json"); - /// ``` - /// - /// [`Command::replace`]: Command::replace() - #[inline] - #[cfg(feature = "unstable-replace")] - #[must_use] - pub fn replace( - mut self, - name: impl Into, - target: impl IntoIterator>, - ) -> Self { - self.replacers - .insert(name.into(), target.into_iter().map(Into::into).collect()); - self - } - /// Exit gracefully if no arguments are present (e.g. `$ myprog`). /// /// **NOTE:** [`subcommands`] count as arguments @@ -3853,10 +3728,6 @@ impl Command { self.max_w } - pub(crate) fn get_replacement(&self, key: &str) -> Option<&[Str]> { - self.replacers.get(key).map(|v| v.as_slice()) - } - pub(crate) fn get_keymap(&self) -> &MKeyMap { &self.args } @@ -4749,7 +4620,6 @@ impl Default for Command { g_settings: Default::default(), args: Default::default(), subcommands: Default::default(), - replacers: Default::default(), groups: Default::default(), current_help_heading: Default::default(), current_disp_ord: Some(0), diff --git a/clap_builder/src/parser/parser.rs b/clap_builder/src/parser/parser.rs index 37d8e589e85..c2910048528 100644 --- a/clap_builder/src/parser/parser.rs +++ b/clap_builder/src/parser/parser.rs @@ -76,20 +76,6 @@ impl<'cmd> Parser<'cmd> { let contains_last = self.cmd.get_arguments().any(|x| x.is_last_set()); while let Some(arg_os) = raw_args.next(&mut args_cursor) { - // Recover the replaced items if any. - if let Some(replaced_items) = arg_os - .to_value() - .ok() - .and_then(|a| self.cmd.get_replacement(a)) - { - debug!( - "Parser::get_matches_with: found replacer: {:?}, target: {:?}", - arg_os, replaced_items - ); - raw_args.insert(&args_cursor, replaced_items); - continue; - } - debug!( "Parser::get_matches_with: Begin parsing '{:?}'", arg_os.to_value_os(), diff --git a/src/_features.rs b/src/_features.rs index 2f551e79261..b47ee259c28 100644 --- a/src/_features.rs +++ b/src/_features.rs @@ -25,5 +25,4 @@ //! //! **Warning:** These may contain breaking changes between minor releases. //! -//! * **unstable-replace**: Enable [`Command::replace`](https://github.com/clap-rs/clap/issues/2836) //! * **unstable-v5**: Preview features which will be stable on the v5.0 release diff --git a/tests/builder/subcommands.rs b/tests/builder/subcommands.rs index c7be253debe..4131fda94f5 100644 --- a/tests/builder/subcommands.rs +++ b/tests/builder/subcommands.rs @@ -225,24 +225,6 @@ Options: utils::assert_output(cmd, "clap-test --help", INVISIBLE_ALIAS_HELP, false); } -#[test] -#[cfg(feature = "unstable-replace")] -fn replace() { - let m = Command::new("prog") - .subcommand( - Command::new("module").subcommand(Command::new("install").about("Install module")), - ) - .replace("install", ["module", "install"]) - .try_get_matches_from(vec!["prog", "install"]) - .unwrap(); - - assert_eq!(m.subcommand_name(), Some("module")); - assert_eq!( - m.subcommand_matches("module").unwrap().subcommand_name(), - Some("install") - ); -} - #[test] fn issue_1031_args_with_same_name() { let res = Command::new("prog") diff --git a/tests/ui.rs b/tests/ui.rs index 2977e25ae1c..2bbd80860d1 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -25,8 +25,6 @@ fn ui_tests() { "string", #[cfg(feature = "wrap_help")] "wrap_help", - #[cfg(feature = "unstable-replace")] - "unstable-replace", ] .join(" "); t.register_bins(trycmd::cargo::compile_examples(["--features", &features]).unwrap());