From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: "Rubén Justo" <rjusto@gmail.com>, "Git List" <git@vger.kernel.org>
Subject: Re* [PATCH] add-patch: response to unknown command
Date: Tue, 21 May 2024 08:52:14 -0700 [thread overview]
Message-ID: <xmqqr0dvb1sh.fsf_-_@gitster.g> (raw)
In-Reply-To: <ZkxHLE_8OpYvmViY@tanuki> (Patrick Steinhardt's message of "Tue, 21 May 2024 09:03:08 +0200")
Patrick Steinhardt <ps@pks.im> writes:
> I'm a bit on the edge here. Is it really less confusing if we confront
> the user with a command that they have never even provided in the first
> place? They implicitly specified the first letter, only, but the user
> first needs to be aware that we discard everything but the first letter
> in the first place.
I share your doubt. If what the user said (e.g. "ues") when they
wanted to say "yes", I find "You said 'u', which I do not understand"
more confusiong than "You said 'ues', which I do not understand".
> Is it even sensible that we don't complain about trailing garbage in the
> user's answer? Shouldn't we rather fix that and make the accepted
> answers more strict, such that if the response is longer than a single
> character we point that out?
I personally guess that it is unlikely that folks are taking
advantage of the fact that everything but the first is ignored, and
I cannot think of a reason why folks prefer that behaviour offhand.
If 'q' and 'a' are next to each other on the user's keyboard, there
is a plausible chance that we see 'qa' when the user who wanted to
say 'a' fat-fingered and we ended up doing the 'q' thing instead,
and we may want to prevent such problems from happening.
Instead of ignoring, we _could_ take 'yn' and apply 'y' to the
current question, and then 'n' to the next question without
prompting (or showing prompt and answer together without taking
further answer), and claim that it is a typesaving feature, but
it is dubious users can sensibly choose the answer to a prompt
they haven't seen.
So, I am inclined to be supportive on that "tighten multi-byte
input" idea, but as I said the above is based on a mere "I cannot
think of ... offhand", so we need to see if people have reasonable
use cases to object first.
------- >8 ------------- >8 ------------- >8 ------------- >8 -------
Subject: add-patch: enforce only one-letter response to prompts
In an "git add -p" session, especially when we are not using the
single-char mode, we may see 'qa' as a response to a prompt
(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?
and then just do the 'q' thing (i.e. quit the session), ignoring
everything other than the first byte.
If 'q' and 'a' are next to each other on the user's keyboard, there
is a plausible chance that we see 'qa' when the user who wanted to
say 'a' fat-fingered and we ended up doing the 'q' thing instead.
As we didn't think of a good reason during the review discussion why
we want to accept excess letters only to ignore them, it appears to
be a safe change to simply reject input that is longer than just one
byte.
Keep the "use only the first byte, downcased" behaviour when we ask
yes/no question, though. Neither on Qwerty or on Dvorak, 'y' and
'n' are not close to each other.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
add-patch.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git c/add-patch.c w/add-patch.c
index 2252895c28..7126bc5d70 100644
--- c/add-patch.c
+++ w/add-patch.c
@@ -1227,6 +1227,7 @@ static int prompt_yesno(struct add_p_state *s, const char *prompt)
fflush(stdout);
if (read_single_character(s) == EOF)
return -1;
+ /* do not limit to 1-byte input to allow 'no' etc. */
switch (tolower(s->answer.buf[0])) {
case 'n': return 0;
case 'y': return 1;
@@ -1509,6 +1510,10 @@ static int patch_update_file(struct add_p_state *s,
if (!s->answer.len)
continue;
+ if (1 < s->answer.len) {
+ error(_("only one letter is expected, got '%s'"), s->answer.buf);
+ continue;
+ }
ch = tolower(s->answer.buf[0]);
if (ch == 'y') {
hunk->use = USE_HUNK;
next prev parent reply other threads:[~2024-05-21 15:52 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-21 0:37 [PATCH] add-patch: response to unknown command Rubén Justo
2024-05-21 7:03 ` Patrick Steinhardt
2024-05-21 12:59 ` Rubén Justo
2024-05-21 15:52 ` Junio C Hamano [this message]
2024-05-21 22:27 ` Re* " Taylor Blau
2024-05-21 23:06 ` Junio C Hamano
2024-05-21 23:20 ` [PATCH v2] add-patch: enforce only one-letter response to prompts Junio C Hamano
2024-05-21 23:36 ` Eric Sunshine
2024-05-22 0:49 ` Junio C Hamano
2024-05-22 6:40 ` Dragan Simic
2024-05-22 16:23 ` Junio C Hamano
2024-05-22 19:03 ` Dragan Simic
2024-05-22 20:41 ` Junio C Hamano
2024-05-22 11:07 ` Patrick Steinhardt
2024-05-22 16:27 ` Junio C Hamano
2024-05-22 17:14 ` [PATCH v3] " Junio C Hamano
2024-05-22 17:38 ` Rubén Justo
2024-05-22 19:27 ` Junio C Hamano
2024-05-22 21:45 ` [PATCH v4] " Junio C Hamano
2024-05-23 5:31 ` Patrick Steinhardt
2024-05-23 15:58 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqr0dvb1sh.fsf_-_@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
--cc=rjusto@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).