Repository: clap-rs/clap
Pull request #2211 (2020-11-14 08:34:01)

Prefer mut_arg over help_about and version_about
The original motivation for this PR is that changing the help & version args `short`, `long` and `hidden` and other stuff wasn't working with `mut_arg`.

In v2, we have `help_short`, `version_short`, `help_message` & `version_message` which used to set these. But it's definitely not a complete solution because other details like `hidden` etc.. can't be set. So, we removed them in favor of `mut_arg`. But since `mut_arg` executes before building the `help` and `version` args, it needs the whole arg built and not just the one attribute you want to change. (I have added tests cases to reflect this).

One other thing we want to think on is recurring changes. Users don't want to keep doing this for every app/subcommand they build. Ideally they want it to propagate from top. We added `help_about` and `version_about` in v3 which propagates. These were requested for v2 originally because users weren't able to set help & version subcommand's about.

So, I decided to get some opinion before actually finalising this:

1. Do we agree on `mut_arg` being executed during the build process after building all the args?
2. Do we need a `global_mut_arg` that propagates and does the same as `mut_arg`?
3. Do we need a `mut_subcommand` and `global_mut_subcommand` that behaves similarly but for subcommands? (This will allow us to replace `help_about` and `version_about` to use this).

kbknapp: It sounds like the conflicting ideas are that the `help` and `version` args are built lazily (just before parsing), but `mut_arg` would need them to exist in order to actually mutate them in any meaningful way. This is consistent with the problems I ran into at the beginning of the v3 build.

The options are we either:

- Only run `mut_arg` at the last possible time (i.e. after all args are built), but this has the problem of not allowing one to mutate an arg based on existing args. i.e. if an `App` struct is built, passed to some other function for further processing and queried for information, and based off that information mutated. All of that would have to happen well before parsing.
- Build the `help` and `version` "dummy" args early in the process. The biggest downside is if someone does not want help/version args this is needless processing. However, it's extremely minimal, so I would be fine with this personally. The other problem that could arise is not knowing if the values provided were clap consumer supplied, or the dummy values meaning we'd need to track the changes.
- A combination of the two - where the dummy args are built early, can be mutated freely, and just prior to parsing they are filled/normalized to remove or fill any remaining dummy values.

To address the having to do this globally, or not I see the two ideas as orthogonal. If you mutate a global argument, that mutation should happen prior to global propagation. If that mutation happens *after* global propagation, that is on purpose (i.e. to change/undo some unwanted propagated value). We just need to be clear about *when* mutation happens vs when propagation happens. I don't think a `mut_arg_global` is warranted. The only somewhat exception to this rule may be the help and version args, which we could document and make global from the start, but I'm not sure how this would work in the specifics of a subcommand where the user *wants* to mutate the help/version args from the parent command, and wants those changes passed on to any other children...but this might be super niche to the point we don't need to worry about it in the general case.

> 1. Do we agree on `mut_arg` being executed during the build process after building all the args?

I would suggest running `mut_arg` *eagerly* i.e. as soon as it's called. Otherwise you end up with "stacking" changes that can be hard to mentally track in your own application. However, I *do* agree `help`/`version` should already be built (which I think leads to building `help`/`version` early as mentioned above, instead of lazily.

> 2. Do we need a `global_mut_arg` that propagates and does the same as `mut_arg`?

I don't believe so. If you `mut_arg` a global arg, those changes will be propagated down once the propagation phase happens.

> 3. Do we need a `mut_subcommand` and `global_mut_subcommand` that behaves similarly but for subcommands? (This will allow us to replace `help_about` and `long_about` to use this).

Interesting. I hadn't thought about this too much yet. I think eventually a `mut_subcommand` would be helpful to provide API parity. I have the same feelings about the `global_*` though.
pksunkara: I think there is a misunderstanding about global_mut_arg. It is not for global arguments but it is for propagating the mut_arg lambda to all the subcommands. That way users can use this to specify the short for version once at the top App and all the subcommands will follow it.
kbknapp: I get that, but I mean if an argument is global (i.e. it will be propagated to all child subcommands) and a mutate lambda gets applied it in the parent, that mutation is "applied" in the parent command, and the already mutated arg is what is actually propagated down to the child subcommand. i.e. there is no need to re-apply that mutation in each child subcommand.

The only part this gets a little wacky, is as you noted with how we handle help/version since those aren't real args until the parsing starts. So I'm suggesting potentially making help/version right away as global args when `App` is initialized (perhaps with dummy values that we can distinguish later are parse time to know if something was user supplied or not), that way they too can take advantage of this global vs mutation behavior.

Otherwise, I agree and we would only be able to apply the mutations *after* help/version are built (i.e. parse time) and would also need some construct for propagating these mutations to child subcommands if applicable which I think is more complex.

pksunkara: I am stuck on making them global args. How would I turn them off in my subcommands? I think people rely on `help` and `version` being a separate arg instead of global.
pksunkara: @ldm0 Any thoughts on this?
ldm0: > 1. Do we agree on `mut_arg` being executed during the build process after building all the args?

I agree. I think seperating the `help` and the `version` out from average args is a bad idea.

> 2. Do we need a `global_mut_arg` that propagates and does the same as `mut_arg`?

I think it's needed. I get what @kbknapp said. But I think applying `global_mut_arg` doesn't equals to applying `mut_arg` on a global argument. Changing different args in different subcommands with the same name is also a valid use case.

> 3. Do we need a `mut_subcommand` and `global_mut_subcommand` that behaves similarly but for subcommands? (This will allow us to replace `help_about` and `long_about` to use this).

I think they are needed, whether from the standpoint of API elegance or from the standpoint of usefulness.

pksunkara: > So I'm suggesting potentially making help/version right away as global args when App is initialized

Just to confirm, you are saying this is a bad idea, right?
ldm0: > > So I'm suggesting potentially making help/version right away as global args when App is initialized
> 
> Just to confirm, you are saying this is a bad idea, right?

Nope, I think this is a great idea. These two idea doesn't conflicts. We can have `mut_arg` after building help and version as current workaround, and than trying to make the `help` and `version` global args later.
pksunkara: That doesnt need to be done later. They can be done now. But that is what I have been trying to do and I find myself stuck on the API design.
ldm0: > That doesnt need to be done later. They can be done now. But that is what I have been trying to do and I find myself stuck on the API design.

Changing the behavior of `DisableHelpFlags` and `DisableVersion`. Letting them turn corresponding arg off in the app may helps.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e957d038..c432759b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,8 +24,6 @@ TODO: `cargo`, `std` features
 
 * **Added Methods**
   * **App**
-    * `App::help_about`
-    * `App::version_about`
   * **Arg**
     * `Arg::hide_env`
 * **Added Settings**
diff --git a/clap_derive/src/lib.rs b/clap_derive/src/lib.rs
index c68951dc..aad634b1 100644
--- a/clap_derive/src/lib.rs
+++ b/clap_derive/src/lib.rs
@@ -12,6 +12,9 @@
 // commit#ea76fa1b1b273e65e3b0b1046643715b49bec51f which is licensed under the
 // MIT/Apache 2.0 license.
 
+#![doc(html_logo_url = "https://clap.rs/images/media/clap.png")]
+#![doc(html_root_url = "https://docs.rs/clap_derive/3.0.0-beta.2")]
+
 //! This crate is custom derive for clap. It should not be used
 //! directly. See [clap documentation](https://docs.rs/clap)
 //! for the usage of `#[derive(Clap)]`.
@@ -42,8 +45,8 @@ pub fn arg_enum(input: TokenStream) -> TokenStream {
 ///
 /// This is far less verbose than defining the [`App`] struct manually,
 /// receiving an instance of [`ArgMatches`] from conducting parsing, and then
-/// implementing a conversion code (usually `impl From<ArgMatches> for
-/// MyStruct`) to instantiate an instance of the user context struct.
+/// implementing a conversion code to instantiate an instance of the user
+/// context struct.
 #[proc_macro_derive(Clap, attributes(clap))]
 #[proc_macro_error]
 pub fn clap(input: TokenStream) -> TokenStream {
diff --git a/clap_generate/src/lib.rs b/clap_generate/src/lib.rs
index ec3aee73..004af948 100644
--- a/clap_generate/src/lib.rs
+++ b/clap_generate/src/lib.rs
@@ -7,6 +7,7 @@
 
 //! Generates stuff for [`clap`](https://github.com/clap-rs/clap) based CLIs
 
+#![doc(html_logo_url = "https://clap.rs/images/media/clap.png")]
 #![doc(html_root_url = "https://docs.rs/clap_generate/3.0.0-beta.2")]
 #![deny(
     missing_docs,
diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs
index f2bbffc9..d7335eac 100644
--- a/src/build/app/mod.rs
+++ b/src/build/app/mod.rs
@@ -73,8 +73,6 @@ pub struct App<'help> {
     pub(crate) license: Option<&'help str>,
     pub(crate) about: Option<&'help str>,
     pub(crate) long_about: Option<&'help str>,
-    pub(crate) help_about: Option<&'help str>,
-    pub(crate) version_about: Option<&'help str>,
     pub(crate) before_help: Option<&'help str>,
     pub(crate) before_long_help: Option<&'help str>,
     pub(crate) after_help: Option<&'help str>,
@@ -496,50 +494,6 @@ impl<'help> App<'help> {
         self
     }
 
-    /// Sets the help text for the auto-generated help argument and subcommand.
-    ///
-    /// By default clap sets this to "Prints help information" for the help
-    /// argument and "Prints this message or the help of the given subcommand(s)"
-    /// for the help subcommand but if you're using a different convention
-    /// for your help messages and would prefer a different phrasing you can
-    /// override it.
-    ///
-    /// **NOTE:** This setting propagates to subcommands.
-    ///
-    /// # Examples
-    ///
-    /// ```no_run
-    /// # use clap::App;
-    /// App::new("myprog")
-    ///     .help_about("Print help information") // Imperative tone
-    /// # ;
-    /// ```
-    pub fn help_about<S: Into<&'help str>>(mut self, help_about: S) -> Self {
-        self.help_about = Some(help_about.into());
-        self
-    }
-
-    /// Sets the help text for the auto-generated version argument.
-    ///
-    /// By default clap sets this to "Prints version information" but if
-    /// you're using a different convention for your help messages and
-    /// would prefer a different phrasing you can override it.
-    ///
-    /// **NOTE:** This setting propagates to subcommands.
-    ///
-    /// # Examples
-    ///
-    /// ```no_run
-    /// # use clap::App;
-    /// App::new("myprog")
-    ///     .version_about("Print version information") // Imperative tone
-    /// # ;
-    /// ```
-    pub fn version_about<S: Into<&'help str>>(mut self, version_about: S) -> Self {
-        self.version_about = Some(version_about.into());
-        self
-    }
-
     /// (Re)Sets the program's name. This will be displayed when displaying help
     /// or version messages.
     ///
@@ -1787,13 +1741,18 @@ impl<'help> App<'help> {
     {
         let arg_id: &str = arg_id.into();
         let id = Id::from(arg_id);
-        let a = self.args.remove_by_name(&id).unwrap_or_else(|| Arg {
+
+        let mut a = self.args.remove_by_name(&id).unwrap_or_else(|| Arg {
             id,
             name: arg_id,
             ..Arg::default()
         });
-        self.args.push(f(a));
 
+        if a.provider == ArgProvider::Generated {
+            a.provider = ArgProvider::GeneratedMutated;
+        }
+
+        self.args.push(f(a));
         self
     }
 
@@ -2389,14 +2348,28 @@ impl<'help> App<'help> {
 
         for sc in &mut self.subcommands {
             for a in self.args.args().filter(|a| a.global) {
-                let is_generated = a.provider == ArgProvider::Generated;
+                let mut propagate = false;
+                let is_generated = matches!(
+                    a.provider,
+                    ArgProvider::Generated | ArgProvider::GeneratedMutated
+                );
 
                 // Remove generated help and version args in the subcommand
+                //
+                // Don't remove if those args are futher mutated
                 if is_generated {
-                    sc.args.remove_by_name(&a.id);
+                    let generated_pos = sc
+                        .args
+                        .args()
+                        .position(|x| x.id == a.id && x.provider == ArgProvider::Generated);
+
+                    if let Some(index) = generated_pos {
+                        sc.args.remove(index);
+                        propagate = true;
+                    }
                 }
 
-                if is_generated || sc.find(&a.id).is_none() {
+                if propagate || sc.find(&a.id).is_none() {
                     sc.args.push(a.clone());
                 }
             }
@@ -2426,12 +2399,6 @@ impl<'help> App<'help> {
                     $sc.g_settings = $sc.g_settings | $_self.g_settings;
                     $sc.term_w = $_self.term_w;
                     $sc.max_w = $_self.max_w;
-                    if $sc.help_about.is_none() && $_self.help_about.is_some() {
-                        $sc.help_about = $_self.help_about.clone();
-                    }
-                    if $sc.version_about.is_none() && $_self.version_about.is_some() {
-                        $sc.version_about = $_self.version_about.clone();
-                    }
                 }
             }};
         }
@@ -2481,10 +2448,6 @@ impl<'help> App<'help> {
             {
                 help.short = Some('h');
             }
-
-            if self.help_about.is_some() {
-                help.about = self.help_about;
-            }
         }
 
         if self.is_set(AppSettings::DisableVersionFlag)
@@ -2521,10 +2484,6 @@ impl<'help> App<'help> {
             {
                 version.short = Some('V');
             }
-
-            if self.version_about.is_some() {
-                version.about = self.version_about;
-            }
         }
 
         if !self.is_set(AppSettings::DisableHelpSubcommand)
@@ -2533,10 +2492,8 @@ impl<'help> App<'help> {
         {
             debug!("App::_check_help_and_version: Building help subcommand");
             self.subcommands.push(
-                App::new("help").about(
-                    self.help_about
-                        .unwrap_or("Prints this message or the help of the given subcommand(s)"),
-                ),
+                App::new("help")
+                    .about("Prints this message or the help of the given subcommand(s)"),
             );
         }
     }
diff --git a/src/build/arg/mod.rs b/src/build/arg/mod.rs
index 7dd04c1b..bc14ac0b 100644
--- a/src/build/arg/mod.rs
+++ b/src/build/arg/mod.rs
@@ -45,6 +45,7 @@ type ValidatorOs<'a> = dyn FnMut(&OsStr) -> Result<(), Box<dyn Error + Send + Sy
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub(crate) enum ArgProvider {
     Generated,
+    GeneratedMutated,
     User,
 }
 
diff --git a/src/lib.rs b/src/lib.rs
index 20524f65..c969060f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -4,6 +4,7 @@
 // notice may not be copied, modified, or distributed except according to those terms.
 
 #![cfg_attr(feature = "doc", feature(external_doc))]
+#![doc(html_logo_url = "https://clap.rs/images/media/clap.png")]
 #![doc(html_root_url = "https://docs.rs/clap/3.0.0-beta.2")]
 #![cfg_attr(feature = "doc", doc(include = "../README.md"))]
 //! https://github.com/clap-rs/clap


Test patch:
diff --git a/tests/help.rs b/tests/help.rs
index 7a6905b2..179f47fb 100644
--- a/tests/help.rs
+++ b/tests/help.rs
@@ -372,17 +372,6 @@ FLAGS:
     -V, --version
             Prints version information";
 
-static CUSTOM_VERSION_AND_HELP: &str = "customize 0.1
-Nobody <odysseus@example.com>
-You can customize the version and help text
-
-USAGE:
-    customize
-
-FLAGS:
-    -H, --help       Print help information
-    -v, --version    Print version information";
-
 static HELP_CONFLICT: &str = "conflict 
 
 USAGE:
@@ -1391,24 +1380,89 @@ fn issue_777_wrap_all_things() {
     assert!(utils::compare_output(app, "ctest --help", ISSUE_777, false));
 }
 
+static OVERRIDE_HELP_SHORT: &str = "test 0.1
+
+USAGE:
+    test
+
+FLAGS:
+    -H, --help       Prints help information
+    -V, --version    Prints version information";
+
+#[test]
+fn override_help_short() {
+    let app = App::new("test")
+        .version("0.1")
+        .mut_arg("help", |h| h.short('H'));
+
+    assert!(utils::compare_output(
+        app.clone(),
+        "test --help",
+        OVERRIDE_HELP_SHORT,
+        false
+    ));
+    assert!(utils::compare_output(
+        app,
+        "test -H",
+        OVERRIDE_HELP_SHORT,
+        false
+    ));
+}
+
+static OVERRIDE_HELP_LONG: &str = "test 0.1
+
+USAGE:
+    test [FLAGS]
+
+FLAGS:
+    -h, --hell       Prints help information
+    -V, --version    Prints version information";
+
 #[test]
-fn customize_version_and_help() {
-    let app = App::new("customize")
+fn override_help_long() {
+    let app = App::new("test")
         .version("0.1")
-        .author("Nobody <odysseus@example.com>")
-        .about("You can customize the version and help text")
-        .mut_arg("help", |h| {
-            h.short('H').long("help").about("Print help information")
-        })
-        .mut_arg("version", |v| {
-            v.short('v')
-                .long("version")
-                .about("Print version information")
-        });
+        .mut_arg("help", |h| h.long("hell"));
+
+    assert!(utils::compare_output(
+        app.clone(),
+        "test --hell",
+        OVERRIDE_HELP_LONG,
+        false
+    ));
     assert!(utils::compare_output(
         app,
-        "customize --help",
-        CUSTOM_VERSION_AND_HELP,
+        "test -h",
+        OVERRIDE_HELP_LONG,
+        false
+    ));
+}
+
+static OVERRIDE_HELP_ABOUT: &str = "test 0.1
+
+USAGE:
+    test
+
+FLAGS:
+    -h, --help       Print help information
+    -V, --version    Prints version information";
+
+#[test]
+fn override_help_about() {
+    let app = App::new("test")
+        .version("0.1")
+        .mut_arg("help", |h| h.about("Print help information"));
+
+    assert!(utils::compare_output(
+        app.clone(),
+        "test --help",
+        OVERRIDE_HELP_ABOUT,
+        false
+    ));
+    assert!(utils::compare_output(
+        app,
+        "test -h",
+        OVERRIDE_HELP_ABOUT,
         false
     ));
 }
@@ -2038,15 +2092,6 @@ fn after_help_no_args() {
     assert_eq!(help, AFTER_HELP_NO_ARGS);
 }
 
-static HELP_ABOUT: &str = "myapp 1.0
-
-USAGE:
-    myapp
-
-FLAGS:
-    -h, --help       Print custom help about text
-    -V, --version    Prints version information";
-
 static HELP_ABOUT_MULTI_SC: &str = "myapp-subcmd-multi 1.0
 
 USAGE:
@@ -2065,63 +2110,24 @@ FLAGS:
     -h, --help       Print custom help about text from multi
     -V, --version    Prints version information";
 
-#[test]
-fn help_about_short_flag() {
-    let app = App::new("myapp")
-        .version("1.0")
-        .help_about("Print custom help about text");
-
-    assert!(utils::compare_output(app, "myapp -h", HELP_ABOUT, false));
-}
-
-#[test]
-fn help_about_long_flag() {
-    let app = App::new("myapp")
-        .version("1.0")
-        .help_about("Print custom help about text");
-
-    assert!(utils::compare_output(
-        app,
-        "myapp --help",
-        HELP_ABOUT,
-        false
-    ));
-}
-
 #[test]
 fn help_about_multi_subcmd() {
     let app = App::new("myapp")
-        .help_about("Print custom help about text")
+        .mut_arg("help", |h| h.about("Print custom help about text"))
         .subcommand(App::new("subcmd").subcommand(App::new("multi").version("1.0")));
 
     assert!(utils::compare_output(
-        app,
+        app.clone(),
         "myapp help subcmd multi",
         HELP_ABOUT_MULTI_SC,
         false
     ));
-}
-
-#[test]
-fn help_about_multi_subcmd_short_flag() {
-    let app = App::new("myapp")
-        .help_about("Print custom help about text")
-        .subcommand(App::new("subcmd").subcommand(App::new("multi").version("1.0")));
-
     assert!(utils::compare_output(
-        app,
+        app.clone(),
         "myapp subcmd multi -h",
         HELP_ABOUT_MULTI_SC,
         false
     ));
-}
-
-#[test]
-fn help_about_multi_subcmd_long_flag() {
-    let app = App::new("myapp")
-        .help_about("Print custom help about text")
-        .subcommand(App::new("subcmd").subcommand(App::new("multi").version("1.0")));
-
     assert!(utils::compare_output(
         app,
         "myapp subcmd multi --help",
@@ -2133,55 +2139,25 @@ fn help_about_multi_subcmd_long_flag() {
 #[test]
 fn help_about_multi_subcmd_override() {
     let app = App::new("myapp")
-        .help_about("Print custom help about text")
-        .subcommand(
-            App::new("subcmd").subcommand(
-                App::new("multi")
-                    .version("1.0")
-                    .help_about("Print custom help about text from multi"),
-            ),
-        );
+        .mut_arg("help", |h| h.about("Print custom help about text"))
+        .subcommand(App::new("subcmd").subcommand(
+            App::new("multi").version("1.0").mut_arg("help", |h| {
+                h.about("Print custom help about text from multi")
+            }),
+        ));
 
     assert!(utils::compare_output(
-        app,
+        app.clone(),
         "myapp help subcmd multi",
         HELP_ABOUT_MULTI_SC_OVERRIDE,
         false
     ));
-}
-
-#[test]
-fn help_about_multi_subcmd_override_short_flag() {
-    let app = App::new("myapp")
-        .help_about("Print custom help about text")
-        .subcommand(
-            App::new("subcmd").subcommand(
-                App::new("multi")
-                    .version("1.0")
-                    .help_about("Print custom help about text from multi"),
-            ),
-        );
-
     assert!(utils::compare_output(
-        app,
+        app.clone(),
         "myapp subcmd multi -h",
         HELP_ABOUT_MULTI_SC_OVERRIDE,
         false
     ));
-}
-
-#[test]
-fn help_about_multi_subcmd_override_long_flag() {
-    let app = App::new("myapp")
-        .help_about("Print custom help about text")
-        .subcommand(
-            App::new("subcmd").subcommand(
-                App::new("multi")
-                    .version("1.0")
-                    .help_about("Print custom help about text from multi"),
-            ),
-        );
-
     assert!(utils::compare_output(
         app,
         "myapp subcmd multi --help",
@@ -2190,158 +2166,6 @@ fn help_about_multi_subcmd_override_long_flag() {
     ));
 }
 
-static VERSION_ABOUT: &str = "myapp 1.0
-
-USAGE:
-    myapp
-
-FLAGS:
-    -h, --help       Prints help information
-    -V, --version    Print custom version about text";
-
-static VERSION_ABOUT_MULTI_SC: &str = "myapp-subcmd-multi 1.0
-
-USAGE:
-    myapp subcmd multi
-
-FLAGS:
-    -h, --help       Prints help information
-    -V, --version    Print custom version about text";
-
-static VERSION_ABOUT_MULTI_SC_OVERRIDE: &str = "myapp-subcmd-multi 1.0
-
-USAGE:
-    myapp subcmd multi
-
-FLAGS:
-    -h, --help       Prints help information
-    -V, --version    Print custom version about text from multi";
-
-#[test]
-fn version_about_short_flag() {
-    let app = App::new("myapp")
-        .version("1.0")
-        .version_about("Print custom version about text");
-
-    assert!(utils::compare_output(app, "myapp -h", VERSION_ABOUT, false));
-}
-
-#[test]
-fn version_about_long_flag() {
-    let app = App::new("myapp")
-        .version("1.0")
-        .version_about("Print custom version about text");
-
-    assert!(utils::compare_output(
-        app,
-        "myapp --help",
-        VERSION_ABOUT,
-        false
-    ));
-}
-
-#[test]
-fn version_about_multi_subcmd() {
-    let app = App::new("myapp")
-        .version_about("Print custom version about text")
-        .subcommand(App::new("subcmd").subcommand(App::new("multi").version("1.0")));
-
-    assert!(utils::compare_output(
-        app,
-        "myapp help subcmd multi",
-        VERSION_ABOUT_MULTI_SC,
-        false
-    ));
-}
-
-#[test]
-fn version_about_multi_subcmd_short_flag() {
-    let app = App::new("myapp")
-        .version_about("Print custom version about text")
-        .subcommand(App::new("subcmd").subcommand(App::new("multi").version("1.0")));
-
-    assert!(utils::compare_output(
-        app,
-        "myapp subcmd multi -h",
-        VERSION_ABOUT_MULTI_SC,
-        false
-    ));
-}
-
-#[test]
-fn version_about_multi_subcmd_long_flag() {
-    let app = App::new("myapp")
-        .version_about("Print custom version about text")
-        .subcommand(App::new("subcmd").subcommand(App::new("multi").version("1.0")));
-
-    assert!(utils::compare_output(
-        app,
-        "myapp subcmd multi --help",
-        VERSION_ABOUT_MULTI_SC,
-        false
-    ));
-}
-
-#[test]
-fn version_about_multi_subcmd_override() {
-    let app = App::new("myapp")
-        .version_about("Print custom version about text")
-        .subcommand(
-            App::new("subcmd").subcommand(
-                App::new("multi")
-                    .version("1.0")
-                    .version_about("Print custom version about text from multi"),
-            ),
-        );
-
-    assert!(utils::compare_output(
-        app,
-        "myapp help subcmd multi",
-        VERSION_ABOUT_MULTI_SC_OVERRIDE,
-        false
-    ));
-}
-
-#[test]
-fn version_about_multi_subcmd_override_short_flag() {
-    let app = App::new("myapp")
-        .version_about("Print custom version about text")
-        .subcommand(
-            App::new("subcmd").subcommand(
-                App::new("multi")
-                    .version("1.0")
-                    .version_about("Print custom version about text from multi"),
-            ),
-        );
-
-    assert!(utils::compare_output(
-        app,
-        "myapp subcmd multi -h",
-        VERSION_ABOUT_MULTI_SC_OVERRIDE,
-        false
-    ));
-}
-
-#[test]
-fn version_about_multi_subcmd_override_long_flag() {
-    let app = App::new("myapp")
-        .version_about("Print custom version about text")
-        .subcommand(
-            App::new("subcmd").subcommand(
-                App::new("multi")
-                    .version("1.0")
-                    .version_about("Print custom version about text from multi"),
-            ),
-        );
-
-    assert!(utils::compare_output(
-        app,
-        "myapp subcmd multi --help",
-        VERSION_ABOUT_MULTI_SC_OVERRIDE,
-        false
-    ));
-}
-
 #[test]
 fn option_usage_order() {
     let app = App::new("order").args(&[
diff --git a/tests/version.rs b/tests/version.rs
index 944cc29d..3c289bd9 100644
--- a/tests/version.rs
+++ b/tests/version.rs
@@ -2,7 +2,7 @@ mod utils;
 
 use std::str;
 
-use clap::{App, AppSettings, Arg, ErrorKind};
+use clap::{App, Arg, ErrorKind};
 
 static VERSION: &str = "clap-test v1.4.8\n";
 
@@ -102,18 +102,167 @@ fn prefer_user_subcmd_version_short() {
         .is_present("version1"));
 }
 
+static OVERRIDE_VERSION_SHORT: &str = "test 1.3
+
+USAGE:
+    test
+
+FLAGS:
+    -h, --help       Prints help information
+    -v, --version    Prints version information";
+
 #[test]
-fn override_ver() {
-    let m = App::new("test")
-        .setting(AppSettings::NoAutoVersion)
-        .author("Kevin K.")
-        .about("tests stuff")
+fn override_version_short() {
+    let app = App::new("test")
+        .version("1.3")
+        .mut_arg("version", |a| a.short('v'));
+
+    let m = app.clone().try_get_matches_from(vec!["test", "-v"]);
+
+    assert!(m.is_err());
+    let err = m.unwrap_err();
+    assert_eq!(err.kind, ErrorKind::DisplayVersion);
+    assert_eq!(err.to_string(), "test 1.3\n");
+
+    let m = app.clone().try_get_matches_from(vec!["test", "--version"]);
+
+    assert!(m.is_err());
+    let err = m.unwrap_err();
+    assert_eq!(err.kind, ErrorKind::DisplayVersion);
+    assert_eq!(err.to_string(), "test 1.3\n");
+
+    assert!(utils::compare_output(
+        app,
+        "test -h",
+        OVERRIDE_VERSION_SHORT,
+        false
+    ));
+}
+
+static OVERRIDE_VERSION_LONG: &str = "test 1.3
+
+USAGE:
+    test [FLAGS]
+
+FLAGS:
+    -h, --help    Prints help information
+    -V, --vers    Prints version information";
+
+#[test]
+fn override_version_long() {
+    let app = App::new("test")
+        .version("1.3")
+        .mut_arg("version", |a| a.long("vers"));
+
+    let m = app.clone().try_get_matches_from(vec!["test", "-V"]);
+
+    assert!(m.is_err());
+    let err = m.unwrap_err();
+    assert_eq!(err.kind, ErrorKind::DisplayVersion);
+    assert_eq!(err.to_string(), "test 1.3\n");
+
+    let m = app.clone().try_get_matches_from(vec!["test", "--vers"]);
+
+    assert!(m.is_err());
+    let err = m.unwrap_err();
+    assert_eq!(err.kind, ErrorKind::DisplayVersion);
+    assert_eq!(err.to_string(), "test 1.3\n");
+
+    assert!(utils::compare_output(
+        app,
+        "test -h",
+        OVERRIDE_VERSION_LONG,
+        false
+    ));
+}
+
+static OVERRIDE_VERSION_ABOUT: &str = "test 1.3
+
+USAGE:
+    test
+
+FLAGS:
+    -h, --help       Prints help information
+    -V, --version    version info";
+
+#[test]
+fn override_version_about() {
+    let app = App::new("test")
         .version("1.3")
-        .mut_arg("version", |a| {
-            a.short('v').long("version").about("some version")
-        })
-        .try_get_matches_from(vec!["test", "--version"]);
+        .mut_arg("version", |a| a.about("version info"));
+
+    assert!(utils::compare_output(
+        app.clone(),
+        "test -h",
+        OVERRIDE_VERSION_ABOUT,
+        false
+    ));
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        OVERRIDE_VERSION_ABOUT,
+        false
+    ));
+}
+
+static VERSION_ABOUT_MULTI_SC: &str = "myapp-subcmd-multi 1.0
+
+USAGE:
+    myapp subcmd multi
+
+FLAGS:
+    -h, --help       Prints help information
+    -V, --version    Print custom version about text";
+
+#[test]
+fn version_about_multi_subcmd() {
+    let app = App::new("myapp")
+        .mut_arg("version", |a| a.about("Print custom version about text"))
+        .subcommand(App::new("subcmd").subcommand(App::new("multi").version("1.0")));
+
+    assert!(utils::compare_output(
+        app.clone(),
+        "myapp help subcmd multi",
+        VERSION_ABOUT_MULTI_SC,
+        false
+    ));
+    assert!(utils::compare_output(
+        app.clone(),
+        "myapp subcmd multi -h",
+        VERSION_ABOUT_MULTI_SC,
+        false
+    ));
+    assert!(utils::compare_output(
+        app,
+        "myapp subcmd multi --help",
+        VERSION_ABOUT_MULTI_SC,
+        false
+    ));
+}
+
+static VERSION_ABOUT_MULTI_SC_OVERRIDE: &str = "myapp-subcmd-multi 1.0
+
+USAGE:
+    myapp subcmd multi
+
+FLAGS:
+    -h, --help       Prints help information
+    -V, --version    Print custom version about text from multi";
+
+#[test]
+fn version_about_multi_subcmd_override() {
+    let app = App::new("myapp")
+        .mut_arg("version", |a| a.about("Print custom version about text"))
+        .subcommand(App::new("subcmd").subcommand(
+            App::new("multi").version("1.0").mut_arg("version", |a| {
+                a.about("Print custom version about text from multi")
+            }),
+        ));
 
-    assert!(m.is_ok(), "{:?}", m.unwrap_err().kind);
-    assert!(m.unwrap().is_present("version"));
+    assert!(utils::compare_output(
+        app,
+        "myapp help subcmd multi",
+        VERSION_ABOUT_MULTI_SC_OVERRIDE,
+        false
+    ));
 }