Merge pull request #4161 from epage/magic

refactor(help): Reduce magic numbers for indentation

CHANGELOG.md

@@ -74,6 +74,7 @@ Deprecated
 - Added `TypedValueParser::map` to make it easier to reuse existing value parsers
 - *(error)* `Error::apply` for changing the formatter for dropping binary size
 - *(help)* Show `PossibleValue::help` in long help (`--help`) (#3312)
+- *(help)* New `{tab}` variable for `Command::help_template`
 
 ### Fixes
 

examples/derive_ref/interop_tests.md

@@ -37,7 +37,7 @@ $ interop_augment_args --unknown
 ? failed
 error: Found argument '--unknown' which wasn't expected, or isn't valid in this context
 
-	If you tried to supply `--unknown` as a value rather than a flag, use `-- --unknown`
+    If you tried to supply `--unknown` as a value rather than a flag, use `-- --unknown`
 
 Usage:
     interop_augment_args[EXE] [OPTIONS]
@@ -75,7 +75,7 @@ $ interop_augment_subcommands derived --unknown
 ? failed
 error: Found argument '--unknown' which wasn't expected, or isn't valid in this context
 
-	If you tried to supply `--unknown` as a value rather than a flag, use `-- --unknown`
+    If you tried to supply `--unknown` as a value rather than a flag, use `-- --unknown`
 
 Usage:
     interop_augment_subcommands[EXE] derived [OPTIONS]
@@ -150,7 +150,7 @@ $ interop_hand_subcommand add --unknown
 ? failed
 error: Found argument '--unknown' which wasn't expected, or isn't valid in this context
 
-	If you tried to supply `--unknown` as a value rather than a flag, use `-- --unknown`
+    If you tried to supply `--unknown` as a value rather than a flag, use `-- --unknown`
 
 Usage:
     interop_hand_subcommand[EXE] add [NAME]...
@@ -251,7 +251,7 @@ $ interop_flatten_hand_args --unknown
 ? failed
 error: Found argument '--unknown' which wasn't expected, or isn't valid in this context
 
-	If you tried to supply `--unknown` as a value rather than a flag, use `-- --unknown`
+    If you tried to supply `--unknown` as a value rather than a flag, use `-- --unknown`
 
 Usage:
     interop_flatten_hand_args[EXE] [OPTIONS]

examples/tutorial_builder/04_01_enum.md

@@ -21,7 +21,7 @@ Tortoise
 $ 04_01_enum medium
 ? failed
 error: "medium" isn't a valid value for '<MODE>'
-	[possible values: fast, slow]
+    [possible values: fast, slow]
 
 For more information try --help
 

examples/tutorial_builder/04_01_possible.md

@@ -21,7 +21,7 @@ Tortoise
 $ 04_01_possible medium
 ? failed
 error: "medium" isn't a valid value for '<MODE>'
-	[possible values: fast, slow]
+    [possible values: fast, slow]
 
 For more information try --help
 

examples/tutorial_derive/04_01_enum.md

@@ -21,7 +21,7 @@ Tortoise
 $ 04_01_enum_derive medium
 ? failed
 error: "medium" isn't a valid value for '<MODE>'
-	[possible values: fast, slow]
+    [possible values: fast, slow]
 
 For more information try --help
 

src/builder/command.rs

@@ -1706,6 +1706,7 @@ impl Command {
     ///   * `{options}`             - Help for options.
     ///   * `{positionals}`         - Help for positional arguments.
     ///   * `{subcommands}`         - Help for subcommands.
+    ///   * `{tag}`                 - Standard tab sized used within clap
     ///   * `{after-help}`          - Help from [`Command::after_help`] or [`Command::after_long_help`].
     ///   * `{before-help}`         - Help from [`Command::before_help`] or [`Command::before_long_help`].
     ///

src/error/format.rs

@@ -6,6 +6,7 @@ use crate::builder::StyledStr;
 use crate::error::ContextKind;
 use crate::error::ContextValue;
 use crate::error::ErrorKind;
+use crate::output::TAB;
 
 /// Defines how to format an error for displaying to the user
 pub trait ErrorFormatter: Sized {
@@ -110,7 +111,8 @@ fn write_dynamic_context(error: &crate::Error, styled: &mut StyledStr) -> bool {
                     ContextValue::Strings(values) => {
                         styled.none(":");
                         for v in values {
-                            styled.none("\n    ");
+                            styled.none("\n");
+                            styled.none(TAB);
                             styled.warning(&**v);
                         }
                     }
@@ -161,7 +163,9 @@ fn write_dynamic_context(error: &crate::Error, styled: &mut StyledStr) -> bool {
                 let possible_values = error.get(ContextKind::ValidValue);
                 if let Some(ContextValue::Strings(possible_values)) = possible_values {
                     if !possible_values.is_empty() {
-                        styled.none("\n\t[possible values: ");
+                        styled.none("\n");
+                        styled.none(TAB);
+                        styled.none("[possible values: ");
                         if let Some((last, elements)) = possible_values.split_last() {
                             for v in elements {
                                 styled.good(escape(v));
@@ -175,7 +179,9 @@ fn write_dynamic_context(error: &crate::Error, styled: &mut StyledStr) -> bool {
 
                 let suggestion = error.get(ContextKind::SuggestedValue);
                 if let Some(ContextValue::String(suggestion)) = suggestion {
-                    styled.none("\n\n\tDid you mean ");
+                    styled.none("\n\n");
+                    styled.none(TAB);
+                    styled.none("Did you mean ");
                     styled.good(quote(suggestion));
                     styled.none("?");
                 }
@@ -193,7 +199,9 @@ fn write_dynamic_context(error: &crate::Error, styled: &mut StyledStr) -> bool {
 
                 let valid_sub = error.get(ContextKind::SuggestedSubcommand);
                 if let Some(ContextValue::String(valid_sub)) = valid_sub {
-                    styled.none("\n\n\tDid you mean ");
+                    styled.none("\n\n");
+                    styled.none(TAB);
+                    styled.none("Did you mean ");
                     styled.good(valid_sub);
                     styled.none("?");
                 }
@@ -216,7 +224,8 @@ fn write_dynamic_context(error: &crate::Error, styled: &mut StyledStr) -> bool {
             if let Some(ContextValue::Strings(invalid_arg)) = invalid_arg {
                 styled.none("The following required arguments were not provided:");
                 for v in invalid_arg {
-                    styled.none("\n    ");
+                    styled.none("\n");
+                    styled.none(TAB);
                     styled.good(&**v);
                 }
                 true
@@ -337,15 +346,19 @@ fn write_dynamic_context(error: &crate::Error, styled: &mut StyledStr) -> bool {
                         Some(ContextValue::String(valid_sub)),
                         Some(ContextValue::String(valid_arg)),
                     ) => {
-                        styled.none("\n\n\tDid you mean ");
+                        styled.none("\n\n");
+                        styled.none(TAB);
+                        styled.none("Did you mean ");
                         styled.none("to put '");
                         styled.good(valid_arg);
                         styled.none("' after the subcommand '");
                         styled.good(valid_sub);
                         styled.none("'?");
                     }
                     (None, Some(ContextValue::String(valid_arg))) => {
-                        styled.none("\n\n\tDid you mean '");
+                        styled.none("\n\n");
+                        styled.none(TAB);
+                        styled.none("Did you mean '");
                         styled.good(valid_arg);
                         styled.none("'?");
                     }
@@ -355,16 +368,20 @@ fn write_dynamic_context(error: &crate::Error, styled: &mut StyledStr) -> bool {
                 let invalid_arg = error.get(ContextKind::InvalidArg);
                 if let Some(ContextValue::String(invalid_arg)) = invalid_arg {
                     if invalid_arg.starts_with('-') {
+                        styled.none("\n\n");
+                        styled.none(TAB);
                         styled.none(format!(
-                                "\n\n\tIf you tried to supply `{}` as a value rather than a flag, use `-- {}`",
+                                "If you tried to supply `{}` as a value rather than a flag, use `-- {}`",
                                 invalid_arg, invalid_arg
                             ));
                     }
 
                     let trailing_arg = error.get(ContextKind::TrailingArg);
                     if trailing_arg == Some(&ContextValue::Bool(true)) {
+                        styled.none("\n\n");
+                        styled.none(TAB);
                         styled.none(format!(
-                            "\n\n\tIf you tried to supply `{}` as a subcommand, remove the '--' before it.",
+                            "If you tried to supply `{}` as a subcommand, remove the '--' before it.",
                             invalid_arg
                         ));
                     }

src/output/help.rs

@@ -12,6 +12,8 @@ use crate::builder::{Arg, Command};
 use crate::output::display_width;
 use crate::output::wrap;
 use crate::output::Usage;
+use crate::output::TAB;
+use crate::output::TAB_WIDTH;
 use crate::util::FlatSet;
 
 /// `clap` Help Writer.
@@ -29,15 +31,17 @@ pub(crate) struct Help<'cmd, 'writer> {
 // Public Functions
 impl<'cmd, 'writer> Help<'cmd, 'writer> {
     const DEFAULT_TEMPLATE: &'static str = "\
-        {before-help}{about-with-newline}\n\
-        {usage-heading}\n    {usage}\n\
-        \n\
-        {all-args}{after-help}\
+{before-help}{about-with-newline}
+{usage-heading}
+{tab}{usage}
+
+{all-args}{after-help}\
     ";
 
     const DEFAULT_NO_ARGS_TEMPLATE: &'static str = "\
-        {before-help}{about-with-newline}\n\
-        {usage-heading}\n    {usage}{after-help}\
+{before-help}{about-with-newline}
+{usage-heading}
+{tab}{usage}{after-help}\
     ";
 
     /// Create a new `Help` instance.
@@ -184,6 +188,9 @@ impl<'cmd, 'writer> Help<'cmd, 'writer> {
                     "subcommands" => {
                         self.write_subcommands(self.cmd);
                     }
+                    "tab" => {
+                        self.none(TAB);
+                    }
                     "after-help" => {
                         self.write_after_help();
                     }
@@ -503,10 +510,10 @@ impl<'cmd, 'writer> Help<'cmd, 'writer> {
             // had a long and short, or just short
             let padding = if arg.long.is_some() {
                 // Only account 4 after the val
-                4
+                TAB_WIDTH
             } else {
                 // Only account for ', --' + 4 after the val
-                8
+                TAB_WIDTH + 4
             };
             let spcs = longest + padding - self_len;
             debug!(
@@ -517,7 +524,7 @@ impl<'cmd, 'writer> Help<'cmd, 'writer> {
             self.spaces(spcs);
         } else {
             let self_len = display_width(&arg.to_string());
-            let padding = 4;
+            let padding = TAB_WIDTH;
             let spcs = longest + padding - self_len;
             debug!(
                 "Help::align_to_about: positional=true arg_len={}, spaces={}",
@@ -555,9 +562,9 @@ impl<'cmd, 'writer> Help<'cmd, 'writer> {
         let trailing_indent = self.get_spaces(trailing_indent);
 
         let spaces = if next_line_help {
-            12 // "tab" * 3
+            TAB_WIDTH * 3
         } else {
-            longest + 12
+            longest + TAB_WIDTH * 3
         };
         let mut help = about.clone();
         help.replace_newline();
@@ -922,7 +929,7 @@ impl<'cmd, 'writer> Help<'cmd, 'writer> {
         self.literal(sc_str);
         if !next_line_help {
             let width = display_width(sc_str);
-            self.spaces(width.max(longest + 4) - width);
+            self.spaces(width.max(longest + TAB_WIDTH) - width);
         }
     }
 }
@@ -964,9 +971,6 @@ pub(crate) fn dimensions() -> Option<(usize, usize)> {
     terminal_size::terminal_size().map(|(w, h)| (w.0.into(), h.0.into()))
 }
 
-const TAB: &str = "    ";
-const TAB_WIDTH: usize = 4;
-
 fn should_show_arg(use_long: bool, arg: &Arg) -> bool {
     debug!(
         "should_show_arg: use_long={:?}, arg={}",

src/output/mod.rs

@@ -8,3 +8,6 @@ pub(crate) use self::help::Help;
 pub(crate) use self::textwrap::core::display_width;
 pub(crate) use self::textwrap::wrap;
 pub(crate) use self::usage::Usage;
+
+pub(crate) const TAB: &str = "    ";
+pub(crate) const TAB_WIDTH: usize = TAB.len();

src/output/usage.rs

@@ -1,6 +1,7 @@
 // Internal
 use crate::builder::StyledStr;
 use crate::builder::{ArgPredicate, Command};
+use crate::output::TAB;
 use crate::parser::ArgMatcher;
 use crate::util::ChildGraph;
 use crate::util::FlatSet;
@@ -32,7 +33,8 @@ impl<'cmd> Usage<'cmd> {
         debug!("Usage::create_usage_with_title");
         let mut styled = StyledStr::new();
         styled.header("Usage:");
-        styled.none("\n    ");
+        styled.none("\n");
+        styled.none(TAB);
         styled.extend(self.create_usage_no_title(used).into_iter());
         styled
     }

tests/builder/flags.rs

@@ -4,7 +4,7 @@ use clap::{arg, Arg, ArgAction, Command};
 const USE_FLAG_AS_ARGUMENT: &str =
     "error: Found argument '--another-flag' which wasn't expected, or isn't valid in this context
 
-\tIf you tried to supply `--another-flag` as a value rather than a flag, use `-- --another-flag`
+    If you tried to supply `--another-flag` as a value rather than a flag, use `-- --another-flag`
 
 Usage:
     mycat [OPTIONS] [filename]
@@ -181,7 +181,7 @@ fn issue_2308_multiple_dashes() {
     static MULTIPLE_DASHES: &str =
         "error: Found argument '-----' which wasn't expected, or isn't valid in this context
 
-	If you tried to supply `-----` as a value rather than a flag, use `-- -----`
+    If you tried to supply `-----` as a value rather than a flag, use `-- -----`
 
 Usage:
     test <arg>

tests/builder/opts.rs

@@ -6,9 +6,9 @@ use clap::{arg, error::ErrorKind, Arg, ArgAction, ArgMatches, Command};
 static DYM: &str =
     "error: Found argument '--optio' which wasn't expected, or isn't valid in this context
 
-\tDid you mean '--option'?
+    Did you mean '--option'?
 
-\tIf you tried to supply `--optio` as a value rather than a flag, use `-- --optio`
+    If you tried to supply `--optio` as a value rather than a flag, use `-- --optio`
 
 Usage:
     clap-test --option <opt>... [positional] [positional2] [positional3]...
@@ -20,9 +20,9 @@ For more information try --help
 static DYM_ISSUE_1073: &str =
     "error: Found argument '--files-without-matches' which wasn't expected, or isn't valid in this context
 
-\tDid you mean '--files-without-match'?
+    Did you mean '--files-without-match'?
 
-\tIf you tried to supply `--files-without-matches` as a value rather than a flag, use `-- --files-without-matches`
+    If you tried to supply `--files-without-matches` as a value rather than a flag, use `-- --files-without-matches`
 
 Usage:
     ripgrep-616 --files-without-match

tests/builder/possible_values.rs

@@ -4,32 +4,32 @@ use clap::{builder::PossibleValue, error::ErrorKind, Arg, ArgAction, Command};
 
 #[cfg(feature = "suggestions")]
 static PV_ERROR: &str = "error: \"slo\" isn't a valid value for '-O <option>'
-\t[possible values: slow, fast, \"ludicrous speed\"]
+    [possible values: slow, fast, \"ludicrous speed\"]
 
-\tDid you mean \"slow\"?
+    Did you mean \"slow\"?
 
 For more information try --help
 ";
 
 #[cfg(not(feature = "suggestions"))]
 static PV_ERROR: &str = "error: \"slo\" isn't a valid value for '-O <option>'
-\t[possible values: slow, fast, \"ludicrous speed\"]
+    [possible values: slow, fast, \"ludicrous speed\"]
 
 For more information try --help
 ";
 
 #[cfg(feature = "suggestions")]
 static PV_ERROR_ESCAPED: &str = "error: \"ludicrous\" isn't a valid value for '-O <option>'
-\t[possible values: slow, fast, \"ludicrous speed\"]
+    [possible values: slow, fast, \"ludicrous speed\"]
 
-\tDid you mean \"ludicrous speed\"?
+    Did you mean \"ludicrous speed\"?
 
 For more information try --help
 ";
 
 #[cfg(not(feature = "suggestions"))]
 static PV_ERROR_ESCAPED: &str = "error: \"ludicrous\" isn't a valid value for '-O <option>'
-\t[possible values: slow, fast, \"ludicrous speed\"]
+    [possible values: slow, fast, \"ludicrous speed\"]
 
 For more information try --help
 ";
@@ -295,7 +295,7 @@ fn missing_possible_value_error() {
 
 static MISSING_PV_ERROR: &str =
     "error: The argument '-O <option>' requires a value but none was supplied
-\t[possible values: slow, fast, \"ludicrous speed\"]
+    [possible values: slow, fast, \"ludicrous speed\"]
 
 For more information try --help
 ";