Skip to content

Commit 75a009d

Browse files
phillipwoodgitster
authored andcommitted
add -p: fix editing of intent-to-add paths
A popular way of partially staging a new file is to run `git add -N <path>` and then use the hunk editing of `git add -p` to select the part of the file that the user wishes to stage. Since 85953a3 ("diff-files --raw: show correct post-image of intent-to-add files", 2020-07-01) this has stopped working as intent-to-add paths are now show as new files rather than changes to an empty blob and `git apply` refused to apply a creation patch for a path that was marked as intent-to-add. 7cfde3f ("apply: allow "new file" patches on i-t-a entries", 2020-08-06) fixed the problem with apply but it still wasn't possible to edit the added hunk properly. 2c8bd84 ("checkout -p: handle new files correctly", 2020-05-27) had previously changed `add -p` to handle new files but it did not implement patch editing correctly. The perl version simply forbade editing and the C version opened the editor with the full diff rather that just the hunk which meant that the user had to edit the hunk header manually to get it to work. The root cause of the problem is that added files store the diff header with the hunk data rather than separating the two as we do for other changes. Changing added files to store the diff header separately fixes the editing problem at the expense of having to special case empty additions as they no longer have any hunks associated with them, only the diff header. The changes move some existing code into a conditional changing the indentation, they are best viewed with --color-moved-ws=allow-indentation-change (or --ignore-space-change works well to get an overview of the changes) Signed-off-by: Phillip Wood <[email protected]> Reported-by: Thomas Sullivan <[email protected]> Reported-by: Yuchen Ying <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 3a238e5 commit 75a009d

File tree

3 files changed

+173
-94
lines changed

3 files changed

+173
-94
lines changed

add-patch.c

Lines changed: 79 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
451451
pend = p + plain->len;
452452
while (p != pend) {
453453
char *eol = memchr(p, '\n', pend - p);
454-
const char *deleted = NULL, *added = NULL, *mode_change = NULL;
454+
const char *deleted = NULL, *mode_change = NULL;
455455

456456
if (!eol)
457457
eol = pend;
@@ -468,12 +468,11 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
468468
} else if (p == plain->buf)
469469
BUG("diff starts with unexpected line:\n"
470470
"%.*s\n", (int)(eol - p), p);
471-
else if (file_diff->deleted || file_diff->added)
471+
else if (file_diff->deleted)
472472
; /* keep the rest of the file in a single "hunk" */
473473
else if (starts_with(p, "@@ ") ||
474474
(hunk == &file_diff->head &&
475-
(skip_prefix(p, "deleted file", &deleted) ||
476-
skip_prefix(p, "new file", &added)))) {
475+
(skip_prefix(p, "deleted file", &deleted)))) {
477476
if (marker == '-' || marker == '+')
478477
/*
479478
* Should not happen; previous hunk did not end
@@ -491,8 +490,6 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
491490

492491
if (deleted)
493492
file_diff->deleted = 1;
494-
else if (added)
495-
file_diff->added = 1;
496493
else if (parse_hunk_header(s, hunk) < 0)
497494
return -1;
498495

@@ -501,6 +498,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
501498
* split
502499
*/
503500
marker = *p;
501+
} else if (hunk == &file_diff->head &&
502+
starts_with(p, "new file")) {
503+
file_diff->added = 1;
504504
} else if (hunk == &file_diff->head &&
505505
skip_prefix(p, "old mode ", &mode_change) &&
506506
is_octal(mode_change, eol - mode_change)) {
@@ -1362,7 +1362,8 @@ static int patch_update_file(struct add_p_state *s,
13621362
ALLOW_EDIT = 1 << 6
13631363
} permitted = 0;
13641364

1365-
if (!file_diff->hunk_nr)
1365+
/* Empty added files have no hunks */
1366+
if (!file_diff->hunk_nr && !file_diff->added)
13661367
return 0;
13671368

13681369
strbuf_reset(&s->buf);
@@ -1371,60 +1372,66 @@ static int patch_update_file(struct add_p_state *s,
13711372
for (;;) {
13721373
if (hunk_index >= file_diff->hunk_nr)
13731374
hunk_index = 0;
1374-
hunk = file_diff->hunk + hunk_index;
1375-
1375+
hunk = file_diff->hunk_nr
1376+
? file_diff->hunk + hunk_index
1377+
: &file_diff->head;
13761378
undecided_previous = -1;
1377-
for (i = hunk_index - 1; i >= 0; i--)
1378-
if (file_diff->hunk[i].use == UNDECIDED_HUNK) {
1379-
undecided_previous = i;
1380-
break;
1381-
}
1382-
13831379
undecided_next = -1;
1384-
for (i = hunk_index + 1; i < file_diff->hunk_nr; i++)
1385-
if (file_diff->hunk[i].use == UNDECIDED_HUNK) {
1386-
undecided_next = i;
1387-
break;
1388-
}
1380+
1381+
if (file_diff->hunk_nr) {
1382+
for (i = hunk_index - 1; i >= 0; i--)
1383+
if (file_diff->hunk[i].use == UNDECIDED_HUNK) {
1384+
undecided_previous = i;
1385+
break;
1386+
}
1387+
1388+
for (i = hunk_index + 1; i < file_diff->hunk_nr; i++)
1389+
if (file_diff->hunk[i].use == UNDECIDED_HUNK) {
1390+
undecided_next = i;
1391+
break;
1392+
}
1393+
}
13891394

13901395
/* Everything decided? */
13911396
if (undecided_previous < 0 && undecided_next < 0 &&
13921397
hunk->use != UNDECIDED_HUNK)
13931398
break;
13941399

13951400
strbuf_reset(&s->buf);
1396-
render_hunk(s, hunk, 0, colored, &s->buf);
1397-
fputs(s->buf.buf, stdout);
1401+
if (file_diff->hunk_nr) {
1402+
render_hunk(s, hunk, 0, colored, &s->buf);
1403+
fputs(s->buf.buf, stdout);
13981404

1399-
strbuf_reset(&s->buf);
1400-
if (undecided_previous >= 0) {
1401-
permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK;
1402-
strbuf_addstr(&s->buf, ",k");
1403-
}
1404-
if (hunk_index) {
1405-
permitted |= ALLOW_GOTO_PREVIOUS_HUNK;
1406-
strbuf_addstr(&s->buf, ",K");
1407-
}
1408-
if (undecided_next >= 0) {
1409-
permitted |= ALLOW_GOTO_NEXT_UNDECIDED_HUNK;
1410-
strbuf_addstr(&s->buf, ",j");
1411-
}
1412-
if (hunk_index + 1 < file_diff->hunk_nr) {
1413-
permitted |= ALLOW_GOTO_NEXT_HUNK;
1414-
strbuf_addstr(&s->buf, ",J");
1415-
}
1416-
if (file_diff->hunk_nr > 1) {
1417-
permitted |= ALLOW_SEARCH_AND_GOTO;
1418-
strbuf_addstr(&s->buf, ",g,/");
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 &&
1425-
!file_diff->deleted) {
1426-
permitted |= ALLOW_EDIT;
1427-
strbuf_addstr(&s->buf, ",e");
1405+
strbuf_reset(&s->buf);
1406+
if (undecided_previous >= 0) {
1407+
permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK;
1408+
strbuf_addstr(&s->buf, ",k");
1409+
}
1410+
if (hunk_index) {
1411+
permitted |= ALLOW_GOTO_PREVIOUS_HUNK;
1412+
strbuf_addstr(&s->buf, ",K");
1413+
}
1414+
if (undecided_next >= 0) {
1415+
permitted |= ALLOW_GOTO_NEXT_UNDECIDED_HUNK;
1416+
strbuf_addstr(&s->buf, ",j");
1417+
}
1418+
if (hunk_index + 1 < file_diff->hunk_nr) {
1419+
permitted |= ALLOW_GOTO_NEXT_HUNK;
1420+
strbuf_addstr(&s->buf, ",J");
1421+
}
1422+
if (file_diff->hunk_nr > 1) {
1423+
permitted |= ALLOW_SEARCH_AND_GOTO;
1424+
strbuf_addstr(&s->buf, ",g,/");
1425+
}
1426+
if (hunk->splittable_into > 1) {
1427+
permitted |= ALLOW_SPLIT;
1428+
strbuf_addstr(&s->buf, ",s");
1429+
}
1430+
if (hunk_index + 1 > file_diff->mode_change &&
1431+
!file_diff->deleted) {
1432+
permitted |= ALLOW_EDIT;
1433+
strbuf_addstr(&s->buf, ",e");
1434+
}
14281435
}
14291436
if (file_diff->deleted)
14301437
prompt_mode_type = PROMPT_DELETION;
@@ -1438,7 +1445,9 @@ static int patch_update_file(struct add_p_state *s,
14381445
color_fprintf(stdout, s->s.prompt_color,
14391446
"(%"PRIuMAX"/%"PRIuMAX") ",
14401447
(uintmax_t)hunk_index + 1,
1441-
(uintmax_t)file_diff->hunk_nr);
1448+
(uintmax_t)(file_diff->hunk_nr
1449+
? file_diff->hunk_nr
1450+
: 1));
14421451
color_fprintf(stdout, s->s.prompt_color,
14431452
_(s->mode->prompt_mode[prompt_mode_type]),
14441453
s->buf.buf);
@@ -1458,16 +1467,24 @@ static int patch_update_file(struct add_p_state *s,
14581467
hunk->use = SKIP_HUNK;
14591468
goto soft_increment;
14601469
} else if (ch == 'a') {
1461-
for (; hunk_index < file_diff->hunk_nr; hunk_index++) {
1462-
hunk = file_diff->hunk + hunk_index;
1463-
if (hunk->use == UNDECIDED_HUNK)
1464-
hunk->use = USE_HUNK;
1470+
if (file_diff->hunk_nr) {
1471+
for (; hunk_index < file_diff->hunk_nr; hunk_index++) {
1472+
hunk = file_diff->hunk + hunk_index;
1473+
if (hunk->use == UNDECIDED_HUNK)
1474+
hunk->use = USE_HUNK;
1475+
}
1476+
} else if (hunk->use == UNDECIDED_HUNK) {
1477+
hunk->use = USE_HUNK;
14651478
}
14661479
} else if (ch == 'd' || ch == 'q') {
1467-
for (; hunk_index < file_diff->hunk_nr; hunk_index++) {
1468-
hunk = file_diff->hunk + hunk_index;
1469-
if (hunk->use == UNDECIDED_HUNK)
1470-
hunk->use = SKIP_HUNK;
1480+
if (file_diff->hunk_nr) {
1481+
for (; hunk_index < file_diff->hunk_nr; hunk_index++) {
1482+
hunk = file_diff->hunk + hunk_index;
1483+
if (hunk->use == UNDECIDED_HUNK)
1484+
hunk->use = SKIP_HUNK;
1485+
}
1486+
} else if (hunk->use == UNDECIDED_HUNK) {
1487+
hunk->use = SKIP_HUNK;
14711488
}
14721489
if (ch == 'q') {
14731490
quit = 1;
@@ -1625,7 +1642,8 @@ static int patch_update_file(struct add_p_state *s,
16251642
if (file_diff->hunk[i].use == USE_HUNK)
16261643
break;
16271644

1628-
if (i < file_diff->hunk_nr) {
1645+
if (i < file_diff->hunk_nr ||
1646+
(!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) {
16291647
/* At least one hunk selected: apply */
16301648
strbuf_reset(&s->buf);
16311649
reassemble_patch(s, file_diff, 0, &s->buf);

git-add--interactive.perl

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -754,13 +754,16 @@ sub parse_diff_header {
754754
my $head = { TEXT => [], DISPLAY => [], TYPE => 'header' };
755755
my $mode = { TEXT => [], DISPLAY => [], TYPE => 'mode' };
756756
my $deletion = { TEXT => [], DISPLAY => [], TYPE => 'deletion' };
757-
my $addition = { TEXT => [], DISPLAY => [], TYPE => 'addition' };
757+
my $addition;
758758

759759
for (my $i = 0; $i < @{$src->{TEXT}}; $i++) {
760+
if ($src->{TEXT}->[$i] =~ /^new file/) {
761+
$addition = 1;
762+
$head->{TYPE} = 'addition';
763+
}
760764
my $dest =
761765
$src->{TEXT}->[$i] =~ /^(old|new) mode (\d+)$/ ? $mode :
762766
$src->{TEXT}->[$i] =~ /^deleted file/ ? $deletion :
763-
$src->{TEXT}->[$i] =~ /^new file/ ? $addition :
764767
$head;
765768
push @{$dest->{TEXT}}, $src->{TEXT}->[$i];
766769
push @{$dest->{DISPLAY}}, $src->{DISPLAY}->[$i];
@@ -1501,12 +1504,6 @@ sub patch_update_file {
15011504
push @{$deletion->{DISPLAY}}, @{$hunk->{DISPLAY}};
15021505
}
15031506
@hunk = ($deletion);
1504-
} elsif (@{$addition->{TEXT}}) {
1505-
foreach my $hunk (@hunk) {
1506-
push @{$addition->{TEXT}}, @{$hunk->{TEXT}};
1507-
push @{$addition->{DISPLAY}}, @{$hunk->{DISPLAY}};
1508-
}
1509-
@hunk = ($addition);
15101507
}
15111508

15121509
$num = scalar @hunk;
@@ -1516,6 +1513,7 @@ sub patch_update_file {
15161513
my ($prev, $next, $other, $undecided, $i);
15171514
$other = '';
15181515

1516+
last if ($ix and !$num);
15191517
if ($num <= $ix) {
15201518
$ix = 0;
15211519
}
@@ -1548,35 +1546,51 @@ sub patch_update_file {
15481546
last;
15491547
}
15501548
}
1551-
last if (!$undecided);
1549+
last if (!$undecided && ($num || !$addition));
15521550

1553-
if ($hunk[$ix]{TYPE} eq 'hunk' &&
1554-
hunk_splittable($hunk[$ix]{TEXT})) {
1555-
$other .= ',s';
1556-
}
1557-
if ($hunk[$ix]{TYPE} eq 'hunk') {
1558-
$other .= ',e';
1559-
}
1560-
for (@{$hunk[$ix]{DISPLAY}}) {
1561-
print;
1551+
if ($num) {
1552+
if ($hunk[$ix]{TYPE} eq 'hunk' &&
1553+
hunk_splittable($hunk[$ix]{TEXT})) {
1554+
$other .= ',s';
1555+
}
1556+
if ($hunk[$ix]{TYPE} eq 'hunk') {
1557+
$other .= ',e';
1558+
}
1559+
for (@{$hunk[$ix]{DISPLAY}}) {
1560+
print;
1561+
}
15621562
}
1563-
print colored $prompt_color, "(", ($ix+1), "/$num) ",
1564-
sprintf(__($patch_update_prompt_modes{$patch_mode}{$hunk[$ix]{TYPE}}), $other);
1563+
my $type = $num ? $hunk[$ix]{TYPE} : $head->{TYPE};
1564+
print colored $prompt_color, "(", ($ix+1), "/", ($num ? $num : 1), ") ",
1565+
sprintf(__($patch_update_prompt_modes{$patch_mode}{$type}), $other);
15651566

15661567
my $line = prompt_single_character;
15671568
last unless defined $line;
15681569
if ($line) {
15691570
if ($line =~ /^y/i) {
1570-
$hunk[$ix]{USE} = 1;
1571+
if ($num) {
1572+
$hunk[$ix]{USE} = 1;
1573+
} else {
1574+
$head->{USE} = 1;
1575+
}
15711576
}
15721577
elsif ($line =~ /^n/i) {
1573-
$hunk[$ix]{USE} = 0;
1578+
if ($num) {
1579+
$hunk[$ix]{USE} = 0;
1580+
} else {
1581+
$head->{USE} = 0;
1582+
}
15741583
}
15751584
elsif ($line =~ /^a/i) {
1576-
while ($ix < $num) {
1577-
if (!defined $hunk[$ix]{USE}) {
1578-
$hunk[$ix]{USE} = 1;
1585+
if ($num) {
1586+
while ($ix < $num) {
1587+
if (!defined $hunk[$ix]{USE}) {
1588+
$hunk[$ix]{USE} = 1;
1589+
}
1590+
$ix++;
15791591
}
1592+
} else {
1593+
$head->{USE} = 1;
15801594
$ix++;
15811595
}
15821596
next;
@@ -1613,19 +1627,28 @@ sub patch_update_file {
16131627
next;
16141628
}
16151629
elsif ($line =~ /^d/i) {
1616-
while ($ix < $num) {
1617-
if (!defined $hunk[$ix]{USE}) {
1618-
$hunk[$ix]{USE} = 0;
1630+
if ($num) {
1631+
while ($ix < $num) {
1632+
if (!defined $hunk[$ix]{USE}) {
1633+
$hunk[$ix]{USE} = 0;
1634+
}
1635+
$ix++;
16191636
}
1637+
} else {
1638+
$head->{USE} = 0;
16201639
$ix++;
16211640
}
16221641
next;
16231642
}
16241643
elsif ($line =~ /^q/i) {
1625-
for ($i = 0; $i < $num; $i++) {
1626-
if (!defined $hunk[$i]{USE}) {
1627-
$hunk[$i]{USE} = 0;
1644+
if ($num) {
1645+
for ($i = 0; $i < $num; $i++) {
1646+
if (!defined $hunk[$i]{USE}) {
1647+
$hunk[$i]{USE} = 0;
1648+
}
16281649
}
1650+
} elsif (!defined $head->{USE}) {
1651+
$head->{USE} = 0;
16291652
}
16301653
$quit = 1;
16311654
last;
@@ -1743,7 +1766,7 @@ sub patch_update_file {
17431766
}
17441767
}
17451768

1746-
@hunk = coalesce_overlapping_hunks(@hunk);
1769+
@hunk = coalesce_overlapping_hunks(@hunk) if ($num);
17471770

17481771
my $n_lofs = 0;
17491772
my @result = ();
@@ -1753,7 +1776,7 @@ sub patch_update_file {
17531776
}
17541777
}
17551778

1756-
if (@result) {
1779+
if (@result or $head->{USE}) {
17571780
my @patch = reassemble_patch($head->{TEXT}, @result);
17581781
my $apply_routine = $patch_mode_flavour{APPLY};
17591782
&$apply_routine(@patch);

0 commit comments

Comments
 (0)