Re: [RFC/PATCH v11 11/13] bisect--helper: `bisect_next_check` shell function in C
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 2, 2016, 19:17 UTC
- Message-ID
- <xmqq60rjdlq4.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <0102015640423d08-3af7e66f-4104-4b0c-a3c7-dff29b843b2d-000000@eu-west-1.amazonses.com>
Pranit Bauva <pranit.bauva@gmail.com> writes:
Show 7 quoted lines
> +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.
Show 7 quoted lines
> + 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.
Show 15 quoted lines
> + 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.
Show 12 quoted lines
> + 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.
Show 11 quoted lines
> + 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.