On Tue, May 21, 2024 at 04:20:16PM -0700, Junio C Hamano wrote:
> 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.
I think it's a good idea regardless of the layout. There are tons of
layouts out there that are very esoteric (I for one use NEO2, which most
nobody has ever heard of), and I'm sure you will find at least one
layout where characters are positioned such that you can fat finger
things.
Another argument that is independent of fat fingering is that it
potentially allows us to expand this feature with multi-byte verbs going
forward.
[snip]
> 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.
Just to prove my point: Workman layout has them right next to each other
:) What we make of that information is a different question though.
> 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 != '/')) {
I find this condition a bit unusual and thus hard to read. If it instead
said `s->answer.len != 1` then it would be way easier to comprehend.
Also, none of the branches othar than for 'g' and '/' use `s->answer`,
so this should be safe. I also very much agree with the general idea of
this patch.
> + error(_("only one letter is expected, got '%s'"), s->answer.buf);
> + continue;
> + }
> if (ch == 'y') {
> hunk->use = USE_HUNK;
> soft_increment:
I assume we also want a test for this new behaviour, right?
Patrick
Attachment:
signature.asc
Description: PGP signature
