Re: [PATCH v2] bisect--helper: convert a function in shell to C

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Tue, 22 Mar 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >>  	struct option options[] = {
> >>  		OPT_BOOL(0, "next-all", &next_all,
> >>  			 N_("perform 'git bisect next'")),
> >>  		OPT_BOOL(0, "no-checkout", &no_checkout,
> >>  			 N_("update BISECT_HEAD instead of checking out the current commit")),
> >> +		OPT_STRING(0, "check-term-format", &term, N_("term"),
> >> +			 N_("check the format of the ref")),
> >
> > Hmm. The existing code suggests to use OPT_BOOL instead.
> > ...
> > The existing convention is to make the first argument *not* a value of the
> > "option", i.e. `--check-term-format "$TERM_BAD"` without an equal sign.
> 
> I think it is preferrable to keep using OPT_BOOL() for this new one
> if we are incrementally building on top of existing code.
> 
> But if the convention is that the option is to specify what opration
> is invoked, using OPT_CMDMODE() to implement all of them would be a
> worthwhile cleanup to consider at some point.

Good point, I keep forgetting that OPT_CMDMODE() was introduced
specifically for subcommands.

Ciao,
Dscho