add-patch: enforce only one-letter response to prompts

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>

diff --git a/add-patch.c b/add-patch.c
index 2252895c28044fc2e4b6ff4a2396732c57ad8940..814de57c4a70054ebc30e770a16922b4ea71b6e8 100644 (file)
--- a/add-patch.c
+++ b/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;
@@ -1510,6 +1511,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 and '/' takes a regexp */
+               if (s->answer.len != 1 && (ch != 'g' && ch != '/')) {
+                       err(s, _("Only one letter is expected, got '%s'"), s->answer.buf);
+                       continue;
+               }
                if (ch == 'y') {
                        hunk->use = USE_HUNK;
 soft_increment:

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 28a95a775d3d9a5c2274cc666c5b4c5f04c26893..6624a4f7c05b595527d2eedc6b3ebda861cc12ac 100755 (executable)
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -160,6 +160,14 @@ test_expect_success 'revert works (commit)' '
        grep "unchanged *+3/-0 file" output
 '
 
+test_expect_success 'reject multi-key input' '
+       saved=$(git hash-object -w file) &&
+       test_when_finished "git cat-file blob $saved >file" &&
+       echo an extra line >>file &&
+       test_write_lines aa | git add -p >actual &&
+       test_grep "is expected, got ${SQ}aa${SQ}" actual
+'
+
 test_expect_success 'setup expected' '
        cat >expected <<-\EOF
        EOF
@@ -526,7 +534,7 @@ test_expect_success 'split hunk setup' '
        test_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test
 '
 
-test_expect_success 'goto hunk' '
+test_expect_success 'goto hunk 1 with "g 1"' '
        test_when_finished "git reset" &&
        tr _ " " >expect <<-EOF &&
        (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? + 1:  -1,2 +1,3          +15
@@ -542,7 +550,20 @@ test_expect_success 'goto hunk' '
        test_cmp expect actual.trimmed
 '
 
-test_expect_success 'navigate to hunk via regex' '
+test_expect_success 'goto hunk 1 with "g1"' '
+       test_when_finished "git reset" &&
+       tr _ " " >expect <<-EOF &&
+       _10
+       +15
+       _20
+       (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
+       EOF
+       test_write_lines s y g1 | git add -p >actual &&
+       tail -n 4 <actual >actual.trimmed &&
+       test_cmp expect actual.trimmed
+'
+
+test_expect_success 'navigate to hunk via regex /pattern' '
        test_when_finished "git reset" &&
        tr _ " " >expect <<-EOF &&
        (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? @@ -1,2 +1,3 @@
@@ -556,6 +577,19 @@ test_expect_success 'navigate to hunk via regex' '
        test_cmp expect actual.trimmed
 '
 
+test_expect_success 'navigate to hunk via regex / pattern' '
+       test_when_finished "git reset" &&
+       tr _ " " >expect <<-EOF &&
+       _10
+       +15
+       _20
+       (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
+       EOF
+       test_write_lines s y / 1,2 | git add -p >actual &&
+       tail -n 4 <actual >actual.trimmed &&
+       test_cmp expect actual.trimmed
+'
+
 test_expect_success 'split hunk "add -p (edit)"' '
        # Split, say Edit and do nothing.  Then:
        #