Re: [PATCH v14 11/27] bisect--helper: `bisect_next_check` & bisect_voc shell function in C

> Pranit Bauva <pranit.bauva@gmail.com> 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 1;
>> +}
>> +
>> +static char *bisect_voc(char *revision_type)
>> +{
>> +     if (!strcmp(revision_type, "bad"))
>> +             return "bad|new";
>> +     if (!strcmp(revision_type, "good"))
>> +             return "good|old";
>> +
>> +     return NULL;
>> +}
>> +
>> +static int bisect_next_check(const struct bisect_terms *terms,
>> +                          const char *current_term)
>> +{
>> +     int missing_good = 1, missing_bad = 1;
>> +     char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad.buf);
>> +     char *good_glob = xstrfmt("%s-*", terms->term_good.buf);
>> +     char *bad_syn, *good_syn;
>> +
>> +     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);
>> +
>> +     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);
>> +             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;
>> +     }
>> +     bad_syn = xstrdup(bisect_voc("bad"));
>> +     good_syn = xstrdup(bisect_voc("good"));
>> +     if (!is_empty_or_missing_file(git_path_bisect_start())) {
>> +             error(_("You need to give me at least one %s and "
>> +                     "%s revision. You can use \"git bisect %s\" "
>> +                     "and \"git bisect %s\" for that. \n"),
>> +                     bad_syn, good_syn, bad_syn, good_syn);
>> +             free(bad_syn);
>> +             free(good_syn);
>> +             return -1;
>> +     }
>> +     else {
>> +             error(_("You need to start by \"git bisect start\". You "
>> +                     "then need to give me at least one %s and %s "
>> +                     "revision. You can use \"git bisect %s\" and "
>> +                     "\"git bisect %s\" for that.\n"),
>> +                     good_syn, bad_syn, bad_syn, good_syn);
>> +             free(bad_syn);
>> +             free(good_syn);
>> +             return -1;
>> +     }
>> +     free(bad_syn);
>> +     free(good_syn);
>> +
>> +     return 0;
>
> This one looks OK, but I think the same "Wouldn't it become cleaner
> to have a 'finish:' label at the end and jump there?" comment
> applies to this implementation, too.