Hey Junio,
On Wed, Aug 3, 2016 at 12:47 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Pranit Bauva <pranit.bauva@xxxxxxxxx> writes:
>
>> +static int mark_good(const char *refname, const struct object_id *oid,
>> + int flag, void *cb_data)
>> +{
>> + int *m_good = (int *)cb_data;
>> + *m_good = 0;
>> + return 0;
>> +}
>
> See below.
>
>> +static int bisect_next_check(const struct bisect_terms *terms,
>> + const char *current_term)
>> +{
>> + int missing_good = 1, missing_bad = 1;
>
> It is somewhat unusual to start with "assume we are OK" and then
> "it turns out that we are not".
>
>> + char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad.buf);
>> + char *good_glob = xstrfmt("%s*", terms->term_good.buf);
>
> The original runs
>
> git for-each-ref "refs/bisect/$TERM_GOOD-*
>
> but this one lacks the final dash.
My bad. Will include it.
>> + if (ref_exists(bad_ref))
>> + missing_bad = 0;
>> + free(bad_ref);
>> +
>> + for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
>> + (void *) &missing_good);
>> + free(good_glob);
>
> The for-each helper does not return until it iterates over all the
> matching refs, but you are only interested in seeing if at least one
> exists. It may make sense to return 1 from mark_good() to terminate
> the traversal early.
Seems a better way. Thanks!
>> + if (!missing_good && !missing_bad)
>> + return 0;
>> +
>> + if (!current_term)
>> + return -1;
>> +
>> + if (missing_good && !missing_bad && current_term &&
>> + !strcmp(current_term, terms->term_good.buf)) {
>> + char *yesno;
>> + /*
>> + * have bad (or new) but not good (or old). We could bisect
>> + * although this is less optimum.
>> + */
>> + fprintf(stderr, "Warning: bisecting only with a %s commit\n",
>> + terms->term_bad.buf);
>
> In the original, this message goes through gettext.
Will do.
>> + if (!isatty(0))
>> + return 0;
>> + /*
>> + * TRANSLATORS: Make sure to include [Y] and [n] in your
>> + * translation. The program will only accept English input
>> + * at this point.
>> + */
>> + yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
>> + if (starts_with(yesno, "N") || starts_with(yesno, "n"))
>> + return -1;
>> + return 0;
>> + }
>
> When the control falls into the above if(){} block, the function
> will always return. It will clarify that this is the end of such a
> logical block to have a blank line here.
Will do.
>> + if (!is_empty_or_missing_file(git_path_bisect_start()))
>> + return error(_("You need to give me at least one good|old and "
>> + "bad|new revision. You can use \"git bisect "
>> + "bad|new\" and \"git bisect good|old\" for "
>> + "that. \n"));
>> + else
>> + return error(_("You need to start by \"git bisect start\". "
>> + "You then need to give me at least one good|"
>> + "old and bad|new revision. You can use \"git "
>> + "bisect bad|new\" and \"git bisect good|old\" "
>> + " for that.\n"));
>
> The i18n on these two messages seem to be different from the
> original, which asks bisect_voc to learn what 'bad' and 'good' are
> called and attempts to use these words from the vocabulary.
I have little idea about i18n. Will look more into it.
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