add -p: fix checking of user input

When a file has been deleted the C version of add -p allows the user
to edit a hunk even though 'e' is not in the list of allowed
responses. (I think 'e' is disallowed because if the file is edited it
is no longer a deletion and we're not set up to rewrite the diff
header).

The invalid response was allowed because the test that determines
whether to display 'e' was not duplicated correctly in the code that
processes the user's choice. Fix this by using flags that are set when
constructing the prompt and checked when processing the user's choice
rather than repeating the check itself.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/add-patch.c b/add-patch.c
index a15fa407bef03ac6e75e2a0f4eeca3f2eccdb211..907c05b3c1b1e46953ab34c2b737c4a6f1f9fbda 100644 (file)
--- a/add-patch.c
+++ b/add-patch.c
@@ -1352,6 +1352,15 @@ static int patch_update_file(struct add_p_state *s,
        struct child_process cp = CHILD_PROCESS_INIT;
        int colored = !!s->colored.len, quit = 0;
        enum prompt_mode_type prompt_mode_type;
+       enum {
+               ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0,
+               ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 << 1,
+               ALLOW_GOTO_NEXT_HUNK = 1 << 2,
+               ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3,
+               ALLOW_SEARCH_AND_GOTO = 1 << 4,
+               ALLOW_SPLIT = 1 << 5,
+               ALLOW_EDIT = 1 << 6
+       } permitted = 0;
 
        if (!file_diff->hunk_nr)
                return 0;
@@ -1388,22 +1397,35 @@ static int patch_update_file(struct add_p_state *s,
                fputs(s->buf.buf, stdout);
 
                strbuf_reset(&s->buf);
-               if (undecided_previous >= 0)
+               if (undecided_previous >= 0) {
+                       permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK;
                        strbuf_addstr(&s->buf, ",k");
-               if (hunk_index)
+               }
+               if (hunk_index) {
+                       permitted |= ALLOW_GOTO_PREVIOUS_HUNK;
                        strbuf_addstr(&s->buf, ",K");
-               if (undecided_next >= 0)
+               }
+               if (undecided_next >= 0) {
+                       permitted |= ALLOW_GOTO_NEXT_UNDECIDED_HUNK;
                        strbuf_addstr(&s->buf, ",j");
-               if (hunk_index + 1 < file_diff->hunk_nr)
+               }
+               if (hunk_index + 1 < file_diff->hunk_nr) {
+                       permitted |= ALLOW_GOTO_NEXT_HUNK;
                        strbuf_addstr(&s->buf, ",J");
-               if (file_diff->hunk_nr > 1)
+               }
+               if (file_diff->hunk_nr > 1) {
+                       permitted |= ALLOW_SEARCH_AND_GOTO;
                        strbuf_addstr(&s->buf, ",g,/");
-               if (hunk->splittable_into > 1)
+               }
+               if (hunk->splittable_into > 1) {
+                       permitted |= ALLOW_SPLIT;
                        strbuf_addstr(&s->buf, ",s");
+               }
                if (hunk_index + 1 > file_diff->mode_change &&
-                   !file_diff->deleted)
+                   !file_diff->deleted) {
+                       permitted |= ALLOW_EDIT;
                        strbuf_addstr(&s->buf, ",e");
-
+               }
                if (file_diff->deleted)
                        prompt_mode_type = PROMPT_DELETION;
                else if (file_diff->added)
@@ -1452,22 +1474,22 @@ soft_increment:
                                break;
                        }
                } else if (s->answer.buf[0] == 'K') {
-                       if (hunk_index)
+                       if (permitted & ALLOW_GOTO_PREVIOUS_HUNK)
                                hunk_index--;
                        else
                                err(s, _("No previous hunk"));
                } else if (s->answer.buf[0] == 'J') {
-                       if (hunk_index + 1 < file_diff->hunk_nr)
+                       if (permitted & ALLOW_GOTO_NEXT_HUNK)
                                hunk_index++;
                        else
                                err(s, _("No next hunk"));
                } else if (s->answer.buf[0] == 'k') {
-                       if (undecided_previous >= 0)
+                       if (permitted & ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK)
                                hunk_index = undecided_previous;
                        else
                                err(s, _("No previous hunk"));
                } else if (s->answer.buf[0] == 'j') {
-                       if (undecided_next >= 0)
+                       if (permitted & ALLOW_GOTO_NEXT_UNDECIDED_HUNK)
                                hunk_index = undecided_next;
                        else
                                err(s, _("No next hunk"));
@@ -1475,7 +1497,7 @@ soft_increment:
                        char *pend;
                        unsigned long response;
 
-                       if (file_diff->hunk_nr < 2) {
+                       if (!(permitted & ALLOW_SEARCH_AND_GOTO)) {
                                err(s, _("No other hunks to goto"));
                                continue;
                        }
@@ -1512,7 +1534,7 @@ soft_increment:
                        regex_t regex;
                        int ret;
 
-                       if (file_diff->hunk_nr < 2) {
+                       if (!(permitted & ALLOW_SEARCH_AND_GOTO)) {
                                err(s, _("No other hunks to search"));
                                continue;
                        }
@@ -1557,7 +1579,7 @@ soft_increment:
                        hunk_index = i;
                } else if (s->answer.buf[0] == 's') {
                        size_t splittable_into = hunk->splittable_into;
-                       if (splittable_into < 2)
+                       if (!(permitted & ALLOW_SPLIT))
                                err(s, _("Sorry, cannot split this hunk"));
                        else if (!split_hunk(s, file_diff,
                                             hunk - file_diff->hunk))
@@ -1565,7 +1587,7 @@ soft_increment:
                                                 _("Split into %d hunks."),
                                                 (int)splittable_into);
                } else if (s->answer.buf[0] == 'e') {
-                       if (hunk_index + 1 == file_diff->mode_change)
+                       if (!(permitted & ALLOW_EDIT))
                                err(s, _("Sorry, cannot edit this hunk"));
                        else if (edit_hunk_loop(s, file_diff, hunk) >= 0) {
                                hunk->use = USE_HUNK;