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

2016-07-27 Thread Junio C Hamano
Pranit Bauva  writes:

> On Tue, Jul 26, 2016 at 11:02 PM, Junio C Hamano  wrote:
>> Torsten Bögershausen  writes:
>>
>>> On 07/25/2016 06:53 PM, Junio C Hamano wrote:
 Pranit Bauva  writes:

>>> >>> +enum terms_defined {
>>> >>> +   TERM_BAD = 1,
>>> >>> +   TERM_GOOD = 2,
>>> >>> +   TERM_NEW = 4,
>>> >>> +   TERM_OLD = 8
>>> >>> +};
>>> >>> +
>> >> ...
>>> Is there any risk that a more generic term like "TERM_BAD" may collide
>>> with some other definition some day ?
>>>
>>> Would it make sense to call it GIT_BISECT_TERM_BAD, GBS_TERM_BAD,
>>> BIS_TERM_BAD or something more unique ?
>>
>> I am not sure if the scope of these symbols would ever escape
>> outside bisect-helper.c (and builtin/bisect.c eventually when we
>> retire git-bisect.sh), but BISECT_TERM_{GOOD,BAD,OLD,NEW} would not
>> be too bad.
>
> I agree that it wouldn't be too bad. This can be considered as low
> hanging fruit and picked up after the completion of the project as
> after the whole conversion, some re-ordering of code would need to be
> done.
> For eg. there is read_bisect_terms() is in bisect.c while
> get_terms() is in builtin/bisect--helper.c but they both do the same
> stuff except the later one uses strbuf and a lot more important stuff.

The criteria to decide if a known "room for improvement" can be left
as a "can be later touched up" is "would it be a long-term
maintenance burden if it is left unfixed?", and from that point of
view, I actually view the latter as a part of the necessary
"polishing in response to review comments" for the initial version
of a new topic to land in the official project's tree.

As to TERM vs BISECT_TERM, I do not think it matters that much as I
said (IOW, I do not think it would make it a maintenance burden if
we kept using TERM_{BAD,GOOD,NEW,OLD}).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2016-07-26 Thread Pranit Bauva
Hey Junio,

On Tue, Jul 26, 2016 at 11:02 PM, Junio C Hamano  wrote:
> Torsten Bögershausen  writes:
>
>> On 07/25/2016 06:53 PM, Junio C Hamano wrote:
>>> Pranit Bauva  writes:
>>>
>> >>> +enum terms_defined {
>> >>> +   TERM_BAD = 1,
>> >>> +   TERM_GOOD = 2,
>> >>> +   TERM_NEW = 4,
>> >>> +   TERM_OLD = 8
>> >>> +};
>> >>> +
> >> ...
>> Is there any risk that a more generic term like "TERM_BAD" may collide
>> with some other definition some day ?
>>
>> Would it make sense to call it GIT_BISECT_TERM_BAD, GBS_TERM_BAD,
>> BIS_TERM_BAD or something more unique ?
>
> I am not sure if the scope of these symbols would ever escape
> outside bisect-helper.c (and builtin/bisect.c eventually when we
> retire git-bisect.sh), but BISECT_TERM_{GOOD,BAD,OLD,NEW} would not
> be too bad.

I agree that it wouldn't be too bad. This can be considered as low
hanging fruit and picked up after the completion of the project as
after the whole conversion, some re-ordering of code would need to be
done. For eg. there is read_bisect_terms() is in bisect.c while
get_terms() is in builtin/bisect--helper.c but they both do the same
stuff except the later one uses strbuf and a lot more important stuff.
Maybe after the whole conversion, the above enum (or #define bitmap)
should also be moved to bisect.h and be used consistently in bisect.c
too.

Regards,
Pranit Bauva
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2016-07-26 Thread Junio C Hamano
Torsten Bögershausen  writes:

> On 07/25/2016 06:53 PM, Junio C Hamano wrote:
>> Pranit Bauva  writes:
>>
> >>> +enum terms_defined {
> >>> +   TERM_BAD = 1,
> >>> +   TERM_GOOD = 2,
> >>> +   TERM_NEW = 4,
> >>> +   TERM_OLD = 8
> >>> +};
> >>> +
 >> ...
> Is there any risk that a more generic term like "TERM_BAD" may collide
> with some other definition some day ?
>
> Would it make sense to call it GIT_BISECT_TERM_BAD, GBS_TERM_BAD,
> BIS_TERM_BAD or something more unique ?

I am not sure if the scope of these symbols would ever escape
outside bisect-helper.c (and builtin/bisect.c eventually when we
retire git-bisect.sh), but BISECT_TERM_{GOOD,BAD,OLD,NEW} would not
be too bad.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2016-07-25 Thread Torsten Bögershausen



On 07/25/2016 06:53 PM, Junio C Hamano wrote:

Pranit Bauva  writes:


>>> +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.


Thanks for the explanation.
Is there any risk that a more generic term like "TERM_BAD" may collide
with some other definition some day ?

Would it make sense to call it GIT_BISECT_TERM_BAD, GBS_TERM_BAD,
BIS_TERM_BAD or something more unique ?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2016-07-25 Thread Christian Couder
On Mon, Jul 25, 2016 at 6:53 PM, Junio C Hamano  wrote:
> Pranit Bauva  writes:
>
>>> 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:
>
 +   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?

I must say that Pranit used that at one point, but it felt weird to me
to use that for things that is not really a command.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2016-07-25 Thread Junio C Hamano
Pranit Bauva  writes:

>>> +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:

>>> +   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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2016-07-22 Thread Pranit Bauva
Hey Torsten,

On Fri, Jul 22, 2016 at 7:59 AM, Torsten Bögershausen  wrote:
>
>
> On 07/20/2016 11:47 PM, Pranit Bauva wrote:
>>
>> Reimplement the `get_terms` and `bisect_terms` shell function in C and
>> add `bisect-terms` subcommand to `git bisect--helper` to call it from
>> git-bisect.sh .
>>
>> In the shell version, the terms were identified by strings but in C
>> version its done by bit manipulation and passing the integer value to
>> the function.
>>
>> Using `--bisect-terms` subcommand is a temporary measure to port shell
>> function in C so as to use the existing test suite. As more functions
>> are ported, this subcommand will be retired and will be called by some
>> other methods.
>>
>> Mentored-by: Lars Schneider 
>> Mentored-by: Christian Couder 
>> Signed-off-by: Pranit Bauva 
>> ---
>>  builtin/bisect--helper.c | 74
>> +++-
>>  git-bisect.sh| 35 ++-
>>  2 files changed, 75 insertions(+), 34 deletions(-)
>>
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> index 001096a..185a8ad 100644
>> --- a/builtin/bisect--helper.c
>> +++ b/builtin/bisect--helper.c
>> @@ -8,6 +8,13 @@
>>  #include "run-command.h"
>>  #include "prompt.h"
>>
>> +enum terms_defined {
>> +   TERM_BAD = 1,
>> +   TERM_GOOD = 2,
>> +   TERM_NEW = 4,
>> +   TERM_OLD = 8
>> +};
>> +
>
> What does TERM stand for  ?
> It could be TERMinal, TERMinator or just TERM.
> Something like BIS_TERM_DEF_BAD .. may be more intuitive,
> and may avoid name clashes in the long run.
>
> 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 ?

I am not sure as why bitmaps would be a better choice except for git's
source code. I saw the source code (especially config.c) and it uses
"#defines" bitmap style. I haven't been able to find this method
before. Also it uses "(1<<2)" instead of "4".

>>  static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
>>  static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
>>  static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
>> @@ -26,6 +33,7 @@ static const char * const git_bisect_helper_usage[] = {
>> N_("git bisect--helper --bisect-write  
>>   []"),
>> N_("git bisect--helper --bisect-check-and-set-terms 
>>  "),
>> N_("git bisect--helper --bisect-next-check [] 
>> > +   N_("git bisect--helper --bisect-terms [--term-good | --term-old |
>> --term-bad | --term-new]"),
>> NULL
>>  };
>>
>> @@ -359,6 +367,43 @@ static int bisect_next_check(const struct
>> bisect_terms *terms,
>> return 0;
>>  }
>>
>> +static int get_terms(struct bisect_terms *terms)
>> +{
>> +   FILE *fp;
>> +   int res;
>> +   fp = fopen(git_path_bisect_terms(), "r");
>> +   if (!fp)
>> +   return -1;
>> +
>> +   bisect_terms_reset(terms);
>> +   res = strbuf_getline(&terms->term_bad, fp) == EOF ||
>> + strbuf_getline(&terms->term_good, fp) == EOF;
>> +
>> +   fclose(fp);
>> +   return res ? -1 : 0;
>> +}
>> +
>> +static int bisect_terms(struct bisect_terms *terms, int term_defined)
>> +{
>> +   if (get_terms(terms)) {
>> +   fprintf(stderr, "no terms defined\n");
>> +   return -1;
>> +   }
>> +   if (!term_defined) {
>> +   printf("Your current terms are %s for the old state\nand "
>> +  "%s for the new state.\n", terms->term_good.buf,
>> +  terms->term_bad.buf);
>> +   return 0;
>> +   }
>> +
>> +   if (term_defined == TERM_GOOD || term_defined == TERM_OLD)
>> +   printf("%s\n", terms->term_good.buf);
>> +   if (term_defined == TERM_BAD || term_defined == TERM_NEW)
>> +   printf("%s\n", terms->term_bad.buf);
>
> May be a switch-case ?
> Or at least "else if" ?

Yes. I will use a "else if". Thanks!

>> +
>> +   return 0;
>> +}
>> +
>>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>>  {
>> enum {
>> @@ -369,9 +414,11 @@ int cmd_bisect__helper(int argc, const char **argv,
>> const char *prefix)
>> CHECK_EXPECTED_REVS,
>> BISECT_WRITE,
>> CHECK_AND_SET_TERMS,
>> -   BISECT_NEXT_CHECK
>> +   BISECT_NEXT_CHECK,
>> +   BISECT_TERMS
>> } cmdmode = 0;
>> int no_checkout = 0, res = 0;
>> +   enum terms_defined term_defined = 0;
>> struct option options[] = {
>> OPT_CMDMODE(0, "next-all", &cmdmode,
>>  N_("perform 'git bisect next'"), NEXT_ALL),
>> @@ -389,6 +436,16 @@ int cmd_bisect__helper(int argc, const char **argv,
>> const char *prefix)
>>  N_("check and set terms in a bisection state"),
>> CHECK_AND_SET_TERMS),
>> OPT_CMDMODE(0, "bisec

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

2016-07-21 Thread Torsten Bögershausen



On 07/20/2016 11:47 PM, Pranit Bauva wrote:

Reimplement the `get_terms` and `bisect_terms` shell function in C and
add `bisect-terms` subcommand to `git bisect--helper` to call it from
git-bisect.sh .

In the shell version, the terms were identified by strings but in C
version its done by bit manipulation and passing the integer value to
the function.

Using `--bisect-terms` subcommand is a temporary measure to port shell
function in C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other methods.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 74 +++-
 git-bisect.sh| 35 ++-
 2 files changed, 75 insertions(+), 34 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 001096a..185a8ad 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -8,6 +8,13 @@
 #include "run-command.h"
 #include "prompt.h"

+enum terms_defined {
+   TERM_BAD = 1,
+   TERM_GOOD = 2,
+   TERM_NEW = 4,
+   TERM_OLD = 8
+};
+

What does TERM stand for  ?
It could be TERMinal, TERMinator or just TERM.
Something like BIS_TERM_DEF_BAD .. may be more intuitive,
and may avoid name clashes in the long run.

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 ?


 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
 static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
 static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
@@ -26,6 +33,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-write 
[]"),
N_("git bisect--helper --bisect-check-and-set-terms   
"),
N_("git bisect--helper --bisect-next-check []  
term_bad, fp) == EOF ||
+ strbuf_getline(&terms->term_good, fp) == EOF;
+
+   fclose(fp);
+   return res ? -1 : 0;
+}
+
+static int bisect_terms(struct bisect_terms *terms, int term_defined)
+{
+   if (get_terms(terms)) {
+   fprintf(stderr, "no terms defined\n");
+   return -1;
+   }
+   if (!term_defined) {
+   printf("Your current terms are %s for the old state\nand "
+  "%s for the new state.\n", terms->term_good.buf,
+  terms->term_bad.buf);
+   return 0;
+   }
+
+   if (term_defined == TERM_GOOD || term_defined == TERM_OLD)
+   printf("%s\n", terms->term_good.buf);
+   if (term_defined == TERM_BAD || term_defined == TERM_NEW)
+   printf("%s\n", terms->term_bad.buf);

May be a switch-case ?
Or at least "else if" ?


+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
@@ -369,9 +414,11 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
CHECK_EXPECTED_REVS,
BISECT_WRITE,
CHECK_AND_SET_TERMS,
-   BISECT_NEXT_CHECK
+   BISECT_NEXT_CHECK,
+   BISECT_TERMS
} cmdmode = 0;
int no_checkout = 0, res = 0;
+   enum terms_defined term_defined = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", &cmdmode,
 N_("perform 'git bisect next'"), NEXT_ALL),
@@ -389,6 +436,16 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("check and set terms in a bisection state"), 
CHECK_AND_SET_TERMS),
OPT_CMDMODE(0, "bisect-next-check", &cmdmode,
 N_("check whether bad or good terms exist"), 
BISECT_NEXT_CHECK),
+   OPT_CMDMODE(0, "bisect-terms", &cmdmode,
+N_("print out the bisect terms"), BISECT_TERMS),
+   OPT_BIT(0, "term-bad", &term_defined,
+N_("show the bad term"), TERM_BAD),
+   OPT_BIT(0, "term-good", &term_defined,
+N_("show the good term"), TERM_GOOD),
+   OPT_BIT(0, "term-new", &term_defined,
+N_("show the new term"), TERM_NEW),
+   OPT_BIT(0, "term-old", &term_defined,
+N_("show the old term"), TERM_OLD),
OPT_BOOL(0, "no-checkout", &no_checkout,
 N_("update BISECT_HEAD instead of checking out the current 
commit")),
OPT_END()
@@ -399,6 +456,16 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
argc = parse_options(argc, argv, prefix, options,
 git_bisect_helper_usage, 0);

+   if (cmdmode != BISECT_TERMS && term_defined)
+   die(_("--term-bad, --term-good, --ter

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

2016-07-20 Thread Pranit Bauva
Reimplement the `get_terms` and `bisect_terms` shell function in C and
add `bisect-terms` subcommand to `git bisect--helper` to call it from
git-bisect.sh .

In the shell version, the terms were identified by strings but in C
version its done by bit manipulation and passing the integer value to
the function.

Using `--bisect-terms` subcommand is a temporary measure to port shell
function in C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other methods.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 74 +++-
 git-bisect.sh| 35 ++-
 2 files changed, 75 insertions(+), 34 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 001096a..185a8ad 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -8,6 +8,13 @@
 #include "run-command.h"
 #include "prompt.h"
 
+enum terms_defined {
+   TERM_BAD = 1,
+   TERM_GOOD = 2,
+   TERM_NEW = 4,
+   TERM_OLD = 8
+};
+
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
 static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
 static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
@@ -26,6 +33,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-write
 []"),
N_("git bisect--helper --bisect-check-and-set-terms  
 "),
N_("git bisect--helper --bisect-next-check []  
term_bad, fp) == EOF ||
+ strbuf_getline(&terms->term_good, fp) == EOF;
+
+   fclose(fp);
+   return res ? -1 : 0;
+}
+
+static int bisect_terms(struct bisect_terms *terms, int term_defined)
+{
+   if (get_terms(terms)) {
+   fprintf(stderr, "no terms defined\n");
+   return -1;
+   }
+   if (!term_defined) {
+   printf("Your current terms are %s for the old state\nand "
+  "%s for the new state.\n", terms->term_good.buf,
+  terms->term_bad.buf);
+   return 0;
+   }
+
+   if (term_defined == TERM_GOOD || term_defined == TERM_OLD)
+   printf("%s\n", terms->term_good.buf);
+   if (term_defined == TERM_BAD || term_defined == TERM_NEW)
+   printf("%s\n", terms->term_bad.buf);
+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
@@ -369,9 +414,11 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
CHECK_EXPECTED_REVS,
BISECT_WRITE,
CHECK_AND_SET_TERMS,
-   BISECT_NEXT_CHECK
+   BISECT_NEXT_CHECK,
+   BISECT_TERMS
} cmdmode = 0;
int no_checkout = 0, res = 0;
+   enum terms_defined term_defined = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", &cmdmode,
 N_("perform 'git bisect next'"), NEXT_ALL),
@@ -389,6 +436,16 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
 N_("check and set terms in a bisection state"), 
CHECK_AND_SET_TERMS),
OPT_CMDMODE(0, "bisect-next-check", &cmdmode,
 N_("check whether bad or good terms exist"), 
BISECT_NEXT_CHECK),
+   OPT_CMDMODE(0, "bisect-terms", &cmdmode,
+N_("print out the bisect terms"), BISECT_TERMS),
+   OPT_BIT(0, "term-bad", &term_defined,
+N_("show the bad term"), TERM_BAD),
+   OPT_BIT(0, "term-good", &term_defined,
+N_("show the good term"), TERM_GOOD),
+   OPT_BIT(0, "term-new", &term_defined,
+N_("show the new term"), TERM_NEW),
+   OPT_BIT(0, "term-old", &term_defined,
+N_("show the old term"), TERM_OLD),
OPT_BOOL(0, "no-checkout", &no_checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -399,6 +456,16 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
argc = parse_options(argc, argv, prefix, options,
 git_bisect_helper_usage, 0);
 
+   if (cmdmode != BISECT_TERMS && term_defined)
+   die(_("--term-bad, --term-good, --term-new and --term-old "
+ "can be used only with --bisect-terms"));
+
+   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."));
+
if (!cmdmode)