Re: [PATCH v15 18/27] bisect--helper: `bisect_autostart` shell function in C
From: Stephan Beyer <hidden>
Date: 2016-11-20 20:15:10
Hi, On 10/14/2016 04:14 PM, Pranit Bauva wrote:
quoted hunk ↗ jump to hunk
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.)
quoted hunk ↗ jump to hunk
@@ -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)?
+ 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.
quoted hunk ↗ jump to hunk
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