Re: [PATCH v2] add usage-strings check and amend remaining usage strings

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Feb 23, 2022 at 4:17 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> >> +               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))
> >
> > 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])))
>
> Much better than what was posted, but such a heuristic deserves some
> in-code comment to check why we see the first two.

Yes, I had the same thought as soon as I walked away from the computer
and was going to post a follow-up email to say as much but got
distracted by other things and never got around to it. Thanks for
filling in the gap.