From: Ben Knoble Date: Thu, 23 Oct 2025 15:14:05 GMT Subject: Re: [PATCH] bisect: fix handling of `help` and invalid subcommands Message-ID: In-Reply-To: > Le 22 oct. 2025 à 16:15, Ruoyu Zhong a écrit : > > Hi Ben, > > > Thanks for the review! > >> On Oct 23, 2025, at 1:52 AM, Ben Knoble wrote: >> Good catch! >> FWIW, in this project we describe the buggy behavior in the present tense (« is broken », « Running git bisect shows », etc.) > > Thanks! Will keep this in mind. > >>> diff --git a/builtin/bisect.c b/builtin/bisect.c >>> index 8b8d870cd1..993caf545d 100644 >>> --- a/builtin/bisect.c >>> +++ b/builtin/bisect.c >>> @@ -1453,9 +1453,13 @@ int cmd_bisect(int argc, >>> if (!argc) >>> usage_msg_opt(_("need a command"), git_bisect_usage, options); >>> + if (!strcmp(argv[0], "help")) >>> + usage_with_options(git_bisect_usage, options); >>> + >> From an extremely quick look at the code, this might be better handled with a new OPT_SUBCOMMAND, though that might mean making the options array statically scoped to this file rather than the function. > > I intended to keep it simple so I did not make it an OPT_SUBCOMMAND at the first > place. Given that Junio is okay with it, I'm going to keep it as is for now. > Still happy to turn this into an OPT_SUBCOMMAND if you would like. > >> It would also be nice to update the usage to match the manual while we’re here, which presumably in turn affects the test between command usage and manuals. > > > Thanks for pointing that out! Yes, I think so too. Will do in a separate patch, > if you agree, in order not to digress too much. Separate is fine, although it might have been nice as part of a series with this one. Since Junio has already marked it to queue, no reason to revisit all the above for this patch. Thanks !