Hey Torsten,
On Fri, Jul 22, 2016 at 7:59 AM, Torsten Bögershausen <tboegi@xxxxxx> 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 <larsxschneider@xxxxxxxxx>
>> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
>> Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx>
>> ---
>> 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 <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" ?
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, "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."));
>> +
>
> []
However I suspect handling "--term-good/--term-bad" is creating
problems in bisect_start(). I am finding a way around. If not then I
will have to get back to using "OPT_ARGUMENT" and handling it in the
individual function.
Regards,
Pranit Bauva
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html