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

Pranit Bauva [off-list ref] 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.

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