From 7f05c15a5e7ca1b92b2f77b5cc1736a55c6adba4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 14 Oct 2021 12:16:20 -0500 Subject: [PATCH] fix: Give Arg::help_heading priority over App::help_heading PR #1211 originally added `help_heading` with the current priority (`App::help_heading` wins). In #1958, the author had proposed to change this > Note that I made the existing arg's heading a priority over the one in App::help_heading This was reverted on reviewer request because > Thanks for the priority explanation. Yes, please make the app's help > heading a priority. I can see how it would be useful when people might > want to reuse args. Re-using args is an important use case but this makes life difficult for anyone using `help_heading` with `clap_derive` because the user can only call `App::help_heading` once per struct. Derive users can't get per-field granularity with `App::help_heading` like the builder API. As a bonus, I think this will be the least surpising to users. Users generally expect the more generic specification (App) to be overridden by the more specific specification (Arg). Honestly, I had thought this PR is how `help_heading` worked until I dug into the code with #2872. In the argument re-use cases, a workaround is for the reusable arguments to **not** set their help_heading and require the caller to set it as needed. Fixes #2873 --- src/build/app/mod.rs | 13 +++++-------- src/build/arg/mod.rs | 6 +++--- src/output/help.rs | 4 ++-- tests/help.rs | 32 +++++++++++++++++++------------- 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 92057b77..72286a3e 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1086,23 +1086,20 @@ impl<'help> App<'help> { /// [argument]: Arg pub fn arg>>(mut self, a: A) -> Self { let mut arg = a.into(); - if let Some(help_heading) = self.current_help_heading { - arg = arg.help_heading(Some(help_heading)); - } + arg.help_heading.get_or_insert(self.current_help_heading); self.args.push(arg); self } - /// Set a custom section heading for future args. Every call to [`App::arg`] - /// (and its related methods) will use this header (instead of the default - /// header for the specified argument type) until a subsequent call to - /// [`App::help_heading`]. + /// Set the default section heading for future args. + /// + /// This will be used for any arg that hasn't had [`Arg::help_heading`] called. /// /// This is useful if the default `OPTIONS` or `ARGS` headings are /// not specific enough for one's use case. /// /// [`App::arg`]: App::arg() - /// [`App::help_heading`]: App::help_heading() + /// [`Arg::help_heading`]: crate::Arg::help_heading() #[inline] pub fn help_heading(mut self, heading: O) -> Self where diff --git a/src/build/arg/mod.rs b/src/build/arg/mod.rs index a3becc65..ab9fc3fb 100644 --- a/src/build/arg/mod.rs +++ b/src/build/arg/mod.rs @@ -118,7 +118,7 @@ pub struct Arg<'help> { pub(crate) env: Option<(&'help OsStr, Option)>, pub(crate) terminator: Option<&'help str>, pub(crate) index: Option, - pub(crate) help_heading: Option<&'help str>, + pub(crate) help_heading: Option>, pub(crate) global: bool, pub(crate) exclusive: bool, pub(crate) value_hint: ValueHint, @@ -156,7 +156,7 @@ impl<'help> Arg<'help> { /// Get the help heading specified for this argument, if any #[inline] pub fn get_help_heading(&self) -> Option<&str> { - self.help_heading + self.help_heading.unwrap_or_default() } /// Get the short option name for this argument, if any @@ -4699,7 +4699,7 @@ impl<'help> Arg<'help> { where O: Into>, { - self.help_heading = heading.into(); + self.help_heading = Some(heading.into()); self } diff --git a/src/output/help.rs b/src/output/help.rs index 82d3df27..da22bae9 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -755,7 +755,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> { .app .args .args() - .filter_map(|arg| arg.help_heading) + .filter_map(|arg| arg.get_help_heading()) .collect::>(); let mut first = if !pos.is_empty() { @@ -783,7 +783,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> { .args .args() .filter(|a| { - if let Some(help_heading) = a.help_heading { + if let Some(help_heading) = a.get_help_heading() { return help_heading == heading; } false diff --git a/tests/help.rs b/tests/help.rs index 6b0561c1..7f098bb0 100644 --- a/tests/help.rs +++ b/tests/help.rs @@ -1788,14 +1788,17 @@ OPTIONS: -f, --fake : some help -h, --help Print help information -s, --speed How fast? [possible values: fast, slow] + --style