On Tue, Feb 22, 2022 at 11:27 AM Abhradeep Chakraborty via
GitGitGadget [off-list ref] wrote:
Usage strings for git (sub)command flags has a style guide that
suggests - first letter should not capitalized (unless requied)
s/requied/required/
and it should skip full-stop at the end of line. But there are
some files where usage-strings do not follow the above mentioned
guide. Moreover, there are no checks to verify if all usage strings
are following the guide/convention or not.
Amend the usage strings that don't follow the convention/guide and
add a check in the `parse_options_check()` function in `parse-options.c`
to check the usage strings against the style guide.
This is a relatively minor observation, but it might make sense to
split this into two patches, the first of which fixes the offending
usage strings, and the second which adds the check to parse-options.c
to prevent more offending strings from entering the project in the
future. Anyhow, not necessarily worth a reroll.
quoted hunk ↗ jump to hunk
Signed-off-by: Abhradeep Chakraborty <redacted>
---
diff --git a/parse-options.c b/parse-options.c
@@ -492,6 +492,15 @@ static void parse_options_check(const struct option *opts)
+ if (opts->type != OPTION_GROUP && opts->help &&
+ !(starts_with(opts->help, "HEAD") ||
+ starts_with(opts->help, "GPG") ||
+ starts_with(opts->help, "DEPRECATED") ||
+ starts_with(opts->help, "SHA1")) &&
+ (opts->help[0] >= 65 && opts->help[0] <= 90))
+ err |= optbug(opts, xstrfmt("help should not start with capital letter unless needed: %s", opts->help));
This list of hardcoded exceptions may become a maintenance burden. I
can figure out why OPTION_GROUP is treated specially here, but why use
magic numbers 65 and 90 rather than a more obvious function like
isupper()?
Perhaps instead of hardcoding an exception list and magic numbers, we
can use a simple heuristic instead. For instance, if the first two
characters of the help string are uppercase, then assume it is an
acronym (i.e. "GPG") or special name (i.e. "HEAD"), thus allowed.
Maybe something like this:
if (opts->type != OPTION_GROUP && opts->help &&
opts->help[0] && isupper(opts->help[0]) &&
!(opts->help[1] && isupper(opts->help[1])))
+ if (opts->help && !ends_with(opts->help, "...") && ends_with(opts->help, "."))
+ err |= optbug(opts, xstrfmt("help should not end with a dot: %s", opts->help));