git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Phillip Wood" <phillip.wood@dunelm.org.uk>,
	"Phillip Wood" <phillip.wood@dunelm.org.uk>
Subject: [PATCH 2/2] builtin add -p: fix hunk splitting
Date: Mon, 20 Dec 2021 14:32:57 +0000	[thread overview]
Message-ID: <5d5639c2b0474680850b7adbb7c5ec81d124eb50.1640010777.git.gitgitgadget@gmail.com> (raw)
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

  parent reply	other threads:[~2021-12-20 14:33 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-20 14:32 [PATCH 0/2] builtin add -p: fix hunk splitting Phillip Wood via GitGitGadget
2021-12-20 14:32 ` [PATCH 1/2] t3701: clean up hunk splitting tests Phillip Wood via GitGitGadget
2021-12-20 21:09   ` Junio C Hamano
2021-12-20 14:32 ` Phillip Wood via GitGitGadget [this message]
2021-12-20 19:06   ` [PATCH 2/2] builtin add -p: fix hunk splitting Ævar Arnfjörð Bjarmason
2022-01-11 11:13     ` Phillip Wood
2022-01-11 11:44       ` Ævar Arnfjörð Bjarmason
2021-12-20 21:30   ` Junio C Hamano
2022-01-11 11:12 ` [PATCH v2 0/2] " Phillip Wood via GitGitGadget
2022-01-11 11:12   ` [PATCH v2 1/2] t3701: clean up hunk splitting tests Phillip Wood via GitGitGadget
2022-01-11 11:12   ` [PATCH v2 2/2] builtin add -p: fix hunk splitting Phillip Wood via GitGitGadget
2022-01-11 12:07   ` [PATCH v2 0/2] " Ævar Arnfjörð Bjarmason
2022-01-11 18:57     ` Phillip Wood
2022-01-12 18:51       ` Junio C Hamano
2022-01-19 20:01         ` Phillip Wood
2022-01-20  5:02           ` Junio C Hamano
2022-01-20  8:42             ` Ævar Arnfjörð Bjarmason
2022-01-20 19:13               ` Junio C Hamano
2022-01-22  9:05           ` Johannes Schindelin
2022-01-24 11:10             ` Phillip Wood
2022-01-12 18:34   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5d5639c2b0474680850b7adbb7c5ec81d124eb50.1640010777.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).