From a87320ae88ec6cfd5c1fbd02416833f68bd22ddd Mon Sep 17 00:00:00 2001 From: Matt Kantor Date: Tue, 11 Aug 2020 20:05:32 -0700 Subject: [PATCH 1/7] 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"). - Many template tags now emit line-wrapped output to match 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. Closes #2002. --- src/build/app/mod.rs | 46 +++++----- src/output/help.rs | 211 ++++++++++++++++++------------------------- src/parse/parser.rs | 4 - 3 files changed, 107 insertions(+), 154 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 68467201..21e35282 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -724,26 +724,30 @@ 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}` - Description of the program. + /// * `{about-with-newline}` - Description 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}` - Info to be displayed after the help message. + /// * `{after-help-padded}` - After help message with additional spacing. + /// * `{before-help}` - Info to be displayed before the help message. + /// * `{before-help-padded}` - Before help message with additional spacing. /// /// # Examples /// @@ -754,9 +758,6 @@ impl<'help> App<'help> { /// .help_template("{bin} ({version}) - {usage}") /// # ; /// ``` - /// [`App::about`]: ./struct.App.html#method.about - /// [`App::after_help`]: ./struct.App.html#method.after_help - /// [`App::before_help`]: ./struct.App.html#method.before_help /// [`AppSettings::UnifiedHelpMessage`]: ./enum.AppSettings.html#variant.UnifiedHelpMessage pub fn help_template>(mut self, s: S) -> Self { self.template = Some(s.into()); @@ -2448,11 +2449,6 @@ impl<'help> App<'help> { !self.args.is_empty() } - #[inline] - pub(crate) fn has_opts(&self) -> bool { - self.get_opts_no_heading().count() > 0 - } - #[inline] pub(crate) fn has_flags(&self) -> bool { self.get_flags_no_heading().count() > 0 diff --git a/src/output/help.rs b/src/output/help.rs index c782bfbf..c72fe0c4 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -74,6 +74,14 @@ 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-padded}{bin} {version}\n\ + {author-with-newline}{about-with-newline}\n\ + USAGE:\n {usage}\n\ + \n\ + {all-args}{after-help-padded}\ + "; + /// Create a new `Help` instance. pub(crate) fn new( w: HelpWriter<'writer>, @@ -116,7 +124,7 @@ 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()?; + self.write_templated_help(Self::DEFAULT_TEMPLATE)?; } self.none("\n")?; @@ -811,13 +819,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 +840,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 +943,13 @@ fn copy_and_capture( 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 +977,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(output)?; + } + } + 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(output)?; + } } - 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 +1043,52 @@ 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.write_before_after_help(output)?; + } + } + b"after-help-padded" => { + 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 + } else { + self.parser.app.before_help + }; + if let Some(output) = before_help { + self.write_before_after_help(output)?; + } + } + b"before-help-padded" => { + let before_help = if self.use_long { + self.parser.app.before_long_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 => { diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 16cc7056..34c43f4f 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -1790,10 +1790,6 @@ impl<'help, 'app> Parser<'help, 'app> { 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() } From 059503e54d0101f7a7fda7a5001b1d3c52a3f08e Mon Sep 17 00:00:00 2001 From: Matt Kantor Date: Fri, 14 Aug 2020 09:09:40 -0700 Subject: [PATCH 2/7] Make {before-help} and {after-help} template tags include padding. Previously there were separate tags for this, {before-help-padded} and {after-help-padded}. Those have been removed and the default ones given their behavior. --- src/build/app/mod.rs | 2 -- src/output/help.rs | 27 ++------------------------- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 21e35282..f4345a86 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -745,9 +745,7 @@ impl<'help> App<'help> { /// * `{positionals}` - Help for positional arguments. /// * `{subcommands}` - Help for subcommands. /// * `{after-help}` - Info to be displayed after the help message. - /// * `{after-help-padded}` - After help message with additional spacing. /// * `{before-help}` - Info to be displayed before the help message. - /// * `{before-help-padded}` - Before help message with additional spacing. /// /// # Examples /// diff --git a/src/output/help.rs b/src/output/help.rs index c72fe0c4..ccd43d8e 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -75,11 +75,11 @@ 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-padded}{bin} {version}\n\ + {before-help}{bin} {version}\n\ {author-with-newline}{about-with-newline}\n\ USAGE:\n {usage}\n\ \n\ - {all-args}{after-help-padded}\ + {all-args}{after-help}\ "; /// Create a new `Help` instance. @@ -1043,19 +1043,6 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> { self.write_subcommands(self.parser.app)?; } b"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.write_before_after_help(output)?; - } - } - b"after-help-padded" => { let after_help = if self.use_long { self.parser .app @@ -1070,16 +1057,6 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> { } } b"before-help" => { - let before_help = if self.use_long { - self.parser.app.before_long_help - } else { - self.parser.app.before_help - }; - if let Some(output) = before_help { - self.write_before_after_help(output)?; - } - } - b"before-help-padded" => { let before_help = if self.use_long { self.parser.app.before_long_help } else { From afcacb0626cfbfc32e5a27f7b14acd1538282f0b Mon Sep 17 00:00:00 2001 From: Matt Kantor Date: Fri, 14 Aug 2020 09:11:45 -0700 Subject: [PATCH 3/7] Apply wrapping to {author} and {about} template tags. Previously wrapping was only applied to the {*-with-newline} variants. --- src/output/help.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/output/help.rs b/src/output/help.rs index ccd43d8e..a7966f86 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -983,7 +983,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> { } b"author" => { if let Some(output) = self.parser.app.author { - self.none(output)?; + self.none(&wrap_help(output, self.term_w))?; } } b"author-with-newline" => { @@ -999,7 +999,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> { self.parser.app.about }; if let Some(output) = about { - self.none(output)?; + self.none(&wrap_help(output, self.term_w))?; } } b"about-with-newline" => { From 2c9180009981c030ee55bd9f39ba7cf581028c41 Mon Sep 17 00:00:00 2001 From: Matt Kantor Date: Fri, 14 Aug 2020 09:27:42 -0700 Subject: [PATCH 4/7] Restore details to doc comment for help_template. --- src/build/app/mod.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index f4345a86..f9923c3b 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -732,8 +732,9 @@ impl<'help> App<'help> { /// * `{version}` - Version number. /// * `{author}` - Author information. /// * `{author-with-newline}` - Author followed by `\n`. - /// * `{about}` - Description of the program. - /// * `{about-with-newline}` - Description 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. @@ -744,8 +745,8 @@ impl<'help> App<'help> { /// * `{options}` - Help for options. /// * `{positionals}` - Help for positional 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. + /// * `{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 /// @@ -756,6 +757,12 @@ impl<'help> App<'help> { /// .help_template("{bin} ({version}) - {usage}") /// # ; /// ``` + /// [`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>(mut self, s: S) -> Self { self.template = Some(s.into()); From 4545a47ff61b91d9a23ee2366b102af7aa2eecbc Mon Sep 17 00:00:00 2001 From: Matt Kantor Date: Fri, 14 Aug 2020 13:58:37 -0700 Subject: [PATCH 5/7] Fix: {before-help} should have fallback just like {after-help}. --- src/output/help.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/output/help.rs b/src/output/help.rs index a7966f86..097b3b4b 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -1058,7 +1058,10 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> { } b"before-help" => { let before_help = if self.use_long { - self.parser.app.before_long_help + self.parser + .app + .before_long_help + .or(self.parser.app.before_help) } else { self.parser.app.before_help }; From de6a5af1b2df7b396e67c8017a10a4422adf6007 Mon Sep 17 00:00:00 2001 From: Matt Kantor Date: Fri, 14 Aug 2020 13:11:12 -0700 Subject: [PATCH 6/7] Add a (failing) test for no args + after help. --- tests/help.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) 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); +} From 85d3daa8c1ad17f79bd4b1c67fdf1dc5d146d36e Mon Sep 17 00:00:00 2001 From: Matt Kantor Date: Fri, 14 Aug 2020 13:06:15 -0700 Subject: [PATCH 7/7] Use a different default template when there are no args. This eliminates extraneous empty lines when there are no user-defined args, the default args are disabled, and `after_help` is set. --- src/build/app/mod.rs | 5 +++++ src/output/help.rs | 17 ++++++++++++++++- src/parse/parser.rs | 4 ++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index f9923c3b..4d770260 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -2454,6 +2454,11 @@ impl<'help> App<'help> { !self.args.is_empty() } + #[inline] + pub(crate) fn has_opts(&self) -> bool { + self.get_opts_no_heading().count() > 0 + } + #[inline] pub(crate) fn has_flags(&self) -> bool { self.get_flags_no_heading().count() > 0 diff --git a/src/output/help.rs b/src/output/help.rs index 097b3b4b..c3d624c4 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -82,6 +82,12 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> { {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>, @@ -124,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_templated_help(Self::DEFAULT_TEMPLATE)?; + 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")?; diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 34c43f4f..16cc7056 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -1790,6 +1790,10 @@ impl<'help, 'app> Parser<'help, 'app> { 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() }