Re: [PATCH v3 0/4] Introduce --first-parent flag for git bisect

Aaron Lipman [off-list ref] writes:

quoted

I suspect that you would ask the same question to whoever added
no_checkout to this thing, and I wouldn't be surprised if we end up
removing both of these parameters (and parsing of the options inside
cmd_bisect__helper()) after thinking about them, but anyway, let's
read on.

Hmm. Is there a preferred way to to add a --first-parent line to the
output of "git bisect start --help" via the parse-options API without
removing the --first-parent option from argv in the process?

I'd rather not to see the code made ugly if the only reason why you
have duplicated command line parsing is to support "git bisect start
-h" while cmd_bisect__helper() still exists.  

That function is supposed to be a thin shim layer whose only reason
of its existence is to serve as an interface with the scripted
version of "git bisect".  When everything is migrated from the shell
script, cmd_bisect__helper() should disappear.

In its place, cmd_bisect() written in C would become a dispatcher
that only looks at the first token on the command line that comes
after ["git", "bisect"] and calls "bisect_start()", "bisect_next()",
etc. with the remainder of the command line with options such as
"--first-parent" and "--no-checkout", which will be parsed by
bisect_start() etc. parsing its argc/argv[] passed by cmd_bisect().
It is likely that cmd_bisect() would not have any call to
parse_options(); instead cmd_start() etc. would call parse_options()
with their own option[] table.

So I am not sure if the change between v2 and v3 is going in the
right direction.

As long as we're capturing the --first-parent option in
cmd_bisect__helper(), checking argv for a --first-parent flag in
bisect_start() is pointless.

This is going backwards, as far as I can tell.  If anything, I'd
rather see cmd_bisect__helper() get fixed so that it does not parse
"--first-parent" (and similarly, "--no-checkout" that you imitated)
into first_parent_only (and no_checkout) variables and passed as
parameters to bisect_start().  cmd_bisect__helper() should instead
just let these flags (and there may be others) sit in argc/argv[]
and have bisect_start() parse them, together with all other options
bisect_start() already has its parser for.

Thanks.