On Mon, Jul 19, 2021 at 01:18:49PM -0400, Taylor Blau wrote: > Since cd57bc41bb (builtin/multi-pack-index.c: display usage on > unrecognized command, 2021-03-30) we have used a "usage" label to avoid > having two separate callers of usage_with_options (one when no arguments > are given, and another for unrecognized sub-commands). > > But the first caller has been broken since cd57bc41bb, since it will > happily jump to usage without arguments, and then pass argv[0] to the > "unrecognized subcommand" error. > > Many compilers will save us from a segfault here, but the end result is > ugly, since it mentions an unrecognized subcommand when we didn't even > pass one, and (on GCC) includes "(null)" in its output. > > Move the "usage" label down past the error about unrecognized > subcommands so that it is only triggered when it should be. While we're > at it, bulk up our test coverage in this area, too. Good find. The code change seems obviously correct. > +test_expect_success 'usage shown without sub-command' ' > + test_expect_code 129 git multi-pack-index 2>err && > + ! test_i18ngrep "unrecognized subcommand" err > +' I think we're avoiding test_i18ngrep in new code these days. -Peff
