Merge pull request #3041 from fishface60/master

Rewrite Multicall handling to just strip path off argv0

examples/multicall_busybox.md

@@ -6,36 +6,36 @@ See the documentation for clap::AppSettings::Multicall for rationale.
 
 This example omits every command except true and false,
 which are the most trivial to implement,
-```bash,ignore
+```bash
 $ busybox true
 ? 0
 $ busybox false
 ? 1
 ```
-*Note: without the links setup, we can't demonostrate the multicall behavior*
+*Note: without the links setup, we can't demonstrate the multicall behavior*
 
 But includes the `--install` option as an example of why it can be useful
 for the main program to take arguments that aren't applet subcommands.
-```bash,ignore
+```bash
 $ busybox --install
 ? failed
 ...
 ```
 
 Though users must pass something:
-```bash,ignore
+```bash
 $ busybox
 ? failed
 busybox 
 
 USAGE:
-    busybox[EXE] [OPTIONS] [SUBCOMMAND]
+    busybox [OPTIONS] [APPLET]
 
 OPTIONS:
     -h, --help                 Print help information
         --install <install>    Install hardlinks for all subcommands in path
 
-SUBCOMMANDS:
+APPLETS:
     false    does nothing unsuccessfully
     help     Print this message or the help of the given subcommand(s)
     true     does nothing successfully

examples/multicall_busybox.rs

@@ -2,32 +2,45 @@ use std::process::exit;
 
 use clap::{App, AppSettings, Arg};
 
+fn applet_commands() -> [App<'static>; 2] {
+    [
+        App::new("true").about("does nothing successfully"),
+        App::new("false").about("does nothing unsuccessfully"),
+    ]
+}
+
 fn main() {
     let app = App::new(env!("CARGO_CRATE_NAME"))
-        .setting(AppSettings::ArgRequiredElseHelp)
-        .subcommand_value_name("APPLET")
-        .subcommand_help_heading("APPLETS")
-        .arg(
-            Arg::new("install")
-                .long("install")
-                .help("Install hardlinks for all subcommands in path")
-                .exclusive(true)
-                .takes_value(true)
-                .default_missing_value("/usr/local/bin")
-                .use_delimiter(false),
+        .setting(AppSettings::Multicall)
+        .subcommand(
+            App::new("busybox")
+                .setting(AppSettings::ArgRequiredElseHelp)
+                .subcommand_value_name("APPLET")
+                .subcommand_help_heading("APPLETS")
+                .arg(
+                    Arg::new("install")
+                        .long("install")
+                        .help("Install hardlinks for all subcommands in path")
+                        .exclusive(true)
+                        .takes_value(true)
+                        .default_missing_value("/usr/local/bin")
+                        .use_delimiter(false),
+                )
+                .subcommands(applet_commands()),
         )
-        .subcommand(App::new("true").about("does nothing successfully"))
-        .subcommand(App::new("false").about("does nothing unsuccessfully"));
+        .subcommands(applet_commands());
 
-    let app = app.setting(AppSettings::Multicall);
     let matches = app.get_matches();
-    if matches.occurrences_of("install") > 0 {
-        unimplemented!("Make hardlinks to the executable here");
+    let mut subcommand = matches.subcommand();
+    if let Some(("busybox", cmd)) = subcommand {
+        if cmd.occurrences_of("install") > 0 {
+            unimplemented!("Make hardlinks to the executable here");
+        }
+        subcommand = cmd.subcommand();
     }
-
-    match matches.subcommand_name() {
-        Some("true") => exit(0),
-        Some("false") => exit(1),
+    match subcommand {
+        Some(("false", _)) => exit(1),
+        Some(("true", _)) => exit(0),
         _ => unreachable!("parser should ensure only valid subcommand names are used"),
     }
 }

examples/multicall_hostname.md

@@ -6,8 +6,8 @@ See the documentation for clap::AppSettings::Multicall for rationale.
 
 This example omits the implementation of displaying address config
 
-```bash,ignore
+```bash
 $ hostname
 www
 ```
-*Note: without the links setup, we can't demonostrate the multicall behavior*
+*Note: without the links setup, we can't demonstrate the multicall behavior*

src/build/app/mod.rs

@@ -607,47 +607,20 @@ impl<'help> App<'help> {
         if self.settings.is_set(AppSettings::Multicall) {
             if let Some((argv0, _)) = it.next() {
                 let argv0 = Path::new(&argv0);
-                if let Some(command) = argv0.file_name().and_then(|f| f.to_str()) {
+                if let Some(command) = argv0.file_stem().and_then(|f| f.to_str()) {
                     // Stop borrowing command so we can get another mut ref to it.
                     let command = command.to_owned();
                     debug!(
                         "App::try_get_matches_from_mut: Parsed command {} from argv",
                         command
                     );
 
-                    let subcommand = self
-                        .subcommands
-                        .iter_mut()
-                        .find(|subcommand| subcommand.aliases_to(&command));
-                    debug!(
-                        "App::try_get_matches_from_mut: Matched subcommand {:?}",
-                        subcommand
-                    );
-
-                    match subcommand {
-                        None if command == self.name => {
-                            debug!("App::try_get_matches_from_mut: no existing applet but matches name");
-                            debug!(
-                                "App::try_get_matches_from_mut: Setting bin_name to command name"
-                            );
-                            self.bin_name.get_or_insert(command);
-                            debug!(
-                                "App::try_get_matches_from_mut: Continuing with top-level parser."
-                            );
-                            return self._do_parse(&mut it);
-                        }
-                        _ => {
-                            debug!(
-                                "App::try_get_matches_from_mut: existing applet or no program name"
-                            );
-                            debug!("App::try_get_matches_from_mut: Reinserting command into arguments so subcommand parser matches it");
-                            it.insert(&[&command]);
-                            debug!("App::try_get_matches_from_mut: Clearing name and bin_name so that displayed command name starts with applet name");
-                            self.name.clear();
-                            self.bin_name = None;
-                            return self._do_parse(&mut it);
-                        }
-                    };
+                    debug!("App::try_get_matches_from_mut: Reinserting command into arguments so subcommand parser matches it");
+                    it.insert(&[&command]);
+                    debug!("App::try_get_matches_from_mut: Clearing name and bin_name so that displayed command name starts with applet name");
+                    self.name.clear();
+                    self.bin_name = None;
+                    return self._do_parse(&mut it);
                 }
             }
         };

src/build/app/settings.rs

@@ -390,10 +390,7 @@ pub enum AppSettings {
     /// [`ErrorKind::UnknownArgument`]: crate::ErrorKind::UnknownArgument
     AllowExternalSubcommands,
 
-    /// Parse the bin name (`argv[0]`) as a subcommand
-    ///
-    /// This adds a small performance penalty to startup
-    /// as it requires comparing the bin name against every subcommand name.
+    /// Strip directory path from argv\[0\] and use as an argument.
     ///
     /// A "multicall" executable is a single executable
     /// that contains a variety of applets,
@@ -411,10 +408,37 @@ pub enum AppSettings {
     ///
     /// # Examples
     ///
-    /// Multicall applets are defined as subcommands
-    /// to an app which has the Multicall setting enabled.
+    /// `hostname` is an example of a multicall executable.
+    /// Both `hostname` and `dnsdomainname` are provided by the same executable
+    /// and which behaviour to use is based on the executable file name.
+    ///
+    /// This is desirable when the executable has a primary purpose
+    /// but there is other related functionality that would be convenient to provide
+    /// and it is convenient for the code to implement it to be in the same executable.
+    ///
+    /// The name of the app is essentially unused
+    /// and may be the same as the name of a subcommand.
+    ///
+    /// The names of the immediate subcommands of the App
+    /// are matched against the basename of the first argument,
+    /// which is conventionally the path of the executable.
+    ///
+    /// This does not allow the subcommand to be passed as the first non-path argument.
+    ///
+    /// ```rust
+    /// # use clap::{App, AppSettings, ErrorKind};
+    /// let mut app = App::new("hostname")
+    ///     .setting(AppSettings::Multicall)
+    ///     .subcommand(App::new("hostname"))
+    ///     .subcommand(App::new("dnsdomainname"));
+    /// let m = app.try_get_matches_from_mut(&["/usr/bin/hostname", "dnsdomainname"]);
+    /// assert!(m.is_err());
+    /// assert_eq!(m.unwrap_err().kind, ErrorKind::UnknownArgument);
+    /// let m = app.get_matches_from(&["/usr/bin/dnsdomainname"]);
+    /// assert_eq!(m.subcommand_name(), Some("dnsdomainname"));
+    /// ```
     ///
-    /// Busybox is a common example of a "multicall" executable
+    /// Busybox is another common example of a multicall executable
     /// with a subcommmand for each applet that can be run directly,
     /// e.g. with the `cat` applet being run by running `busybox cat`,
     /// or with `cat` as a link to the `busybox` binary.
@@ -424,52 +448,41 @@ pub enum AppSettings {
     /// e.g. to test the applet without installing it
     /// or there may already be a command of that name installed.
     ///
+    /// To make an applet usable as both a multicall link and a subcommand
+    /// the subcommands must be defined both in the top-level App
+    /// and as subcommands of the "main" applet.
+    ///
     /// ```rust
     /// # use clap::{App, AppSettings};
+    /// fn applet_commands() -> [App<'static>; 2] {
+    ///     [App::new("true"), App::new("false")]
+    /// }
     /// let mut app = App::new("busybox")
     ///     .setting(AppSettings::Multicall)
-    ///     .subcommand(App::new("true"))
-    ///     .subcommand(App::new("false"));
+    ///     .subcommand(
+    ///         App::new("busybox")
+    ///             .subcommand_value_name("APPLET")
+    ///             .subcommand_help_heading("APPLETS")
+    ///             .subcommands(applet_commands()),
+    ///     )
+    ///     .subcommands(applet_commands());
     /// // When called from the executable's canonical name
     /// // its applets can be matched as subcommands.
-    /// let m = app.try_get_matches_from_mut(&["busybox", "true"]).unwrap();
-    /// assert_eq!(m.subcommand_name(), Some("true"));
+    /// let m = app.try_get_matches_from_mut(&["/usr/bin/busybox", "true"]).unwrap();
+    /// assert_eq!(m.subcommand_name(), Some("busybox"));
+    /// assert_eq!(m.subcommand().unwrap().1.subcommand_name(), Some("true"));
     /// // When called from a link named after an applet that applet is matched.
-    /// let m = app.get_matches_from(&["true"]);
+    /// let m = app.get_matches_from(&["/usr/bin/true"]);
     /// assert_eq!(m.subcommand_name(), Some("true"));
     /// ```
     ///
-    /// `hostname` is another example of a multicall executable.
-    /// It differs from busybox by not supporting running applets via subcommand
-    /// and is instead only runnable via links.
-    ///
-    /// This is desirable when the executable has a primary purpose
-    /// rather than being a collection of varied applets,
-    /// so it is appropriate to name the executable after its purpose,
-    /// but there is other related functionality that would be convenient to provide
-    /// and it is convenient for the code to implement it to be in the same executable.
-    ///
-    /// This behaviour can be opted-into
-    /// by naming a subcommand with the same as the program
-    /// as applet names take priority.
+    /// **NOTE:** Applets are slightly semantically different from subcommands,
+    /// so it's recommended to use [`App::subcommand_help_heading`] and
+    /// [`App::subcommand_value_name`] to change the descriptive text as above.
     ///
-    /// ```rust
-    /// # use clap::{App, AppSettings, ErrorKind};
-    /// let mut app = App::new("hostname")
-    ///     .setting(AppSettings::Multicall)
-    ///     .subcommand(App::new("hostname"))
-    ///     .subcommand(App::new("dnsdomainname"));
-    /// let m = app.try_get_matches_from_mut(&["hostname", "dnsdomainname"]);
-    /// assert!(m.is_err());
-    /// assert_eq!(m.unwrap_err().kind, ErrorKind::UnknownArgument);
-    /// let m = app.get_matches_from(&["hostname"]);
-    /// assert_eq!(m.subcommand_name(), Some("hostname"));
-    /// ```
-    ///
-    /// [`subcommands`]: crate::App::subcommand()
-    /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html
     /// [`NoBinaryName`]: crate::AppSettings::NoBinaryName
-    /// [`try_get_matches_from_mut`]: crate::App::try_get_matches_from_mut()
+    /// [`App::subcommand_value_name`]: crate::App::subcommand_value_name
+    /// [`App::subcommand_help_heading`]: crate::App::subcommand_help_heading
     #[cfg(feature = "unstable-multicall")]
     Multicall,
 

tests/builder/subcommands.rs

@@ -511,13 +511,17 @@ For more information try --help
 #[cfg(feature = "unstable-multicall")]
 #[test]
 fn busybox_like_multicall() {
+    fn applet_commands() -> [App<'static>; 2] {
+        [App::new("true"), App::new("false")]
+    }
     let app = App::new("busybox")
         .setting(AppSettings::Multicall)
-        .subcommand(App::new("true"))
-        .subcommand(App::new("false"));
+        .subcommand(App::new("busybox").subcommands(applet_commands()))
+        .subcommands(applet_commands());
 
     let m = app.clone().get_matches_from(&["busybox", "true"]);
-    assert_eq!(m.subcommand_name(), Some("true"));
+    assert_eq!(m.subcommand_name(), Some("busybox"));
+    assert_eq!(m.subcommand().unwrap().1.subcommand_name(), Some("true"));
 
     let m = app.clone().get_matches_from(&["true"]);
     assert_eq!(m.subcommand_name(), Some("true"));