Merge pull request #3677 from epage/multicall

fix(multicall): Polish user messages

examples/repl.rs

@@ -66,7 +66,6 @@ fn cli() -> Command<'static> {
     Command::new("repl")
         .multicall(true)
         .arg_required_else_help(true)
-        .disable_help_flag(true)
         .subcommand_required(true)
         .subcommand_value_name("APPLET")
         .subcommand_help_heading("APPLETS")

src/build/command.rs

@@ -3640,6 +3640,11 @@ impl<'help> App<'help> {
     pub fn is_multicall_set(&self) -> bool {
         self.is_set(AppSettings::Multicall)
     }
+
+    #[cfg(not(feature = "unstable-multicall"))]
+    fn is_multicall_set(&self) -> bool {
+        false
+    }
 }
 
 /// Deprecated
@@ -4025,6 +4030,15 @@ impl<'help> App<'help> {
             // Make sure all the globally set flags apply to us as well
             self.settings = self.settings | self.g_settings;
 
+            #[cfg(feature = "unstable-multicall")]
+            {
+                if self.is_multicall_set() {
+                    self.settings.insert(AppSettings::SubcommandRequired.into());
+                    self.settings.insert(AppSettings::DisableHelpFlag.into());
+                    self.settings.insert(AppSettings::DisableVersionFlag.into());
+                }
+            }
+
             self._propagate();
             self._check_help_and_version();
             self._propagate_global_args();
@@ -4142,6 +4156,14 @@ impl<'help> App<'help> {
                 }
             }
 
+            let self_bin_name =
+                if cfg!(feature = "unstable-multicall") && self.is_multicall_set() {
+                    self.bin_name.as_deref().unwrap_or("")
+                } else {
+                    self.bin_name.as_deref().unwrap_or(&self.name)
+                }
+                .to_owned();
+
             for mut sc in &mut self.subcommands {
                 debug!("Command::_build_bin_names:iter: bin_name set...");
 
@@ -4163,12 +4185,7 @@ impl<'help> App<'help> {
                         sc_names = format!("{{{}}}", sc_names);
                     }
 
-                    let usage_name = format!(
-                        "{}{}{}",
-                        self.bin_name.as_ref().unwrap_or(&self.name),
-                        mid_string,
-                        sc_names
-                    );
+                    let usage_name = format!("{}{}{}", self_bin_name, mid_string, sc_names);
                     debug!(
                         "Command::_build_bin_names:iter: Setting usage_name of {} to {:?}",
                         sc.name, usage_name
@@ -4183,8 +4200,9 @@ impl<'help> App<'help> {
 
                 if sc.bin_name.is_none() {
                     let bin_name = format!(
-                        "{} {}",
-                        self.bin_name.as_ref().unwrap_or(&self.name),
+                        "{}{}{}",
+                        self_bin_name,
+                        if !self_bin_name.is_empty() { " " } else { "" },
                         &*sc.name
                     );
                     debug!(

src/build/debug_asserts.rs

@@ -56,6 +56,16 @@ pub(crate) fn assert_app(cmd: &Command) {
     for arg in cmd.get_arguments() {
         assert_arg(arg);
 
+        #[cfg(feature = "unstable-multicall")]
+        {
+            assert!(
+                !cmd.is_multicall_set(),
+                "Command {}: Arguments like {} cannot be set on a multicall command",
+                cmd.get_name(),
+                arg.name
+            );
+        }
+
         if let Some(s) = arg.short.as_ref() {
             short_flags.push(Flag::Arg(format!("-{}", s), &*arg.name));
         }

src/error/mod.rs

@@ -386,9 +386,8 @@ impl Error {
             ])
     }
 
-    pub(crate) fn unrecognized_subcommand(cmd: &Command, subcmd: String, name: String) -> Self {
+    pub(crate) fn unrecognized_subcommand(cmd: &Command, subcmd: String, usage: String) -> Self {
         let info = vec![subcmd.clone()];
-        let usage = format!("USAGE:\n    {} <subcommands>", name);
         Self::new(ErrorKind::UnrecognizedSubcommand)
             .with_cmd(cmd)
             .set_info(info)

src/output/usage.rs

@@ -154,7 +154,7 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
                 usage.push(']');
             }
         }
-        usage.shrink_to_fit();
+        let usage = usage.trim().to_owned();
         debug!("Usage::create_help_usage: usage={}", usage);
         usage
     }

src/parse/parser.rs

@@ -529,10 +529,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
             return ClapError::unrecognized_subcommand(
                 self.cmd,
                 arg_os.display().to_string(),
-                self.cmd
-                    .get_bin_name()
-                    .unwrap_or_else(|| self.cmd.get_name())
-                    .to_owned(),
+                Usage::new(self.cmd).create_usage_with_title(&[]),
             );
         }
         ClapError::unknown_argument(
@@ -626,9 +623,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                     return Err(ClapError::unrecognized_subcommand(
                         sc,
                         cmd.to_string_lossy().into_owned(),
-                        sc.get_bin_name()
-                            .unwrap_or_else(|| sc.get_name())
-                            .to_owned(),
+                        Usage::new(sc).create_usage_with_title(&[]),
                     ));
                 };
             }

tests/builder/help.rs

@@ -683,7 +683,7 @@ fn help_multi_subcommand_error() {
     static EXPECTED: &str = "error: The subcommand 'foo' wasn't recognized
 
 USAGE:
-    ctest subcmd multi <subcommands>
+    ctest subcmd multi [OPTIONS]
 
 For more information try --help
 ";

tests/builder/subcommands.rs

@@ -486,7 +486,7 @@ fn subcommand_not_recognized() {
         "error: The subcommand 'help' wasn't recognized
 
 USAGE:
-    fake <subcommands>
+    fake [SUBCOMMAND]
 
 For more information try --help
 ",
@@ -517,7 +517,7 @@ fn busybox_like_multicall() {
 
     let m = cmd.clone().try_get_matches_from(&["a.out"]);
     assert!(m.is_err());
-    assert_eq!(m.unwrap_err().kind(), ErrorKind::UnknownArgument);
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::UnrecognizedSubcommand);
 }
 
 #[cfg(feature = "unstable-multicall")]
@@ -539,7 +539,7 @@ fn hostname_like_multicall() {
 
     let m = cmd.clone().try_get_matches_from(&["a.out"]);
     assert!(m.is_err());
-    assert_eq!(m.unwrap_err().kind(), ErrorKind::UnknownArgument);
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::UnrecognizedSubcommand);
 
     let m = cmd.try_get_matches_from_mut(&["hostname", "hostname"]);
     assert!(m.is_err());
@@ -549,3 +549,148 @@ fn hostname_like_multicall() {
     assert!(m.is_err());
     assert_eq!(m.unwrap_err().kind(), ErrorKind::UnknownArgument);
 }
+
+#[cfg(feature = "unstable-multicall")]
+#[test]
+fn bad_multicall_command_error() {
+    let cmd = Command::new("repl")
+        .version("1.0.0")
+        .propagate_version(true)
+        .multicall(true)
+        .subcommand(Command::new("foo"))
+        .subcommand(Command::new("bar"));
+
+    let err = cmd.clone().try_get_matches_from(&["world"]).unwrap_err();
+    assert_eq!(err.kind(), ErrorKind::UnrecognizedSubcommand);
+    static HELLO_EXPECTED: &str = "\
+error: The subcommand 'world' wasn't recognized
+
+USAGE:
+    <SUBCOMMAND>
+
+For more information try help
+";
+    utils::assert_eq(HELLO_EXPECTED, err.to_string());
+
+    let err = cmd.clone().try_get_matches_from(&["baz"]).unwrap_err();
+    assert_eq!(err.kind(), ErrorKind::InvalidSubcommand);
+    static BAZ_EXPECTED: &str = "\
+error: The subcommand 'baz' wasn't recognized
+
+\tDid you mean 'bar'?
+
+If you believe you received this message in error, try re-running with ' -- baz'
+
+USAGE:
+    <SUBCOMMAND>
+
+For more information try help
+";
+    utils::assert_eq(BAZ_EXPECTED, err.to_string());
+
+    // Verify whatever we did to get the above to work didn't disable `--help` and `--version`.
+
+    let err = cmd
+        .clone()
+        .try_get_matches_from(&["foo", "--help"])
+        .unwrap_err();
+    assert_eq!(err.kind(), ErrorKind::DisplayHelp);
+
+    let err = cmd
+        .clone()
+        .try_get_matches_from(&["foo", "--version"])
+        .unwrap_err();
+    assert_eq!(err.kind(), ErrorKind::DisplayVersion);
+}
+
+#[cfg(feature = "unstable-multicall")]
+#[test]
+#[should_panic = "Command repl: Arguments like oh-no cannot be set on a multicall command"]
+fn cant_have_args_with_multicall() {
+    let mut cmd = Command::new("repl")
+        .version("1.0.0")
+        .propagate_version(true)
+        .multicall(true)
+        .subcommand(Command::new("foo"))
+        .subcommand(Command::new("bar"))
+        .arg(Arg::new("oh-no"));
+    cmd.build();
+}
+
+#[cfg(feature = "unstable-multicall")]
+#[test]
+fn multicall_help_flag() {
+    static EXPECTED: &str = "\
+foo-bar 1.0.0
+
+USAGE:
+    foo bar [value]
+
+ARGS:
+    <value>    
+
+OPTIONS:
+    -h, --help       Print help information
+    -V, --version    Print version information
+";
+    let cmd = Command::new("repl")
+        .version("1.0.0")
+        .propagate_version(true)
+        .multicall(true)
+        .subcommand(Command::new("foo").subcommand(Command::new("bar").arg(Arg::new("value"))));
+    utils::assert_output(cmd, "foo bar --help", EXPECTED, false);
+}
+
+#[cfg(feature = "unstable-multicall")]
+#[test]
+fn multicall_help_subcommand() {
+    static EXPECTED: &str = "\
+foo-bar 1.0.0
+
+USAGE:
+    foo bar [value]
+
+ARGS:
+    <value>    
+
+OPTIONS:
+    -h, --help       Print help information
+    -V, --version    Print version information
+";
+    let cmd = Command::new("repl")
+        .version("1.0.0")
+        .propagate_version(true)
+        .multicall(true)
+        .subcommand(Command::new("foo").subcommand(Command::new("bar").arg(Arg::new("value"))));
+    utils::assert_output(cmd, "help foo bar", EXPECTED, false);
+}
+
+#[cfg(feature = "unstable-multicall")]
+#[test]
+fn multicall_render_help() {
+    static EXPECTED: &str = "\
+foo-bar 1.0.0
+
+USAGE:
+    foo bar [value]
+
+ARGS:
+    <value>    
+
+OPTIONS:
+    -h, --help       Print help information
+    -V, --version    Print version information
+";
+    let mut cmd = Command::new("repl")
+        .version("1.0.0")
+        .propagate_version(true)
+        .multicall(true)
+        .subcommand(Command::new("foo").subcommand(Command::new("bar").arg(Arg::new("value"))));
+    cmd.build();
+    let subcmd = cmd.find_subcommand_mut("foo").unwrap();
+    let subcmd = subcmd.find_subcommand_mut("bar").unwrap();
+
+    let mut buf = Vec::new();
+    subcmd.write_help(&mut buf).unwrap();
+    utils::assert_eq(EXPECTED, String::from_utf8(buf).unwrap());
+}