Repository: clap-rs/clap (rust)

Issue:
Use a default template instead of separate code path in Help::write_help
As @kitlith noticed in https://github.com/clap-rs/clap/pull/1986#issuecomment-653928929,:

> Using a template vs not using a template go through two somewhat different code-paths, right? You don't internally use a template unless the user provides one.

In other words, instead of using some sort of hardcoded template, we use a separate function that acts like a hardcoded template.

I think that it worth exploring simply using a hardcoded template instead of a separate code path. We may also export this template as `pub static: &str DEFAULT_TEMPLATE`, but I'm opposed to considering _precise contents_ of the default template as an API contract as we may (and probably will) tweak it here and there.

Related code:
https://github.com/clap-rs/clap/blob/9fc8ec965e42db926ef6aa894810c23600dac49d/src/output/help.rs#L110-L115

The function in question:
https://github.com/clap-rs/clap/blob/9fc8ec965e42db926ef6aa894810c23600dac49d/src/output/help.rs#L821

Pull request description:
Use a template to produce the default help message
This makes some changes to the template system:

- Template tags for optional items (like `{author}`) now expand to nothing when the value is unset instead of a default string (like `"unknown author"`).
- More template tags now emit line-wrapped output to match the behavior of `write_default_help`.
- Items with long variants now expand to the appropriate thing for `-h` vs `--help`.
- The now-obsolete `{long-about}` tag has been removed.
- A few new tags have been added.

These are externally-visible changes, but if this makes it into 3.0 that's probably reasonable?

Note that line-wrapping can have some odd edge cases since it does not account for preceding/trailing characters on the same line as the tag. This is already the case in `master`, but will affect some additional tags with this changeset. See #2065 for details.

An alternative approach would be to make the template language itself fancier, adding explicit mechanisms to control wrapping and/or conditional output. But since the template system is "by design very simple" this could be undesirable.

Closes #2002.

Patch:
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
index e2b17588..e067b7a8 100644
--- a/.github/ISSUE_TEMPLATE/bug_report.md
+++ b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -10,7 +10,7 @@ assignees: ''
 ### Make sure you completed the following tasks
 
 - [ ] Searched the [discussions](https://github.com/clap-rs/clap/discussions)
-- [ ] Searched the closed issues
+- [ ] Searched the closes issues
 
 ### Code
 
diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs
index 68467201..4d770260 100644
--- a/src/build/app/mod.rs
+++ b/src/build/app/mod.rs
@@ -724,26 +724,29 @@ impl<'help> App<'help> {
     /// **NOTE:** The template system is by design very simple. Therefore, the
     /// tags have to be written in the lowercase and without spacing.
     ///
-    /// Tags arg given inside curly brackets.
+    /// Tags are given inside curly brackets.
     ///
     /// Valid tags are:
     ///
-    ///   * `{bin}`         - Binary name.
-    ///   * `{version}`     - Version number.
-    ///   * `{author}`      - Author information.
-    ///   * `{about}`       - General description (from [`App::about`])
-    ///   * `{usage}`       - Automatically generated or given usage string.
-    ///   * `{all-args}`    - Help for all arguments (options, flags, positionals arguments,
-    ///                       and subcommands) including titles.
-    ///   * `{unified}`     - Unified help for options and flags. Note, you must *also* set
-    ///                       [`AppSettings::UnifiedHelpMessage`] to fully merge both options and
-    ///                       flags, otherwise the ordering is "best effort"
-    ///   * `{flags}`       - Help for flags.
-    ///   * `{options}`     - Help for options.
-    ///   * `{positionals}` - Help for positionals arguments.
-    ///   * `{subcommands}` - Help for subcommands.
-    ///   * `{after-help}`  - Help from [`App::after_help`]
-    ///   * `{before-help}`  - Help from [`App::before_help`]
+    ///   * `{bin}`                 - Binary name.
+    ///   * `{version}`             - Version number.
+    ///   * `{author}`              - Author information.
+    ///   * `{author-with-newline}` - Author followed by `\n`.
+    ///   * `{about}`               - General description (from [`App::about`] or
+    ///                               [`App::long_about`]).
+    ///   * `{about-with-newline}`  - About followed by `\n`.
+    ///   * `{usage}`               - Automatically generated or given usage string.
+    ///   * `{all-args}`            - Help for all arguments (options, flags, positional
+    ///                               arguments, and subcommands) including titles.
+    ///   * `{unified}`             - Unified help for options and flags. Note, you must *also*
+    ///                               set [`AppSettings::UnifiedHelpMessage`] to fully merge both
+    ///                               options and flags, otherwise the ordering is "best effort".
+    ///   * `{flags}`               - Help for flags.
+    ///   * `{options}`             - Help for options.
+    ///   * `{positionals}`         - Help for positional arguments.
+    ///   * `{subcommands}`         - Help for subcommands.
+    ///   * `{after-help}`          - Help from [`App::after_help`] or [`App::after_long_help`].
+    ///   * `{before-help}`         - Help from [`App::before_help`] or [`App::before_long_help`].
     ///
     /// # Examples
     ///
@@ -755,8 +758,11 @@ impl<'help> App<'help> {
     /// # ;
     /// ```
     /// [`App::about`]: ./struct.App.html#method.about
+    /// [`App::long_about`]: ./struct.App.html#method.long_about
     /// [`App::after_help`]: ./struct.App.html#method.after_help
+    /// [`App::after_long_help`]: ./struct.App.html#method.after_long_help
     /// [`App::before_help`]: ./struct.App.html#method.before_help
+    /// [`App::before_long_help`]: ./struct.App.html#method.before_long_help
     /// [`AppSettings::UnifiedHelpMessage`]: ./enum.AppSettings.html#variant.UnifiedHelpMessage
     pub fn help_template<S: Into<&'help str>>(mut self, s: S) -> Self {
         self.template = Some(s.into());
diff --git a/src/output/help.rs b/src/output/help.rs
index c782bfbf..c3d624c4 100644
--- a/src/output/help.rs
+++ b/src/output/help.rs
@@ -74,6 +74,20 @@ pub(crate) struct Help<'help, 'app, 'parser, 'writer> {
 
 // Public Functions
 impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
+    const DEFAULT_TEMPLATE: &'static str = "\
+        {before-help}{bin} {version}\n\
+        {author-with-newline}{about-with-newline}\n\
+        USAGE:\n    {usage}\n\
+        \n\
+        {all-args}{after-help}\
+    ";
+
+    const DEFAULT_NO_ARGS_TEMPLATE: &'static str = "\
+        {before-help}{bin} {version}\n\
+        {author-with-newline}{about-with-newline}\n\
+        USAGE:\n    {usage}{after-help}\
+    ";
+
     /// Create a new `Help` instance.
     pub(crate) fn new(
         w: HelpWriter<'writer>,
@@ -116,7 +130,16 @@ 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 {
-            self.write_default_help()?;
+            let flags = self.parser.has_flags();
+            let pos = self.parser.has_positionals();
+            let opts = self.parser.has_opts();
+            let subcmds = self.parser.has_subcommands();
+
+            if flags || opts || pos || subcmds {
+                self.write_templated_help(Self::DEFAULT_TEMPLATE)?;
+            } else {
+                self.write_templated_help(Self::DEFAULT_NO_ARGS_TEMPLATE)?;
+            }
         }
 
         self.none("\n")?;
@@ -811,13 +834,6 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
         Ok(())
     }
 
-    /// Writes version of a Parser Object to the wrapped stream.
-    fn write_version(&mut self) -> io::Result<()> {
-        debug!("Help::write_version");
-        self.none(self.parser.app.version.unwrap_or(""))?;
-        Ok(())
-    }
-
     /// Writes binary name of a Parser Object to the wrapped stream.
     fn write_bin_name(&mut self) -> io::Result<()> {
         debug!("Help::write_bin_name");
@@ -839,98 +855,6 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
         }
         Ok(())
     }
-
-    /// Writes default help for a Parser Object to the wrapped stream.
-    pub(crate) fn write_default_help(&mut self) -> ClapResult<()> {
-        debug!("Help::write_default_help");
-        if self.use_long {
-            if let Some(h) = self
-                .parser
-                .app
-                .before_long_help
-                .or_else(|| self.parser.app.before_help)
-            {
-                self.write_before_after_help(h)?;
-                self.none("\n\n")?;
-            }
-        } else if let Some(h) = self
-            .parser
-            .app
-            .before_help
-            .or_else(|| self.parser.app.before_long_help)
-        {
-            self.write_before_after_help(h)?;
-            self.none("\n\n")?;
-        }
-
-        macro_rules! write_thing {
-            ($thing:expr) => {{
-                self.none(&wrap_help(&$thing, self.term_w))?;
-                self.none("\n")?
-            }};
-        }
-
-        // Print the version
-        self.write_bin_name()?;
-        self.none(" ")?;
-        self.write_version()?;
-        self.none("\n")?;
-
-        if let Some(author) = self.parser.app.author {
-            write_thing!(author);
-        }
-
-        if self.use_long && self.parser.app.long_about.is_some() {
-            debug!("Help::write_default_help: writing long about");
-            write_thing!(self.parser.app.long_about.unwrap());
-        } else if self.parser.app.about.is_some() {
-            debug!("Help::write_default_help: writing about");
-            write_thing!(self.parser.app.about.unwrap());
-        }
-
-        self.none("\n")?;
-        self.warning("USAGE:")?;
-        self.none(&format!(
-            "\n{}{}\n\n",
-            TAB,
-            Usage::new(self.parser).create_usage_no_title(&[])
-        ))?;
-
-        let flags = self.parser.has_flags();
-        let pos = self.parser.has_positionals();
-        let opts = self.parser.has_opts();
-        let subcmds = self.parser.has_subcommands();
-
-        if flags || opts || pos || subcmds {
-            self.write_all_args()?;
-        }
-
-        if self.use_long {
-            if let Some(h) = self
-                .parser
-                .app
-                .after_long_help
-                .or_else(|| self.parser.app.after_help)
-            {
-                if flags || opts || pos || subcmds {
-                    self.none("\n\n")?;
-                }
-                self.write_before_after_help(h)?;
-            }
-        } else if let Some(h) = self
-            .parser
-            .app
-            .after_help
-            .or_else(|| self.parser.app.after_long_help)
-        {
-            if flags || opts || pos || subcmds {
-                self.none("\n\n")?;
-            }
-            self.write_before_after_help(h)?;
-        }
-
-        self.writer.flush().map_err(Error::from)
-    }
 }
 
 /// Possible results for a copying function that stops when a given
@@ -1034,28 +958,13 @@ fn copy_and_capture<R: Read, W: Write>(
 impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
     /// Write help to stream for the parser in the format defined by the template.
     ///
-    /// Tags arg given inside curly brackets:
-    /// Valid tags are:
-    ///     * `{bin}`         - Binary name.
-    ///     * `{version}`     - Version number.
-    ///     * `{author}`      - Author information.
-    ///     * `{usage}`       - Automatically generated or given usage string.
-    ///     * `{all-args}`    - Help for all arguments (options, flags, positionals arguments,
-    ///                         and subcommands) including titles.
-    ///     * `{unified}`     - Unified help for options and flags.
-    ///     * `{flags}`       - Help for flags.
-    ///     * `{options}`     - Help for options.
-    ///     * `{positionals}` - Help for positionals arguments.
-    ///     * `{subcommands}` - Help for subcommands.
-    ///     * `{after-help}`  - Info to be displayed after the help message.
-    ///     * `{before-help}` - Info to be displayed before the help message.
+    /// For details about the template language see [`App::help_template`].
     ///
-    /// The template system is, on purpose, very simple. Therefore, the tags have to be written
-    /// in the lowercase and without spacing.
+    /// [`App::help_template`]: ./struct.App.html#method.help_template
     fn write_templated_help(&mut self, template: &str) -> ClapResult<()> {
         debug!("Help::write_templated_help");
         let mut tmplr = Cursor::new(&template);
-        let mut tag_buf = Cursor::new(vec![0u8; 15]);
+        let mut tag_buf = Cursor::new(vec![0u8; 20]);
 
         // The strategy is to copy the template from the reader to wrapped stream
         // until a tag is found. Depending on its value, the appropriate content is copied
@@ -1083,16 +992,41 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
                     self.write_bin_name()?;
                 }
                 b"version" => {
-                    self.none(self.parser.app.version.unwrap_or("unknown version"))?;
+                    if let Some(output) = self.parser.app.version {
+                        self.none(output)?;
+                    }
                 }
                 b"author" => {
-                    self.none(self.parser.app.author.unwrap_or("unknown author"))?;
+                    if let Some(output) = self.parser.app.author {
+                        self.none(&wrap_help(output, self.term_w))?;
+                    }
+                }
+                b"author-with-newline" => {
+                    if let Some(output) = self.parser.app.author {
+                        self.none(&wrap_help(output, self.term_w))?;
+                        self.none("\n")?;
+                    }
                 }
                 b"about" => {
-                    self.none(self.parser.app.about.unwrap_or("unknown about"))?;
+                    let about = if self.use_long {
+                        self.parser.app.long_about.or(self.parser.app.about)
+                    } else {
+                        self.parser.app.about
+                    };
+                    if let Some(output) = about {
+                        self.none(&wrap_help(output, self.term_w))?;
+                    }
                 }
-                b"long-about" => {
-                    self.none(self.parser.app.long_about.unwrap_or("unknown about"))?;
+                b"about-with-newline" => {
+                    let about = if self.use_long {
+                        self.parser.app.long_about.or(self.parser.app.about)
+                    } else {
+                        self.parser.app.about
+                    };
+                    if let Some(output) = about {
+                        self.none(&wrap_help(output, self.term_w))?;
+                        self.none("\n")?;
+                    }
                 }
                 b"usage" => {
                     self.none(&Usage::new(self.parser).create_usage_no_title(&[]))?;
@@ -1124,10 +1058,32 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
                     self.write_subcommands(self.parser.app)?;
                 }
                 b"after-help" => {
-                    self.none(self.parser.app.after_help.unwrap_or("unknown after-help"))?;
+                    let after_help = if self.use_long {
+                        self.parser
+                            .app
+                            .after_long_help
+                            .or(self.parser.app.after_help)
+                    } else {
+                        self.parser.app.after_help
+                    };
+                    if let Some(output) = after_help {
+                        self.none("\n\n")?;
+                        self.write_before_after_help(output)?;
+                    }
                 }
                 b"before-help" => {
-                    self.none(self.parser.app.before_help.unwrap_or("unknown before-help"))?;
+                    let before_help = if self.use_long {
+                        self.parser
+                            .app
+                            .before_long_help
+                            .or(self.parser.app.before_help)
+                    } else {
+                        self.parser.app.before_help
+                    };
+                    if let Some(output) = before_help {
+                        self.write_before_after_help(output)?;
+                        self.none("\n\n")?;
+                    }
                 }
                 // Unknown tag, write it back.
                 r => {


Test patch:
diff --git a/tests/help.rs b/tests/help.rs
index 68d9a1bc..2c1b9d94 100644
--- a/tests/help.rs
+++ b/tests/help.rs
@@ -1796,3 +1796,28 @@ and on, so I'll stop now.",
     ));
     assert!(utils::compare_output(app, "prog --help", ISSUE_1642, false));
 }
+
+const AFTER_HELP_NO_ARGS: &str = "myapp 1.0
+
+USAGE:
+    myapp
+
+This is after help.
+";
+
+#[test]
+fn after_help_no_args() {
+    let mut app = App::new("myapp")
+        .version("1.0")
+        .setting(AppSettings::DisableHelpFlags)
+        .setting(AppSettings::DisableVersion)
+        .after_help("This is after help.");
+
+    let help = {
+        let mut output = Vec::new();
+        app.write_help(&mut output).unwrap();
+        String::from_utf8(output).unwrap()
+    };
+
+    assert_eq!(help, AFTER_HELP_NO_ARGS);
+}