Skip to content

Commit 7008ddc

Browse files
phillipwoodgitster
authored andcommitted
builtin add -p: fix hunk splitting
The C reimplementation of "add -p" fails to split the last hunk in a file if hunk ends with an addition or deletion without any post context line unless it is the last file to be processed. To determine whether a hunk can be split a counter is incremented each time a context line follows an insertion or deletion. If at the end of the hunk the value of this counter is greater than one then the hunk can be split into that number of smaller hunks. If the last hunk in a file ends with an insertion or deletion then there is no following context line and the counter will not be incremented. This case is already handled at the end of the loop where counter is incremented if the last hunk ended with an insertion or deletion. Unfortunately there is no similar check between files (likely because the perl version only ever parses one diff at a time). Fix this by checking if the last hunk ended with an insertion or deletion when we see the diff header of a new file and extend the existing regression test. Reproted-by: SZEDER Gábor <[email protected]> Signed-off-by: Phillip Wood <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d16632f commit 7008ddc

File tree

2 files changed

+55
-11
lines changed

2 files changed

+55
-11
lines changed

add-patch.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,17 @@ static int is_octal(const char *p, size_t len)
383383
return 1;
384384
}
385385

386+
static void complete_file(char marker, struct hunk *hunk)
387+
{
388+
if (marker == '-' || marker == '+')
389+
/*
390+
* Last hunk ended in non-context line (i.e. it
391+
* appended lines to the file, so there are no
392+
* trailing context lines).
393+
*/
394+
hunk->splittable_into++;
395+
}
396+
386397
static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
387398
{
388399
struct strvec args = STRVEC_INIT;
@@ -472,6 +483,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
472483
eol = pend;
473484

474485
if (starts_with(p, "diff ")) {
486+
complete_file(marker, hunk);
475487
ALLOC_GROW_BY(s->file_diff, s->file_diff_nr, 1,
476488
file_diff_alloc);
477489
file_diff = s->file_diff + s->file_diff_nr - 1;
@@ -598,13 +610,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
598610
file_diff->hunk->colored_end = hunk->colored_end;
599611
}
600612
}
601-
602-
if (marker == '-' || marker == '+')
603-
/*
604-
* Last hunk ended in non-context line (i.e. it appended lines
605-
* to the file, so there are no trailing context lines).
606-
*/
607-
hunk->splittable_into++;
613+
complete_file(marker, hunk);
608614

609615
/* non-colored shorter than colored? */
610616
if (colored_p != colored_pend) {

t/t3701-add-interactive.sh

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,9 @@ test_expect_success 'correct message when there is nothing to do' '
326326
test_expect_success 'setup again' '
327327
git reset --hard &&
328328
test_chmod +x file &&
329-
echo content >>file
329+
echo content >>file &&
330+
test_write_lines A B C D>file2 &&
331+
git add file2
330332
'
331333

332334
# Write the patch file with a new line at the top and bottom
@@ -341,13 +343,27 @@ test_expect_success 'setup patch' '
341343
content
342344
+lastline
343345
\ No newline at end of file
346+
diff --git a/file2 b/file2
347+
index 8422d40..35b930a 100644
348+
--- a/file2
349+
+++ b/file2
350+
@@ -1,4 +1,5 @@
351+
-A
352+
+Z
353+
B
354+
+Y
355+
C
356+
-D
357+
+X
344358
EOF
345359
'
346360

347361
# Expected output, diff is similar to the patch but w/ diff at the top
348362
test_expect_success 'setup expected' '
349363
echo diff --git a/file b/file >expected &&
350-
sed "/^index/s/ 100644/ 100755/" patch >>expected &&
364+
sed -e "/^index 180b47c/s/ 100644/ 100755/" \
365+
-e /1,5/s//1,4/ \
366+
-e /Y/d patch >>expected &&
351367
cat >expected-output <<-\EOF
352368
--- a/file
353369
+++ b/file
@@ -366,15 +382,37 @@ test_expect_success 'setup expected' '
366382
content
367383
+lastline
368384
\ No newline at end of file
385+
--- a/file2
386+
+++ b/file2
387+
@@ -1,4 +1,5 @@
388+
-A
389+
+Z
390+
B
391+
+Y
392+
C
393+
-D
394+
+X
395+
@@ -1,2 +1,2 @@
396+
-A
397+
+Z
398+
B
399+
@@ -2,2 +2,3 @@
400+
B
401+
+Y
402+
C
403+
@@ -3,2 +4,2 @@
404+
C
405+
-D
406+
+X
369407
EOF
370408
'
371409

372410
# Test splitting the first patch, then adding both
373411
test_expect_success 'add first line works' '
374412
git commit -am "clear local changes" &&
375413
git apply patch &&
376-
test_write_lines s y y | git add -p file 2>error >raw-output &&
377-
sed -n -e "s/^([1-2]\/[1-2]) Stage this hunk[^@]*\(@@ .*\)/\1/" \
414+
test_write_lines s y y s y n y | git add -p 2>error >raw-output &&
415+
sed -n -e "s/^([1-9]\/[1-9]) Stage this hunk[^@]*\(@@ .*\)/\1/" \
378416
-e "/^[-+@ \\\\]"/p raw-output >output &&
379417
test_must_be_empty error &&
380418
git diff --cached >diff &&

0 commit comments

Comments
 (0)