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>
Phillip Wood committed
Aug 17, 2020 at 13:23 UTC
ce910287e727e5b534d806df725c68a8dfc4790a
1 file changed
+38
-16
add-patch.c
+38
-16
@@ -1352,6 +1352,15 @@ static int patch_update_file(struct add_p_state *s,
1352
struct child_process cp = CHILD_PROCESS_INIT;
1353
int colored = !!s->colored.len, quit = 0;
1354
enum prompt_mode_type prompt_mode_type;
1355
+ enum {
1356
+ ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0,
1357
+ ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 << 1,
1358
+ ALLOW_GOTO_NEXT_HUNK = 1 << 2,
1359
+ ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3,
1360
+ ALLOW_SEARCH_AND_GOTO = 1 << 4,
1361
+ ALLOW_SPLIT = 1 << 5,
1362
+ ALLOW_EDIT = 1 << 6
1363
+ } permitted = 0;
1364
1365
if (!file_diff->hunk_nr)
1366
return 0;
@@ -1388,22 +1397,35 @@ static int patch_update_file(struct add_p_state *s,
1397
fputs(s->buf.buf, stdout);
1398
1399
strbuf_reset(&s->buf);
1391
- if (undecided_previous >= 0)
1400
+ if (undecided_previous >= 0) {
1401
+ permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK;
1402
strbuf_addstr(&s->buf, ",k");
1393
- if (hunk_index)
1403
+ }
1404
+ if (hunk_index) {
1405
+ permitted |= ALLOW_GOTO_PREVIOUS_HUNK;
1406
strbuf_addstr(&s->buf, ",K");
1395
- if (undecided_next >= 0)
1407
+ }
1408
+ if (undecided_next >= 0) {
1409
+ permitted |= ALLOW_GOTO_NEXT_UNDECIDED_HUNK;
1410
strbuf_addstr(&s->buf, ",j");
1397
- if (hunk_index + 1 < file_diff->hunk_nr)
1411
+ }
1412
+ if (hunk_index + 1 < file_diff->hunk_nr) {
1413
+ permitted |= ALLOW_GOTO_NEXT_HUNK;
1414
strbuf_addstr(&s->buf, ",J");
1399
- if (file_diff->hunk_nr > 1)
1415
+ }
1416
+ if (file_diff->hunk_nr > 1) {
1417
+ permitted |= ALLOW_SEARCH_AND_GOTO;
1418
strbuf_addstr(&s->buf, ",g,/");
1401
- if (hunk->splittable_into > 1)
1419
+ }
1420
+ if (hunk->splittable_into > 1) {
1421
+ permitted |= ALLOW_SPLIT;
1422
strbuf_addstr(&s->buf, ",s");
1423
+ }
1424
if (hunk_index + 1 > file_diff->mode_change &&
1404
- !file_diff->deleted)
1425
+ !file_diff->deleted) {
1426
+ permitted |= ALLOW_EDIT;
1427
strbuf_addstr(&s->buf, ",e");
1406
-
1428
+ }
1429
if (file_diff->deleted)
1430
prompt_mode_type = PROMPT_DELETION;
1431
else if (file_diff->added)
@@ -1452,22 +1474,22 @@ soft_increment:
1474
break;
1475
}
1476
} else if (s->answer.buf[0] == 'K') {
1455
- if (hunk_index)
1477
+ if (permitted & ALLOW_GOTO_PREVIOUS_HUNK)
1478
hunk_index--;
1479
else
1480
err(s, _("No previous hunk"));
1481
} else if (s->answer.buf[0] == 'J') {
1460
- if (hunk_index + 1 < file_diff->hunk_nr)
1482
+ if (permitted & ALLOW_GOTO_NEXT_HUNK)
1483
hunk_index++;
1484
else
1485
err(s, _("No next hunk"));
1486
} else if (s->answer.buf[0] == 'k') {
1465
- if (undecided_previous >= 0)
1487
+ if (permitted & ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK)
1488
hunk_index = undecided_previous;
1489
else
1490
err(s, _("No previous hunk"));
1491
} else if (s->answer.buf[0] == 'j') {
1470
- if (undecided_next >= 0)
1492
+ if (permitted & ALLOW_GOTO_NEXT_UNDECIDED_HUNK)
1493
hunk_index = undecided_next;
1494
else
1495
err(s, _("No next hunk"));
@@ -1475,7 +1497,7 @@ soft_increment:
1497
char *pend;
1498
unsigned long response;
1499
1478
- if (file_diff->hunk_nr < 2) {
1500
+ if (!(permitted & ALLOW_SEARCH_AND_GOTO)) {
1501
err(s, _("No other hunks to goto"));
1502
continue;
1503
}
@@ -1512,7 +1534,7 @@ soft_increment:
1534
regex_t regex;
1535
int ret;
1536
1515
- if (file_diff->hunk_nr < 2) {
1537
+ if (!(permitted & ALLOW_SEARCH_AND_GOTO)) {
1538
err(s, _("No other hunks to search"));
1539
continue;
1540
}
@@ -1557,7 +1579,7 @@ soft_increment:
1579
hunk_index = i;
1580
} else if (s->answer.buf[0] == 's') {
1581
size_t splittable_into = hunk->splittable_into;
1560
- if (splittable_into < 2)
1582
+ if (!(permitted & ALLOW_SPLIT))
1583
err(s, _("Sorry, cannot split this hunk"));
1584
else if (!split_hunk(s, file_diff,
1585
hunk - file_diff->hunk))
@@ -1565,7 +1587,7 @@ soft_increment:
1587
_("Split into %d hunks."),
1588
(int)splittable_into);
1589
} else if (s->answer.buf[0] == 'e') {
1568
- if (hunk_index + 1 == file_diff->mode_change)
1590
+ if (!(permitted & ALLOW_EDIT))
1591
err(s, _("Sorry, cannot edit this hunk"));
1592
else if (edit_hunk_loop(s, file_diff, hunk) >= 0) {
1593
hunk->use = USE_HUNK;