Re: [PATCH 2/2] builtin: use git_read_line_interactively to prompt
From: Junio C Hamano <hidden>
Date: 2021-05-13 22:51:51
Firmin Martin [off-list ref] writes:
quoted hunk
diff --git a/builtin/am.c b/builtin/am.c index 8355e3566f..32eae4eb2e 100644 --- a/builtin/am.c +++ b/builtin/am.c@@ -1643,7 +1643,7 @@ static int do_interactive(struct am_state *state) assert(state->msg); for (;;) { - char reply[64]; + struct strbuf reply = STRBUF_INIT;
If you call this reply_strbuf, and introduce "const char *reply", and ...
quoted hunk
puts(_("Commit Body is:")); puts("--------------------------");@@ -1656,17 +1656,17 @@ static int do_interactive(struct am_state *state) * input at this point. */ printf(_("Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all: ")); - if (!fgets(reply, sizeof(reply), stdin)) + if (git_read_line_interactively(&reply) == EOF) die("unable to read from stdin; aborting");
... assign "reply = reply_strbuf.buf" here, you do not have to change anything below.
- if (*reply == 'y' || *reply == 'Y') {
+ if (reply.len && !strncasecmp(reply.buf, "yes", reply.len)) {And we shouldn't sneak an unrelated change like this into a patch whose purpose is to allow reading from the standard input instead of always from "/dev/tty". As I explained earlier, git_read_line_interactively() is usable only when we know that the standard input is *not* used for the data we are operating on and is available to be used for simulating interactive input. Since "git am -i" never reads the mailbox from the standard input, the use of the helper here is acceptable.
quoted hunk
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 1fdb7d9d10..134347eb39 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c@@ -340,7 +340,7 @@ static int decide_next(const struct bisect_terms *terms, if (missing_good && !missing_bad && !strcmp(current_term, terms->term_good)) { - char *yesno; + struct strbuf yesno = STRBUF_INIT; /* * have bad (or new) but not good (or old). We could bisect * although this is less optimum.@@ -353,8 +353,9 @@ static int decide_next(const struct bisect_terms *terms, * translation. The program will only accept English input * at this point. */ - yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO); - if (starts_with(yesno, "N") || starts_with(yesno, "n")) + printf(_("Are you sure [Y/n]? ")); + git_read_line_interactively(&yesno);
The use of this helper here is also OK as the standard input to "git bisect" is available for simulating interactive input.
+ if (yesno.len && !strncasecmp(yesno.buf, "no", yesno.len)) return -1; return 0; }
It may make sense to have a thin wrapper around this pattern, e.g.
static int ask_yesno(const char *question)
{
struct strbuf answer = STRBUF_INIT;
printf("%s [Y/n]? ", question);
git_read_line_interactively(&answer);
return !(answer.buf[0] == 'N' || answer.buf[0] == 'n');
}
so that the above caller can become
return ask_yesno(_("Are you sure")) ? 0 : -1;
and ...
- yesno = git_prompt(_("Do you want me to do it for you "
- "[Y/n]? "), PROMPT_ECHO);
- res = tolower(*yesno) == 'n' ?
+ printf(_("Do you want me to do it for you [Y/n]? "));
+ git_read_line_interactively(&yesno);
+ res = (yesno.len && strncasecmp(yesno.buf, "no", yesno.len)) ?
-1 : bisect_start(terms, empty_strvec, 0);
... this caller can become
if (ask_yesno(_("Do you want me to do it for you")))
res = bisect_start(terms, empty_strvec, 0);
else
res = -1;
Thanks.