Pranit Bauva <pranit.bauva@xxxxxxxxx> 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.
>
> 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!
The original you removed because the above can take it over is this
in your patch.
-bisect_terms () {
- get_terms
- if ! test -s "$GIT_DIR/BISECT_TERMS"
- then
- die "$(gettext "no terms defined")"
- fi
- case "$#" in
- 0)
- gettextln "Your current terms are $TERM_GOOD for the old state
-and $TERM_BAD for the new state."
- ;;
- 1)
- arg=$1
- case "$arg" in
- --term-good|--term-old)
- printf '%s\n' "$TERM_GOOD"
- ;;
- --term-bad|--term-new)
- printf '%s\n' "$TERM_BAD"
- ;;
- *)
- die "$(eval_gettext "invalid argument \$arg for 'git bisect terms'.
-Supported options are: --term-good|--term-old and --term-bad|--term-new.")"
- ;;
- esac
- ;;
- *)
- usage ;;
- esac
-}
-
The fprintf() that says "no terms defined" can be made error(). The
"invalid argument" message used to be die in the original, and
should be sent to the standard error stream as you noticed.
But a bigger difference is that the original made sure that the
caller asked one question at a time. "terms --term-good --term-bad"
was responded with a "usage". That is no longer true in the
rewrite.