On Wed, Jan 15, 2025 at 05:25:22PM -0800, Junio C Hamano wrote: > Using the show_usage_and_exit_if_asked() helper we introduced > earlier, fix callers of usage() that want to show the help text when > explicitly asked by the end-user. The help text now goes to the > standard output stream for them. > > The callers in this step are oddballs in that their invocations of > usage() are *not* guarded by > > if (argc == 2 && !strcmp(argv[1], "-h") > usage(...); > > They are (unnecessarily) being clever and do things like > > if (argc != 2 || !strcmp(argv[1], "-h") > usage(...); > > to say "I know I take only one argument, so argc != 2 is always an > error regardless of what is in argv[]. Ah, by the way, even if argc > is 2, "-h" is a request for usage text, so we do the same". Some > just do not treat "-h" any specially, and let it take the same error > code paths as a parameter error. As the author of at least one of these, I feel judged. :) But yes, untangling this is obviously good (especially if we change the exit code later!). And pulling these into their own commit made reviewing much easier. > diff --git a/builtin/var.c b/builtin/var.c > index 1449656cc9..6a09c1c39a 100644 > --- a/builtin/var.c > +++ b/builtin/var.c > @@ -221,6 +221,7 @@ int cmd_var(int argc, > const struct git_var *git_var; > char *val; > > + show_usage_and_exit_if_asked(argc, argv, var_usage); > if (argc != 2) > usage(var_usage); Hmm, what's going on in this one? It does not check "-h" at all. Ah, I see. It simply hits the get_git_var() call, sees there is no var called "-h", and exits with an error. So checking for "-h" up front is correct. It is an oddball, though not quite the same as the others (the oddest ball?). -Peff