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

Hey Junio,

On Thu, Aug 25, 2016 at 4:10 AM, Junio C Hamano [off-list ref] wrote:

Pranit Bauva [off-list ref] writes:

quoted

+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.

For this goto can simply things. Will do!

Regards,
Pranit Bauva