Re: [PATCH v15 18/27] bisect--helper: `bisect_autostart` shell function in C
- From
Stephan Beyer <s-beyer@gmx.net>
- Date
- Nov 20, 2016, 20:15 UTC
- Message-ID
- <723476f6-2c8c-38df-1771-9a525196d9de@gmx.net>
- In-Reply-To
- <01020157c38b1b1a-067117ef-cd0d-469b-ba80-ea1a1169f694-000000@eu-west-1.amazonses.com>
Hi,
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
Show 9 quoted lines
> 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.)
Show 13 quoted lines
> @@ -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)?
Show 5 quoted lines
> + return 1; > + > + /* > + * TRANSLATORS: Make sure to include [Y] and [n] in your > + * translation. THe program will only accept English input
Typo "THe"
> + * 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.
> + 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.
Show 9 quoted lines
> 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.
~Stephan