git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/2] builtin add -p: fix hunk splitting

From
PGPhillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Date
Dec 20, 2021, 14:32 UTC
Message-ID
<5d5639c2b0474680850b7adbb7c5ec81d124eb50.1640010777.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1100.git.1640010777.gitgitgadget@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>

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>
---
 add-patch.c                |  7 ++++++
 t/t3701-add-interactive.sh | 46 ++++++++++++++++++++++++++++++++++----
 2 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index 8c41cdfe39b..5cea70666e9 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -472,6 +472,13 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 			eol = pend;
 
 		if (starts_with(p, "diff ")) {
+			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++;
 			ALLOC_GROW_BY(s->file_diff, s->file_diff_nr, 1,
 				   file_diff_alloc);
 			file_diff = s->file_diff + s->file_diff_nr - 1;
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 77de0029ba5..94537a6b40a 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -326,7 +326,9 @@ test_expect_success 'correct message when there is nothing to do' '
 test_expect_success 'setup again' '
 	git reset --hard &&
 	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
@@ -341,13 +343,27 @@ test_expect_success 'setup patch' '
 	 content
 	+lastline
 	\ 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
 '
 
 # Expected output, diff is similar to the patch but w/ diff at the top
 test_expect_success 'setup 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
 	--- a/file
 	+++ b/file
@@ -366,6 +382,28 @@ test_expect_success 'setup expected' '
 	 content
 	+lastline
 	\ 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
 '
 
@@ -373,8 +411,8 @@ test_expect_success 'setup expected' '
 test_expect_success 'add first line works' '
 	git commit -am "clear local changes" &&
 	git apply patch &&
-	test_write_lines s y y | git add -p file 2>error >raw-output &&
-	sed -n -e "s/^([1-2]\/[1-2]) Stage this hunk[^@]*\(@@ .*\)/\1/" \
+	test_write_lines s y y s y n y | git add -p 2>error >raw-output &&
+	sed -n -e "s/^([1-9]\/[1-9]) Stage this hunk[^@]*\(@@ .*\)/\1/" \
 	       -e "/^[-+@ \\\\]"/p raw-output >output &&
 	test_must_be_empty error &&
 	git diff --cached >diff &&
-- 
gitgitgadget
Previous: Junio C HamanoNext: Ævar Arnfjörð Bjarmason
Message 4 of 21 in “builtin add -p: fix hunk splitting”
  1. 0/2 builtin add -p: fix hunk splittingPhillip Wood via GitGitGadget, Dec 20, 2021
  2. 1/2 t3701: clean up hunk splitting testsPhillip Wood via GitGitGadget, Dec 20, 2021
  3. Junio C HamanoDec 20, 2021
  4. 2/2 builtin add -p: fix hunk splittingPhillip Wood via GitGitGadget, Dec 20, 2021
  5. Ævar Arnfjörð BjarmasonDec 20, 2021
  6. Phillip WoodJan 11, 2022
  7. Ævar Arnfjörð BjarmasonJan 11, 2022
  8. Junio C HamanoDec 20, 2021
  9. 0/2 builtin add -p: fix hunk splittingPhillip Wood via GitGitGadget, Jan 11, 2022
  10. 1/2 t3701: clean up hunk splitting testsPhillip Wood via GitGitGadget, Jan 11, 2022
  11. 2/2 builtin add -p: fix hunk splittingPhillip Wood via GitGitGadget, Jan 11, 2022
  12. Ævar Arnfjörð BjarmasonJan 11, 2022
  13. Phillip WoodJan 11, 2022
  14. Junio C HamanoJan 12, 2022
  15. Phillip WoodJan 19, 2022
  16. Junio C HamanoJan 20, 2022
  17. Ævar Arnfjörð BjarmasonJan 20, 2022
  18. Junio C HamanoJan 20, 2022
  19. Johannes SchindelinJan 22, 2022
  20. Phillip WoodJan 24, 2022
  21. Junio C HamanoJan 12, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.