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 <szeder.dev@gmail.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Phillip Wood 2022-01-11 11:12:10 +00:00 committed by Junio C Hamano
parent d16632f694
commit 7008ddc645
2 changed files with 55 additions and 11 deletions

View File

@ -383,6 +383,17 @@ static int is_octal(const char *p, size_t len)
return 1; return 1;
} }
static void complete_file(char marker, struct hunk *hunk)
{
if (marker == '-' || marker == '+')
/*
* Last hunk ended in non-context line (i.e. it
* appended lines to the file, so there are no
* trailing context lines).
*/
hunk->splittable_into++;
}
static int parse_diff(struct add_p_state *s, const struct pathspec *ps) static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
{ {
struct strvec args = STRVEC_INIT; struct strvec args = STRVEC_INIT;
@ -472,6 +483,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
eol = pend; eol = pend;
if (starts_with(p, "diff ")) { if (starts_with(p, "diff ")) {
complete_file(marker, hunk);
ALLOC_GROW_BY(s->file_diff, s->file_diff_nr, 1, ALLOC_GROW_BY(s->file_diff, s->file_diff_nr, 1,
file_diff_alloc); file_diff_alloc);
file_diff = s->file_diff + s->file_diff_nr - 1; 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)
file_diff->hunk->colored_end = hunk->colored_end; file_diff->hunk->colored_end = hunk->colored_end;
} }
} }
complete_file(marker, hunk);
if (marker == '-' || marker == '+')
/*
* Last hunk ended in non-context line (i.e. it appended lines
* to the file, so there are no trailing context lines).
*/
hunk->splittable_into++;
/* non-colored shorter than colored? */ /* non-colored shorter than colored? */
if (colored_p != colored_pend) { if (colored_p != colored_pend) {

View File

@ -326,7 +326,9 @@ test_expect_success 'correct message when there is nothing to do' '
test_expect_success 'setup again' ' test_expect_success 'setup again' '
git reset --hard && git reset --hard &&
test_chmod +x file && test_chmod +x file &&
echo content >>file echo content >>file &&
test_write_lines A B C D>file2 &&
git add file2
' '
# Write the patch file with a new line at the top and bottom # Write the patch file with a new line at the top and bottom
@ -341,13 +343,27 @@ test_expect_success 'setup patch' '
content content
+lastline +lastline
\ No newline at end of file \ No newline at end of file
diff --git a/file2 b/file2
index 8422d40..35b930a 100644
--- a/file2
+++ b/file2
@@ -1,4 +1,5 @@
-A
+Z
B
+Y
C
-D
+X
EOF EOF
' '
# Expected output, diff is similar to the patch but w/ diff at the top # Expected output, diff is similar to the patch but w/ diff at the top
test_expect_success 'setup expected' ' test_expect_success 'setup expected' '
echo diff --git a/file b/file >expected && echo diff --git a/file b/file >expected &&
sed "/^index/s/ 100644/ 100755/" patch >>expected && sed -e "/^index 180b47c/s/ 100644/ 100755/" \
-e /1,5/s//1,4/ \
-e /Y/d patch >>expected &&
cat >expected-output <<-\EOF cat >expected-output <<-\EOF
--- a/file --- a/file
+++ b/file +++ b/file
@ -366,6 +382,28 @@ test_expect_success 'setup expected' '
content content
+lastline +lastline
\ No newline at end of file \ No newline at end of file
--- a/file2
+++ b/file2
@@ -1,4 +1,5 @@
-A
+Z
B
+Y
C
-D
+X
@@ -1,2 +1,2 @@
-A
+Z
B
@@ -2,2 +2,3 @@
B
+Y
C
@@ -3,2 +4,2 @@
C
-D
+X
EOF EOF
' '
@ -373,8 +411,8 @@ test_expect_success 'setup expected' '
test_expect_success 'add first line works' ' test_expect_success 'add first line works' '
git commit -am "clear local changes" && git commit -am "clear local changes" &&
git apply patch && git apply patch &&
test_write_lines s y y | git add -p file 2>error >raw-output && test_write_lines s y y s y n y | git add -p 2>error >raw-output &&
sed -n -e "s/^([1-2]\/[1-2]) Stage this hunk[^@]*\(@@ .*\)/\1/" \ sed -n -e "s/^([1-9]\/[1-9]) Stage this hunk[^@]*\(@@ .*\)/\1/" \
-e "/^[-+@ \\\\]"/p raw-output >output && -e "/^[-+@ \\\\]"/p raw-output >output &&
test_must_be_empty error && test_must_be_empty error &&
git diff --cached >diff && git diff --cached >diff &&