Hey Stephan,
On Mon, Nov 21, 2016 at 1:45 AM, Stephan Beyer <s-beyer@xxxxxxx> wrote:
> Hi,
>
> On 10/14/2016 04:14 PM, Pranit Bauva wrote:
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> index 502bf18..1767916 100644
>> --- a/builtin/bisect--helper.c
>> +++ b/builtin/bisect--helper.c
>> @@ -422,6 +425,7 @@ static int bisect_next(...)
>> {
>> int res, no_checkout;
>>
>> + bisect_autostart(terms);
>
> You are not checking for return values here. (The shell code simply
> exited if there is no tty, but you don't.)
True. I didn't notice it carefully. Thanks for pointing it out.
>> @@ -754,6 +758,32 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
>> return retval || bisect_auto_next(terms, NULL);
>> }
>>
>> +static int bisect_autostart(struct bisect_terms *terms)
>> +{
>> + if (is_empty_or_missing_file(git_path_bisect_start())) {
>> + const char *yesno;
>> + const char *argv[] = {NULL};
>> + fprintf(stderr, _("You need to start by \"git bisect "
>> + "start\"\n"));
>> +
>> + if (!isatty(0))
>
> isatty(STDIN_FILENO)?
Seems better.
>> + return 1;
>> +
>> + /*
>> + * TRANSLATORS: Make sure to include [Y] and [n] in your
>> + * translation. THe program will only accept English input
>
> Typo "THe"
Sure.
>> + * at this point.
>> + */
>
> Taking "at this point" into consideration, I think the Y and n can be
> easily translated now that it is in C. I guess, by using...
>
>> + yesno = git_prompt(_("Do you want me to do it for you "
>> + "[Y/n]? "), PROMPT_ECHO);
>> + if (starts_with(yesno, "n") || starts_with(yesno, "N"))
>
> ... starts_with(yesno, _("n")) || starts_with(yesno, _("N"))
> here (but not sure). However, this would be an extra patch on top of
> this series.
Can add it as an extra patch. Thanks for informing.
>> + exit(0);
>
> Shouldn't this also be "return 1;"? Saying "no" is the same outcome as
> not having a tty to ask for yes or no.
Yes.
>> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>> {
>> enum {
>> @@ -790,6 +821,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>> N_("find the next bisection commit"), BISECT_NEXT),
>> OPT_CMDMODE(0, "bisect-auto-next", &cmdmode,
>> N_("verify the next bisection state then find the next bisection state"), BISECT_AUTO_NEXT),
>> + OPT_CMDMODE(0, "bisect-autostart", &cmdmode,
>> + N_("start the bisection if BISECT_START empty or missing"), BISECT_AUTOSTART),
>
> The word "is" is missing.
Sure. Thanks for going through these patches very carefully.
Regards,
Pranit Bauva