Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(help): Reduce magic numbers for indentation #4161

Merged
merged 3 commits into from Aug 31, 2022
Merged

refactor(help): Reduce magic numbers for indentation #4161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
803d87b
refactor(help): Reduce magic numbers
epage Aug 31, 2022
439c9e7
feat(help): Expose clap's indentation to help_template
epage Aug 31, 2022
671914b
fix(error): Make whitespace consistent with self/help
epage Aug 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions examples/derive_ref/interop_tests.md
View file
Open in desktop
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]...
Expand Down Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion examples/tutorial_builder/04_01_enum.md
View file
Open in desktop
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion examples/tutorial_builder/04_01_possible.md
View file
Open in desktop
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion examples/tutorial_derive/04_01_enum.md
View file
Open in desktop
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions src/builder/command.rs
View file
Open in desktop
Expand Up @@ -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`].
///
Expand Down
35 changes: 26 additions & 9 deletions src/error/format.rs
View file
Open in desktop
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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));
Expand All @@ -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("?");
}
Expand All @@ -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("?");
}
Expand All @@ -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
Expand Down Expand Up @@ -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("'?");
}
Expand All @@ -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
));
}
Expand Down
34 changes: 19 additions & 15 deletions src/output/help.rs
View file
Open in desktop
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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!(
Expand All @@ -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={}",
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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={}",
Expand Down
3 changes: 3 additions & 0 deletions src/output/mod.rs
View file
Open in desktop
Expand Up @@ -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();
4 changes: 3 additions & 1 deletion src/output/usage.rs
View file
Open in desktop
@@ -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;
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions tests/builder/flags.rs
View file
Open in desktop
Expand Up @@ -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]
Expand Down Expand Up @@ -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>
Expand Down
8 changes: 4 additions & 4 deletions tests/builder/opts.rs
View file
Open in desktop
Expand Up @@ -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]...
Expand All @@ -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
Expand Down
14 changes: 7 additions & 7 deletions tests/builder/possible_values.rs
View file
Open in desktop
Expand Up @@ -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
";
Expand Down Expand Up @@ -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
";
Expand Down