Repository: clap-rs/clap
Pull request #2336 (2021-02-08 05:04:09)

Better help message support for hidden and heading stuff
Based on #2211, this PR fixes quite a few bugs in the help message generation regarding hidden args and the args with custom help headings.

Only the last commit is new. I will rebase this branch once #2211 is merged.


Patch:
diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs
index d7335eac..a5a0c2eb 100644
--- a/src/build/app/mod.rs
+++ b/src/build/app/mod.rs
@@ -196,7 +196,7 @@ impl<'help> App<'help> {
         self.args.args()
     }
 
-    /// Get the list of *positional* arguments.
+    /// Iterate through the *positionals*.
     #[inline]
     pub fn get_positionals(&self) -> impl Iterator<Item = &Arg<'help>> {
         self.get_arguments().filter(|a| a.is_positional())
@@ -214,6 +214,12 @@ impl<'help> App<'help> {
             .filter(|a| a.is_set(ArgSettings::TakesValue) && a.get_index().is_none())
     }
 
+    /// Iterate through the *positionals* that don't have custom heading.
+    pub fn get_positionals_with_no_heading(&self) -> impl Iterator<Item = &Arg<'help>> {
+        self.get_positionals()
+            .filter(|a| a.get_help_heading().is_none())
+    }
+
     /// Iterate through the *flags* that don't have custom heading.
     pub fn get_flags_with_no_heading(&self) -> impl Iterator<Item = &Arg<'help>> {
         self.get_flags().filter(|a| a.get_help_heading().is_none())
@@ -2646,14 +2652,8 @@ impl<'help> App<'help> {
         !self.args.is_empty()
     }
 
-    #[inline]
-    pub(crate) fn has_opts(&self) -> bool {
-        self.get_opts_with_no_heading().count() > 0
-    }
-
-    #[inline]
-    pub(crate) fn has_flags(&self) -> bool {
-        self.get_flags_with_no_heading().count() > 0
+    pub(crate) fn has_positionals(&self) -> bool {
+        self.args.keys().any(|x| x.is_position())
     }
 
     pub(crate) fn has_visible_subcommands(&self) -> bool {
diff --git a/src/output/help.rs b/src/output/help.rs
index 2f2ebbb8..360b66ac 100644
--- a/src/output/help.rs
+++ b/src/output/help.rs
@@ -110,10 +110,22 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
         } else if let Some(tmpl) = self.parser.app.template {
             self.write_templated_help(tmpl)?;
         } else {
-            let flags = self.parser.has_flags();
-            let pos = self.parser.has_positionals();
-            let opts = self.parser.has_opts();
-            let subcmds = self.parser.has_subcommands();
+            let pos = self
+                .parser
+                .app
+                .get_positionals()
+                .any(|arg| should_show_arg(self.use_long, arg));
+            let flags = self
+                .parser
+                .app
+                .get_flags()
+                .any(|arg| should_show_arg(self.use_long, arg));
+            let opts = self
+                .parser
+                .app
+                .get_opts()
+                .any(|arg| should_show_arg(self.use_long, arg));
+            let subcmds = self.parser.app.has_visible_subcommands();
 
             if flags || opts || pos || subcmds {
                 self.write_templated_help(Self::DEFAULT_TEMPLATE)?;
@@ -740,20 +752,25 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
     /// including titles of a Parser Object to the wrapped stream.
     pub(crate) fn write_all_args(&mut self) -> io::Result<()> {
         debug!("Help::write_all_args");
-        let flags = self.parser.has_flags();
         let pos = self
             .parser
             .app
-            .get_positionals()
+            .get_positionals_with_no_heading()
             .filter(|arg| should_show_arg(self.use_long, arg))
-            .any(|_| true);
+            .collect::<Vec<_>>();
+        let flags = self
+            .parser
+            .app
+            .get_flags_with_no_heading()
+            .filter(|arg| should_show_arg(self.use_long, arg))
+            .collect::<Vec<_>>();
         let opts = self
             .parser
             .app
             .get_opts_with_no_heading()
             .filter(|arg| should_show_arg(self.use_long, arg))
             .collect::<Vec<_>>();
-        let subcmds = self.parser.has_visible_subcommands();
+        let subcmds = self.parser.app.has_visible_subcommands();
 
         let custom_headings = self
             .parser
@@ -763,10 +780,10 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
             .filter_map(|arg| arg.help_heading)
             .collect::<IndexSet<_>>();
 
-        let mut first = if pos {
+        let mut first = if !pos.is_empty() {
             // Write positional args if any
             self.warning("ARGS:\n")?;
-            self.write_args_unsorted(&self.parser.app.get_positionals().collect::<Vec<_>>())?;
+            self.write_args_unsorted(&pos)?;
             false
         } else {
             true
@@ -774,7 +791,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
 
         let unified_help = self.parser.is_set(AppSettings::UnifiedHelpMessage);
 
-        if unified_help && (flags || !opts.is_empty()) {
+        if unified_help && (!flags.is_empty() || !opts.is_empty()) {
             let opts_flags = self
                 .parser
                 .app
@@ -789,13 +806,12 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
             self.write_args(&*opts_flags)?;
             first = false;
         } else {
-            if flags {
+            if !flags.is_empty() {
                 if !first {
                     self.none("\n\n")?;
                 }
                 self.warning("FLAGS:\n")?;
-                let flags_v: Vec<_> = self.parser.app.get_flags_with_no_heading().collect();
-                self.write_args(&flags_v)?;
+                self.write_args(&flags)?;
                 first = false;
             }
             if !opts.is_empty() {
diff --git a/src/output/usage.rs b/src/output/usage.rs
index d6f51b8d..3af10fc7 100644
--- a/src/output/usage.rs
+++ b/src/output/usage.rs
@@ -278,7 +278,7 @@ impl<'help, 'app, 'parser> Usage<'help, 'app, 'parser> {
                 pos.multiple_str()
             ))
         } else if self.p.is_set(AS::DontCollapseArgsInUsage)
-            && self.p.has_positionals()
+            && self.p.app.has_positionals()
             && incl_reqs
         {
             debug!("Usage::get_args_tag:iter: Don't collapse returning all");
diff --git a/src/parse/parser.rs b/src/parse/parser.rs
index 581c1598..60038a3c 100644
--- a/src/parse/parser.rs
+++ b/src/parse/parser.rs
@@ -271,7 +271,7 @@ impl<'help, 'app> Parser<'help, 'app> {
             .app
             .get_positionals()
             .any(|p| p.is_set(ArgSettings::Last) && p.is_set(ArgSettings::Required))
-            && self.has_subcommands()
+            && self.app.has_subcommands()
             && !self.is_set(AS::SubcommandsNegateReqs)
         {
             panic!(
@@ -371,7 +371,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                 if self.is_new_arg(&arg_os, &needs_val_of) {
                     if arg_os == "--" {
                         debug!("Parser::get_matches_with: setting TrailingVals=true");
-                        self.set(AS::TrailingValues);
+                        self.app.set(AS::TrailingValues);
                         continue;
                     } else if arg_os.starts_with("--") {
                         needs_val_of = self.parse_long_arg(matcher, &arg_os, remaining_args)?;
@@ -647,7 +647,7 @@ impl<'help, 'app> Parser<'help, 'app> {
             );
         }
         // If the argument must be a subcommand.
-        if !self.has_args() || self.is_set(AS::InferSubcommands) && self.has_subcommands() {
+        if !self.app.has_args() || self.is_set(AS::InferSubcommands) && self.app.has_subcommands() {
             return ClapError::unrecognized_subcommand(
                 arg_os.to_string_lossy().to_string(),
                 self.app
@@ -1066,7 +1066,7 @@ impl<'help, 'app> Parser<'help, 'app> {
 
         // If AllowLeadingHyphen is set, we want to ensure `-val` gets parsed as `-val` and not
         // `-v` `-a` `-l` assuming `v` `a` and `l` are all, or mostly, valid shorts.
-        if self.is_set(AS::AllowLeadingHyphen) && arg.chars().any(|c| !self.contains_short(c)) {
+        if self.is_set(AS::AllowLeadingHyphen) && arg.chars().any(|c| !self.app.contains_short(c)) {
             debug!(
                 "Parser::parse_short_arg: LeadingHyphenAllowed yet -{} isn't valid",
                 arg
@@ -1612,39 +1612,7 @@ impl<'help, 'app> Parser<'help, 'app> {
 
 // Query Methods
 impl<'help, 'app> Parser<'help, 'app> {
-    fn contains_short(&self, s: char) -> bool {
-        self.app.contains_short(s)
-    }
-
-    pub(crate) fn has_args(&self) -> bool {
-        self.app.has_args()
-    }
-
-    pub(crate) fn has_opts(&self) -> bool {
-        self.app.has_opts()
-    }
-
-    pub(crate) fn has_flags(&self) -> bool {
-        self.app.has_flags()
-    }
-
-    pub(crate) fn has_positionals(&self) -> bool {
-        self.app.args.keys().any(|x| x.is_position())
-    }
-
-    pub(crate) fn has_subcommands(&self) -> bool {
-        self.app.has_subcommands()
-    }
-
-    pub(crate) fn has_visible_subcommands(&self) -> bool {
-        self.app.has_visible_subcommands()
-    }
-
     pub(crate) fn is_set(&self, s: AS) -> bool {
         self.app.is_set(s)
     }
-
-    pub(crate) fn set(&mut self, s: AS) {
-        self.app.set(s)
-    }
 }


Test patch:
diff --git a/tests/help.rs b/tests/help.rs
index 179f47fb..ec596a21 100644
--- a/tests/help.rs
+++ b/tests/help.rs
@@ -2220,3 +2220,197 @@ FLAGS:
         false
     ));
 }
+
+static ONLY_CUSTOM_HEADING_FLAGS: &'static str = "test 1.4
+
+USAGE:
+    test [OPTIONS]
+
+OPTIONS:
+        --speed <speed>    How fast
+
+NETWORKING:
+        --flag    Some flag";
+
+#[test]
+fn only_custom_heading_flags() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .mut_arg("help", |a| a.hidden(true))
+        .arg(
+            Arg::new("speed")
+                .long("speed")
+                .takes_value(true)
+                .about("How fast"),
+        )
+        .help_heading("NETWORKING")
+        .arg(Arg::new("flag").long("flag").about("Some flag"));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        ONLY_CUSTOM_HEADING_FLAGS,
+        false
+    ));
+}
+
+static ONLY_CUSTOM_HEADING_OPTS: &'static str = "test 1.4
+
+USAGE:
+    test
+
+FLAGS:
+    -h, --help    Prints help information
+
+NETWORKING:
+    -s, --speed <SPEED>    How fast";
+
+#[test]
+fn only_custom_heading_opts() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .help_heading("NETWORKING")
+        .arg(Arg::from("-s, --speed [SPEED] 'How fast'"));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        ONLY_CUSTOM_HEADING_OPTS,
+        false
+    ));
+}
+
+static CUSTOM_HEADING_POS: &'static str = "test 1.4
+
+USAGE:
+    test [ARGS]
+
+ARGS:
+    <gear>    Which gear
+
+FLAGS:
+    -h, --help    Prints help information
+
+NETWORKING:
+    <speed>    How fast";
+
+#[test]
+fn custom_heading_pos() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .arg(Arg::new("gear").about("Which gear"))
+        .help_heading("NETWORKING")
+        .arg(Arg::new("speed").about("How fast"));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        CUSTOM_HEADING_POS,
+        false
+    ));
+}
+
+static ONLY_CUSTOM_HEADING_POS: &'static str = "test 1.4
+
+USAGE:
+    test [speed]
+
+FLAGS:
+    -h, --help    Prints help information
+
+NETWORKING:
+    <speed>    How fast";
+
+#[test]
+fn only_custom_heading_pos() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .help_heading("NETWORKING")
+        .arg(Arg::new("speed").about("How fast"));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        ONLY_CUSTOM_HEADING_POS,
+        false
+    ));
+}
+
+static ONLY_CUSTOM_HEADING_FLAGS_NO_ARGS: &'static str = "test 1.4
+
+USAGE:
+    test
+
+NETWORKING:
+        --flag    Some flag";
+
+#[test]
+fn only_custom_heading_flags_no_args() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .mut_arg("help", |a| a.hidden(true))
+        .help_heading("NETWORKING")
+        .arg(Arg::new("flag").long("flag").about("Some flag"));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        ONLY_CUSTOM_HEADING_FLAGS_NO_ARGS,
+        false
+    ));
+}
+
+static ONLY_CUSTOM_HEADING_OPTS_NO_ARGS: &'static str = "test 1.4
+
+USAGE:
+    test
+
+NETWORKING:
+    -s, --speed <SPEED>    How fast";
+
+#[test]
+fn only_custom_heading_opts_no_args() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .mut_arg("help", |a| a.hidden(true))
+        .help_heading("NETWORKING")
+        .arg(Arg::from("-s, --speed [SPEED] 'How fast'"));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        ONLY_CUSTOM_HEADING_OPTS_NO_ARGS,
+        false
+    ));
+}
+
+static ONLY_CUSTOM_HEADING_POS_NO_ARGS: &'static str = "test 1.4
+
+USAGE:
+    test [speed]
+
+NETWORKING:
+    <speed>    How fast";
+
+#[test]
+fn only_custom_heading_pos_no_args() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .mut_arg("help", |a| a.hidden(true))
+        .help_heading("NETWORKING")
+        .arg(Arg::new("speed").about("How fast"));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        ONLY_CUSTOM_HEADING_POS_NO_ARGS,
+        false
+    ));
+}
diff --git a/tests/hidden_args.rs b/tests/hidden_args.rs
index 27318f75..7a1ae0b5 100644
--- a/tests/hidden_args.rs
+++ b/tests/hidden_args.rs
@@ -1,6 +1,6 @@
 mod utils;
 
-use clap::{App, Arg};
+use clap::{App, AppSettings, Arg};
 
 static HIDDEN_ARGS: &str = "test 1.4
 Kevin K.
@@ -204,3 +204,201 @@ fn hidden_long_args_short_help() {
         false
     ));
 }
+
+static HIDDEN_FLAG_ARGS: &str = "test 1.4
+
+USAGE:
+    test [OPTIONS]
+
+OPTIONS:
+        --option <opt>    some option";
+
+#[test]
+fn hidden_flag_args() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .mut_arg("help", |a| a.hidden(true))
+        .args(&[Arg::from("--option [opt] 'some option'")]);
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        HIDDEN_FLAG_ARGS,
+        false
+    ));
+}
+
+static HIDDEN_OPT_ARGS: &str = "test 1.4
+
+USAGE:
+    test [FLAGS]
+
+FLAGS:
+        --flag    some flag
+    -h, --help    Prints help information";
+
+#[test]
+fn hidden_opt_args() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .args(&[
+            Arg::from("--flag 'some flag'"),
+            Arg::from("--option [opt] 'some option'").hidden(true),
+        ]);
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        HIDDEN_OPT_ARGS,
+        false
+    ));
+}
+
+static HIDDEN_POS_ARGS: &str = "test 1.4
+
+USAGE:
+    test [another]
+
+ARGS:
+    <another>    another pos
+
+FLAGS:
+    -h, --help    Prints help information";
+
+#[test]
+fn hidden_pos_args() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .args(&[
+            Arg::new("pos").about("some pos").hidden(true),
+            Arg::new("another").about("another pos"),
+        ]);
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        HIDDEN_POS_ARGS,
+        false
+    ));
+}
+
+static HIDDEN_SUBCMDS: &str = "test 1.4
+
+USAGE:
+    test
+
+FLAGS:
+    -h, --help    Prints help information";
+
+#[test]
+fn hidden_subcmds() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .subcommand(App::new("sub").setting(AppSettings::Hidden));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        HIDDEN_SUBCMDS,
+        false
+    ));
+}
+
+static HIDDEN_FLAG_ARGS_ONLY: &str = "test 1.4
+
+USAGE:
+    test
+
+After help";
+
+#[test]
+fn hidden_flag_args_only() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .after_help("After help")
+        .mut_arg("help", |a| a.hidden(true));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        HIDDEN_FLAG_ARGS_ONLY,
+        false
+    ));
+}
+
+static HIDDEN_OPT_ARGS_ONLY: &str = "test 1.4
+
+USAGE:
+    test
+
+After help";
+
+#[test]
+fn hidden_opt_args_only() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .after_help("After help")
+        .mut_arg("help", |a| a.hidden(true))
+        .args(&[Arg::from("--option [opt] 'some option'").hidden(true)]);
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        HIDDEN_OPT_ARGS_ONLY,
+        false
+    ));
+}
+
+static HIDDEN_POS_ARGS_ONLY: &str = "test 1.4
+
+USAGE:
+    test
+
+After help";
+
+#[test]
+fn hidden_pos_args_only() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .after_help("After help")
+        .mut_arg("help", |a| a.hidden(true))
+        .args(&[Arg::new("pos").about("some pos").hidden(true)]);
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        HIDDEN_POS_ARGS_ONLY,
+        false
+    ));
+}
+
+static HIDDEN_SUBCMDS_ONLY: &str = "test 1.4
+
+USAGE:
+    test
+
+After help";
+
+#[test]
+fn hidden_subcmds_only() {
+    let app = App::new("test")
+        .version("1.4")
+        .setting(AppSettings::DisableVersionFlag)
+        .after_help("After help")
+        .mut_arg("help", |a| a.hidden(true))
+        .subcommand(App::new("sub").setting(AppSettings::Hidden));
+
+    assert!(utils::compare_output(
+        app,
+        "test --help",
+        HIDDEN_SUBCMDS_ONLY,
+        false
+    ));
+}