Hey Stephan,
On Mon, Oct 30, 2017 at 10:04 PM, Stephan Beyer <s-beyer@xxxxxxx> wrote:
> On 10/27/2017 05:06 PM, Pranit Bauva wrote:
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> index 0f9c3e63821b8..ab0580ce0089a 100644
>> --- a/builtin/bisect--helper.c
>> +++ b/builtin/bisect--helper.c
> [...]
>> +static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
>> +{
>> + int i;
>> +
>> + if (get_terms(terms))
>> + return error(_("no terms defined"));
>> +
>> + if (argc > 1)
>> + return error(_("--bisect-term requires exactly one argument"));
>> +
>> + if (argc == 0)
>> + return !printf(_("Your current terms are %s for the old state\n"
>> + "and %s for the new state.\n"),
>> + terms->term_good, terms->term_bad);
>
> Same as in 1/8: you probably want "printf(...); return 0;" except there
> is a good reason.
No good reason. I will make the change.
>> +
>> + for (i = 0; i < argc; i++) {
>> + if (!strcmp(argv[i], "--term-good"))
>> + printf(_("%s\n"), terms->term_good);
>> + else if (!strcmp(argv[i], "--term-bad"))
>> + printf(_("%s\n"), terms->term_bad);
>
> The last two printfs: I think there is no point in translating "%s\n",
> so using "%s\n" instead of _("%s\n") looks more reasonable.
Also this probably does weird things with GETTEXT_POISON. I am
investigating what's happening as Martin pointed out in other thread..
>> + else
>> + error(_("BUG: invalid argument %s for 'git bisect terms'.\n"
>> + "Supported options are: "
>> + "--term-good|--term-old and "
>> + "--term-bad|--term-new."), argv[i]);
>
> Should this be "return error(...)"?
Yeah. I missed this
>> + }
>> +
>> + return 0;
>> +}
>> +
>
> Stephan
Regards,
Pranit Bauva
