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

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 <redacted>

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