Re: [PATCH v4] add-patch: enforce only one-letter response to prompts

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2789 bytes --]

On Wed, May 22, 2024 at 02:45:48PM -0700, Junio C Hamano wrote:
> In a "git add -p" session, especially when we are not using the
> single-key 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.
> 
> The two exceptions are the 'g' command that takes a hunk number, and
> the '/' command that takes a regular expression.  They have 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@pobox.com>

This version looks good to me, thanks!

> ---
>  * Hopefully the final iteration.  The differences are:
> 
>    - The end-user facing "here is what is wrong with your input"
>      message is given with err() to be consistent with other such
>      messages.
> 
>    - I gave up basing this on v2.44.0, as it is a new feature that
>      does not have to be merged down to older maintenance tracks.
>      This is now based on 80dbfac2 (Merge branch
>      'rj/add-p-typo-reaction', 2024-05-08), which is before v2.45.1
>      but has modern enough t3701 and add-patch.c:err() sends its
>      output to the standard output stream.
> 
>    - The tests for 'g' and '/' to check both the stuck and the split
>      forms have been updated for the more recent prompt that
>      includes 'p'.
> 
>    - The test for multi-key sequence expects the err() output on the
>      standard output stream.
> 
>    As an experiment, this message has the range-diff at the end, not
>    before the primary part of the patch text.  I think this format
>    should be easier to read for reviewers.

Huh, interesting. I do like that format better indeed. You did that
manually instead of using `--range-diff`, right?

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]