Hello Junio,
Please see my comments below.
On 2024-05-22 01:20, Junio C Hamano wrote:
In an "git add -p" session, especially when we are not using the
s/In an/In a/
single-char mode, we may see 'qa' as a response to a prompt
Perhaps s/single-char/single-character/
(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.
The two exceptions are the 'g' command that takes a hunk number, and
the '/' command that takes a regular expression. They has to be
accompanied by their operands (this makes me wonder how users who
set the interactive.singlekey configuration feed these operands---it
turns out that we notice there is no operand and give them another
chance to type the operand separately, without using single key
input this time), so we accept a string that is more than one byte
long.
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@xxxxxxxxx>
---
* This version fixes the breakage in t3701 where we exercise the
'/' command. Further code inspection reveals that 'g' also needs
to be special cased.
The previous iteration was <xmqqr0dvb1sh.fsf_-_@gitster.g>.
add-patch.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/add-patch.c b/add-patch.c
index 79eda168eb..a6c3367d59 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1228,6 +1228,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;
@@ -1506,6 +1507,12 @@ static int patch_update_file(struct add_p_state
*s,
if (!s->answer.len)
continue;
ch = tolower(s->answer.buf[0]);
+
+ /* 'g' takes a hunk number, '/' takes a regexp */
+ if (1 < s->answer.len && (ch != 'g' && ch != '/')) {
To me, "s->answer.len > 1" would be much more readable, and
I was surprised a bit to see the flipped variant. This made
me curious; would you, please, let me know why do you prefer
this form?
+ error(_("only one letter is expected, got '%s'"), s->answer.buf);
+ continue;
+ }
if (ch == 'y') {
hunk->use = USE_HUNK;
soft_increment:
The patch is looking good to me, and I find it good that it
improves the strictness of the user input, which should also
improve the overall user experience.
