Re: [PATCH v14 12/27] bisect--helper: `get_terms` & `bisect_terms` shell function in C

Pranit Bauva [off-list ref] writes:

+static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
+{
+	int i;
+
+	if (get_terms(terms)) {
+		fprintf(stderr, _("no terms defined\n"));
+		return -1;
+	}
+	if (argc == 0) {
+		printf(_("Your current terms are %s for the old state\nand "
+		       "%s for the new state.\n"), terms->term_good.buf,
+		       terms->term_bad.buf);
+		return 0;
+	}
+
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argv[i], "--term-good"))
+			printf("%s\n", terms->term_good.buf);
+		else if (!strcmp(argv[i], "--term-bad"))
+			printf("%s\n", terms->term_bad.buf);
+		else
+			printf(_("invalid argument %s for 'git bisect "
+				  "terms'.\nSupported options are: "
+				  "--term-good|--term-old and "
+				  "--term-bad|--term-new."), argv[i]);
+	}

The original took only one and gave one answer (and errored out when
the user asked for more), but this one loops.  I can see either way
is OK and do not think of a good reason to favor one over the other;
unless there is a strong reason why you need this extended behaviour
that allows users to ask multiple questions, I'd say we should keep
the original behaviour.

Hey Junio,

On Thu, Aug 25, 2016 at 11:35 PM, Junio C Hamano [off-list ref] wrote:

Pranit Bauva [off-list ref] writes:

quoted

+static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
+{
+     int i;
+
+     if (get_terms(terms)) {
+             fprintf(stderr, _("no terms defined\n"));
+             return -1;
+     }
+     if (argc == 0) {
+             printf(_("Your current terms are %s for the old state\nand "
+                    "%s for the new state.\n"), terms->term_good.buf,
+                    terms->term_bad.buf);
+             return 0;
+     }
+
+     for (i = 0; i < argc; i++) {
+             if (!strcmp(argv[i], "--term-good"))
+                     printf("%s\n", terms->term_good.buf);
+             else if (!strcmp(argv[i], "--term-bad"))
+                     printf("%s\n", terms->term_bad.buf);
+             else
+                     printf(_("invalid argument %s for 'git bisect "
+                               "terms'.\nSupported options are: "
+                               "--term-good|--term-old and "
+                               "--term-bad|--term-new."), argv[i]);
+     }

The original took only one and gave one answer (and errored out when
the user asked for more), but this one loops.  I can see either way
is OK and do not think of a good reason to favor one over the other;
unless there is a strong reason why you need this extended behaviour
that allows users to ask multiple questions, I'd say we should keep
the original behaviour.

True! I can just use return error() instead of printf. Also I noticed
that this is printing to stdout while the original printed it to
stderr. Thanks!

Regards,
Pranit Bauva