Skip to content

Commit cd6c082

Browse files
gitstergitster
committed
Merge branch 'rs/add-patch-options-fix'
The code in "git add -p" and friends to iterate over hunks was riddled with bugs, which has been corrected. * rs/add-patch-options-fix: add-patch: reset "permitted" at loop start add-patch: let options a and d roll over like y and n add-patch: let options k and K roll over like j and J add-patch: let options y, n, j, and e roll over to next undecided add-patch: document that option J rolls over add-patch: improve help for options j, J, k, and K
2 parents 282a968 + 208e23e commit cd6c082

File tree

3 files changed

+118
-41
lines changed

3 files changed

+118
-41
lines changed

‎Documentation/git-add.adoc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -342,10 +342,10 @@ patch::
342342
d - do not stage this hunk or any of the later hunks in the file
343343
g - select a hunk to go to
344344
/ - search for a hunk matching the given regex
345-
j - leave this hunk undecided, see next undecided hunk
346-
J - leave this hunk undecided, see next hunk
347-
k - leave this hunk undecided, see previous undecided hunk
348-
K - leave this hunk undecided, see previous hunk
345+
j - go to the next undecided hunk, roll over at the bottom
346+
J - go to the next hunk, roll over at the bottom
347+
k - go to the previous undecided hunk, roll over at the top
348+
K - go to the previous hunk, roll over at the top
349349
s - split the current hunk into smaller hunks
350350
e - manually edit the current hunk
351351
p - print the current hunk

‎add-patch.c‎

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,17 +1408,38 @@ static size_t display_hunks(struct add_p_state *s,
14081408
}
14091409

14101410
static const char help_patch_remainder[] =
1411-
N_("j - leave this hunk undecided, see next undecided hunk\n"
1412-
"J - leave this hunk undecided, see next hunk\n"
1413-
"k - leave this hunk undecided, see previous undecided hunk\n"
1414-
"K - leave this hunk undecided, see previous hunk\n"
1411+
N_("j - go to the next undecided hunk, roll over at the bottom\n"
1412+
"J - go to the next hunk, roll over at the bottom\n"
1413+
"k - go to the previous undecided hunk, roll over at the top\n"
1414+
"K - go to the previous hunk, roll over at the top\n"
14151415
"g - select a hunk to go to\n"
14161416
"/ - search for a hunk matching the given regex\n"
14171417
"s - split the current hunk into smaller hunks\n"
14181418
"e - manually edit the current hunk\n"
14191419
"p - print the current hunk, 'P' to use the pager\n"
14201420
"? - print help\n");
14211421

1422+
static size_t dec_mod(size_t a, size_t m)
1423+
{
1424+
return a > 0 ? a - 1 : m - 1;
1425+
}
1426+
1427+
static size_t inc_mod(size_t a, size_t m)
1428+
{
1429+
return a < m - 1 ? a + 1 : 0;
1430+
}
1431+
1432+
static bool get_first_undecided(const struct file_diff *file_diff, size_t *idx)
1433+
{
1434+
for (size_t i = 0; i < file_diff->hunk_nr; i++) {
1435+
if (file_diff->hunk[i].use == UNDECIDED_HUNK) {
1436+
*idx = i;
1437+
return true;
1438+
}
1439+
}
1440+
return false;
1441+
}
1442+
14221443
static int patch_update_file(struct add_p_state *s,
14231444
struct file_diff *file_diff)
14241445
{
@@ -1429,15 +1450,6 @@ static int patch_update_file(struct add_p_state *s,
14291450
struct child_process cp = CHILD_PROCESS_INIT;
14301451
int colored = !!s->colored.len, quit = 0, use_pager = 0;
14311452
enum prompt_mode_type prompt_mode_type;
1432-
enum {
1433-
ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0,
1434-
ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 << 1,
1435-
ALLOW_GOTO_NEXT_HUNK = 1 << 2,
1436-
ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3,
1437-
ALLOW_SEARCH_AND_GOTO = 1 << 4,
1438-
ALLOW_SPLIT = 1 << 5,
1439-
ALLOW_EDIT = 1 << 6
1440-
} permitted = 0;
14411453

14421454
/* Empty added files have no hunks */
14431455
if (!file_diff->hunk_nr && !file_diff->added)
@@ -1447,6 +1459,16 @@ static int patch_update_file(struct add_p_state *s,
14471459
render_diff_header(s, file_diff, colored, &s->buf);
14481460
fputs(s->buf.buf, stdout);
14491461
for (;;) {
1462+
enum {
1463+
ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0,
1464+
ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 << 1,
1465+
ALLOW_GOTO_NEXT_HUNK = 1 << 2,
1466+
ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3,
1467+
ALLOW_SEARCH_AND_GOTO = 1 << 4,
1468+
ALLOW_SPLIT = 1 << 5,
1469+
ALLOW_EDIT = 1 << 6
1470+
} permitted = 0;
1471+
14501472
if (hunk_index >= file_diff->hunk_nr)
14511473
hunk_index = 0;
14521474
hunk = file_diff->hunk_nr
@@ -1456,13 +1478,17 @@ static int patch_update_file(struct add_p_state *s,
14561478
undecided_next = -1;
14571479

14581480
if (file_diff->hunk_nr) {
1459-
for (i = hunk_index - 1; i >= 0; i--)
1481+
for (i = dec_mod(hunk_index, file_diff->hunk_nr);
1482+
i != hunk_index;
1483+
i = dec_mod(i, file_diff->hunk_nr))
14601484
if (file_diff->hunk[i].use == UNDECIDED_HUNK) {
14611485
undecided_previous = i;
14621486
break;
14631487
}
14641488

1465-
for (i = hunk_index + 1; i < file_diff->hunk_nr; i++)
1489+
for (i = inc_mod(hunk_index, file_diff->hunk_nr);
1490+
i != hunk_index;
1491+
i = inc_mod(i, file_diff->hunk_nr))
14661492
if (file_diff->hunk[i].use == UNDECIDED_HUNK) {
14671493
undecided_next = i;
14681494
break;
@@ -1496,15 +1522,15 @@ static int patch_update_file(struct add_p_state *s,
14961522
permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK;
14971523
strbuf_addstr(&s->buf, ",k");
14981524
}
1499-
if (hunk_index) {
1525+
if (file_diff->hunk_nr > 1) {
15001526
permitted |= ALLOW_GOTO_PREVIOUS_HUNK;
15011527
strbuf_addstr(&s->buf, ",K");
15021528
}
15031529
if (undecided_next >= 0) {
15041530
permitted |= ALLOW_GOTO_NEXT_UNDECIDED_HUNK;
15051531
strbuf_addstr(&s->buf, ",j");
15061532
}
1507-
if (hunk_index + 1 < file_diff->hunk_nr) {
1533+
if (file_diff->hunk_nr > 1) {
15081534
permitted |= ALLOW_GOTO_NEXT_HUNK;
15091535
strbuf_addstr(&s->buf, ",J");
15101536
}
@@ -1569,6 +1595,8 @@ static int patch_update_file(struct add_p_state *s,
15691595
if (hunk->use == UNDECIDED_HUNK)
15701596
hunk->use = USE_HUNK;
15711597
}
1598+
if (!get_first_undecided(file_diff, &hunk_index))
1599+
hunk_index = 0;
15721600
} else if (hunk->use == UNDECIDED_HUNK) {
15731601
hunk->use = USE_HUNK;
15741602
}
@@ -1579,6 +1607,8 @@ static int patch_update_file(struct add_p_state *s,
15791607
if (hunk->use == UNDECIDED_HUNK)
15801608
hunk->use = SKIP_HUNK;
15811609
}
1610+
if (!get_first_undecided(file_diff, &hunk_index))
1611+
hunk_index = 0;
15821612
} else if (hunk->use == UNDECIDED_HUNK) {
15831613
hunk->use = SKIP_HUNK;
15841614
}
@@ -1588,24 +1618,25 @@ static int patch_update_file(struct add_p_state *s,
15881618
}
15891619
} else if (s->answer.buf[0] == 'K') {
15901620
if (permitted & ALLOW_GOTO_PREVIOUS_HUNK)
1591-
hunk_index--;
1621+
hunk_index = dec_mod(hunk_index,
1622+
file_diff->hunk_nr);
15921623
else
1593-
err(s, _("No previous hunk"));
1624+
err(s, _("No other hunk"));
15941625
} else if (s->answer.buf[0] == 'J') {
15951626
if (permitted & ALLOW_GOTO_NEXT_HUNK)
15961627
hunk_index++;
15971628
else
1598-
err(s, _("No next hunk"));
1629+
err(s, _("No other hunk"));
15991630
} else if (s->answer.buf[0] == 'k') {
16001631
if (permitted & ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK)
16011632
hunk_index = undecided_previous;
16021633
else
1603-
err(s, _("No previous hunk"));
1634+
err(s, _("No other undecided hunk"));
16041635
} else if (s->answer.buf[0] == 'j') {
16051636
if (permitted & ALLOW_GOTO_NEXT_UNDECIDED_HUNK)
16061637
hunk_index = undecided_next;
16071638
else
1608-
err(s, _("No next hunk"));
1639+
err(s, _("No other undecided hunk"));
16091640
} else if (s->answer.buf[0] == 'g') {
16101641
char *pend;
16111642
unsigned long response;

‎t/t3701-add-interactive.sh‎

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,8 @@ test_expect_success 'different prompts for mode change/deleted' '
333333
sed -n "s/^\(([0-9/]*) Stage .*?\).*/\1/p" actual >actual.filtered &&
334334
cat >expect <<-\EOF &&
335335
(1/1) Stage deletion [y,n,q,a,d,p,?]?
336-
(1/2) Stage mode change [y,n,q,a,d,j,J,g,/,p,?]?
337-
(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]?
336+
(1/2) Stage mode change [y,n,q,a,d,k,K,j,J,g,/,p,?]?
337+
(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]?
338338
EOF
339339
test_cmp expect actual.filtered
340340
'
@@ -521,13 +521,13 @@ test_expect_success 'split hunk setup' '
521521
test_expect_success 'goto hunk 1 with "g 1"' '
522522
test_when_finished "git reset" &&
523523
tr _ " " >expect <<-EOF &&
524-
(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? + 1: -1,2 +1,3 +15
524+
(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? + 1: -1,2 +1,3 +15
525525
_ 2: -2,4 +3,8 +21
526526
go to which hunk? @@ -1,2 +1,3 @@
527527
_10
528528
+15
529529
_20
530-
(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
530+
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_
531531
EOF
532532
test_write_lines s y g 1 | git add -p >actual &&
533533
tail -n 7 <actual >actual.trimmed &&
@@ -540,7 +540,7 @@ test_expect_success 'goto hunk 1 with "g1"' '
540540
_10
541541
+15
542542
_20
543-
(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
543+
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_
544544
EOF
545545
test_write_lines s y g1 | git add -p >actual &&
546546
tail -n 4 <actual >actual.trimmed &&
@@ -550,11 +550,11 @@ test_expect_success 'goto hunk 1 with "g1"' '
550550
test_expect_success 'navigate to hunk via regex /pattern' '
551551
test_when_finished "git reset" &&
552552
tr _ " " >expect <<-EOF &&
553-
(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? @@ -1,2 +1,3 @@
553+
(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? @@ -1,2 +1,3 @@
554554
_10
555555
+15
556556
_20
557-
(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
557+
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_
558558
EOF
559559
test_write_lines s y /1,2 | git add -p >actual &&
560560
tail -n 5 <actual >actual.trimmed &&
@@ -567,7 +567,7 @@ test_expect_success 'navigate to hunk via regex / pattern' '
567567
_10
568568
+15
569569
_20
570-
(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
570+
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_
571571
EOF
572572
test_write_lines s y / 1,2 | git add -p >actual &&
573573
tail -n 4 <actual >actual.trimmed &&
@@ -579,11 +579,11 @@ test_expect_success 'print again the hunk' '
579579
tr _ " " >expect <<-EOF &&
580580
+15
581581
20
582-
(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? @@ -1,2 +1,3 @@
582+
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? @@ -1,2 +1,3 @@
583583
10
584584
+15
585585
20
586-
(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
586+
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_
587587
EOF
588588
test_write_lines s y g 1 p | git add -p >actual &&
589589
tail -n 7 <actual >actual.trimmed &&
@@ -595,11 +595,11 @@ test_expect_success TTY 'print again the hunk (PAGER)' '
595595
cat >expect <<-EOF &&
596596
<GREEN>+<RESET><GREEN>15<RESET>
597597
20<RESET>
598-
<BOLD;BLUE>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET>PAGER <CYAN>@@ -1,2 +1,3 @@<RESET>
598+
<BOLD;BLUE>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? <RESET>PAGER <CYAN>@@ -1,2 +1,3 @@<RESET>
599599
PAGER 10<RESET>
600600
PAGER <GREEN>+<RESET><GREEN>15<RESET>
601601
PAGER 20<RESET>
602-
<BOLD;BLUE>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET>
602+
<BOLD;BLUE>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? <RESET>
603603
EOF
604604
test_write_lines s y g 1 P |
605605
(
@@ -802,15 +802,15 @@ test_expect_success 'colors can be overridden' '
802802
<BOLD>-old<RESET>
803803
<BLUE>+<RESET><BLUE>new<RESET>
804804
<CYAN> more-context<RESET>
805-
<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
805+
<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
806806
<CYAN> more-context<RESET>
807807
<BLUE>+<RESET><BLUE>another-one<RESET>
808-
<YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
808+
<YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
809809
<CYAN> context<RESET>
810810
<BOLD>-old<RESET>
811811
<BLUE>+new<RESET>
812812
<CYAN> more-context<RESET>
813-
<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET>
813+
<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? <RESET>
814814
EOF
815815
test_cmp expect actual
816816
'
@@ -1385,4 +1385,50 @@ test_expect_success 'splitting edited hunk' '
13851385
test_cmp expect actual
13861386
'
13871387

1388+
test_expect_success 'options J, K roll over' '
1389+
test_write_lines a b c d e f g h i >file &&
1390+
git add file &&
1391+
test_write_lines X b c d e f g h X >file &&
1392+
test_write_lines J J K q | git add -p >out &&
1393+
test_write_lines 1 2 1 2 >expect &&
1394+
sed -n -e "s-/.*--" -e "s/^(//p" <out >actual &&
1395+
test_cmp expect actual
1396+
'
1397+
1398+
test_expect_success 'options y, n, a, d, j, k, e roll over to next undecided (1)' '
1399+
test_write_lines a b c d e f g h i j k l m n o p q >file &&
1400+
git add file &&
1401+
test_write_lines X b c d e f g h X j k l m n o p X >file &&
1402+
test_set_editor : &&
1403+
test_write_lines g3 y g3 n g3 a g3 d g3 j g3 e k q | git add -p >out &&
1404+
test_write_lines 1 3 1 3 1 3 1 3 1 3 1 3 1 2 >expect &&
1405+
sed -n -e "s-/.*--" -e "s/^(//p" <out >actual &&
1406+
test_cmp expect actual
1407+
'
1408+
1409+
test_expect_success 'options y, n, a, d, j, k, e roll over to next undecided (2)' '
1410+
test_write_lines a b c d e f g h i j k l m n o p q >file &&
1411+
git add file &&
1412+
test_write_lines X b c d e f g h X j k l m n o p X >file &&
1413+
test_set_editor : &&
1414+
test_write_lines y g3 y g3 n g3 a g3 d g3 j g3 e g1 k q | git add -p >out &&
1415+
test_write_lines 1 2 3 2 3 2 3 2 3 2 3 2 3 2 1 2 >expect &&
1416+
sed -n -e "s-/.*--" -e "s/^(//p" <out >actual &&
1417+
test_cmp expect actual
1418+
'
1419+
1420+
test_expect_success 'invalid option s is rejected' '
1421+
test_write_lines a b c d e f g h i j k >file &&
1422+
git add file &&
1423+
test_write_lines X b X d e f g h i j X >file &&
1424+
test_write_lines j s q | git add -p >out &&
1425+
sed -ne "s/ @@.*//" -e "s/ \$//" -e "/^(/p" <out >actual &&
1426+
cat >expect <<-EOF &&
1427+
(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,p,?]?
1428+
(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? Sorry, cannot split this hunk
1429+
(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?
1430+
EOF
1431+
test_cmp expect actual
1432+
'
1433+
13881434
test_done

0 commit comments

Comments
 (0)