Re: [PATCH v2] bisect--helper: convert a function in shell to C
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Mar 22, 2016, 16:49 UTC
- Message-ID
- <alpine.DEB.2.20.1603221748360.4690@virtualbox>
- In-Reply-To
- <xmqqh9fyjy9w.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 22 Mar 2016, Junio C Hamano wrote:
Show 21 quoted lines
> 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