Re: [PATCH v10 12/12] bisect--helper: `get_terms` & `bisect_terms` shell function in C

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 25, 2016, 16:53 UTC
Message-ID: <xmqqh9bd7j6k.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAFZEwPOH+V+Nfq1KoOBBdVgeFD7YCG5c1PJrUHB50O1DkMg39Q@mail.gmail.com>

Pranit Bauva <pranit.bauva@gmail.com> writes:

Show 9 quoted lines

>>> +enum terms_defined {
>>> +       TERM_BAD = 1,
>>> +       TERM_GOOD = 2,
>>> +       TERM_NEW = 4,
>>> +       TERM_OLD = 8
>>> +};
>>> +
>>
>> What does TERM stand for  ?

The terms (words) used to denote the newer and the older parts of the history. Traditionally, as a regression-hunting tool (i.e. it used to work, where did I break it?), we called older parts of the history "good" and newer one "bad", but as people gained experience with the tool, it was found that the pair of words was error-prone to use for an opposite use case "I do not recall fixing it, but it seems to have been fixed magically, when did that happen?", and a more explicit "new" and "old" were introduced.

>> And why are the defines 1,2,4,8 ?
>> It looks as if a #define bitmap may be a better choice here ?
>> How do we do these kind of bit-wise opions otherwise ?

We might want to ask if these should even be bitwise option. A word with individually controllable bits (i.e. "flag word") makes sense only when the bits within it are largely independent. But the code does this pretty much upfront:

Show 5 quoted lines

>>> +       if (term_defined != 0 && term_defined != TERM_BAD &&
>>> +           term_defined != TERM_GOOD && term_defined != TERM_NEW &&
>>> +           term_defined != TERM_OLD)
>>> +               die(_("only one option among --term-bad, --term-good, "
>>> +                     "--term-new and --term-old can be used."));

which is a very strong indication that these bits are not.

I suspect that OPTION_CMDMODE would be a better choice to group these four options and mark them mutually incompatible automatically than OPT_BIT?