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 <larsxschnei...@gmail.com>
Mentored-by: Christian Couder <chrisc...@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.ba...@gmail.com>
---
 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 <state> <revision> <TERM_GOOD> <TERM_BAD> 
[<nolog>]"),
        N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> 
<TERM_BAD>"),
        N_("git bisect--helper --bisect-next-check [<term>] <TERM_GOOD> 
<TERM_BAD"),
+       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" ?

+
+       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."));
+
[]
--
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

Reply via email to