Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > s/requied/required/ Thanks for pointing out. Will fix it. > 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. I made a single commit because both changes are small. But I think You're right - I should split it into two. > 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()? Hmm, this was a quick fix came into my mind. So, I didn't look at other (and better) options for this. Will fix it :) > 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]))) Okay, got it! Thanks :)
