threads / patch / 49812

patchAllow "no-op hunks" when editing the diff in git add -e

Subject: [PATCH 0/1] Allow "no-op hunks" when editing the diff in git add -e

## tl;dr

3 messages between Nov 12, 2018 and Nov 12, 2018. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Johannes Schindelin via GitGitGadget· Nov 12, 2018, 20:54 UTC · lore

I use git add -e frequently. Often there are multiple hunks and I end up deleting the + lines and converting the - lines to context lines, as I like to stage massive changes in an incremental fashion (and commit those staged changes incrementally, too).

Some time after I invented git add -e, this feature was broken, and I had to start identifying and deleting those hunks with no changes.

I finally got around to find the regression, and to fix it. Here is the outcome of this effort.

Johannes Schindelin (1):
  apply --recount: allow "no-op hunks"

 apply.c                |  2 +-
 t/t4136-apply-check.sh | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-74%2Fdscho%2Fapply-recount-empty-hunk-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-74/dscho/apply-recount-empty-hunk-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/74

-- 
gitgitgadget

Johannes Schindelin via GitGitGadget· Nov 12, 2018, 20:54 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 1/1] apply --recount: allow "no-op hunks"

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When editing patches e.g. in `git add -e`, it is quite common that a hunk ends up having no -/+ lines, i.e. it is now supposed to do nothing.

This use case was broken by ad6e8ed37bc1 (apply: reject a hunk that does not do anything, 2015-06-01) with the good intention of catching a very real, different issue in hand-edited patches.

So let's use the `--recount` option as the tell-tale whether the user would actually be okay with no-op hunks.

Add a test case to make sure that this use case does not regress again.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 apply.c                |  2 +-
 t/t4136-apply-check.sh | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

Show changes to 2 files +13 −1

apply.c, t/t4136-apply-check.sh

diff --git a/apply.c b/apply.c
index 073d5f0451..76955afb00 100644
--- a/apply.c
+++ b/apply.c
@@ -1748,7 +1748,7 @@ static int parse_fragment(struct apply_state *state,
 	}
 	if (oldlines || newlines)
 		return -1;
-	if (!deleted && !added)
+	if (!patch->recount && !deleted && !added)
 		return -1;
 
 	fragment->leading = leading;
diff --git a/t/t4136-apply-check.sh b/t/t4136-apply-check.sh
index 6d92872318..4c3f264a63 100755
--- a/t/t4136-apply-check.sh
+++ b/t/t4136-apply-check.sh
@@ -29,6 +29,18 @@ test_expect_success 'apply exits non-zero with no-op patch' '
 	test_must_fail git apply --check input
 '
 
+test_expect_success '`apply --recount` allows no-op patch' '
+	echo 1 >1 &&
+	git apply --recount --check <<-\EOF
+	diff --get a/1 b/1
+	index 6696ea4..606eddd 100644
+	--- a/1
+	+++ b/1
+	@@ -1,1 +1,1 @@
+	 1
+	EOF
+'
+
 test_expect_success 'invalid combination: create and copy' '
 	test_must_fail git apply --check - <<-\EOF
 	diff --git a/1 b/2

-- 
gitgitgadget

Josh Steadmon· Nov 12, 2018, 23:12 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 1/1] apply --recount: allow "no-op hunks"

On 2018.11.12 12:54, Johannes Schindelin via GitGitGadget wrote:

Show 15 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> When editing patches e.g. in `git add -e`, it is quite common that a
> hunk ends up having no -/+ lines, i.e. it is now supposed to do nothing.
> 
> This use case was broken by ad6e8ed37bc1 (apply: reject a hunk that does
> not do anything, 2015-06-01) with the good intention of catching a very
> real, different issue in hand-edited patches.
> 
> So let's use the `--recount` option as the tell-tale whether the user
> would actually be okay with no-op hunks.
> 
> Add a test case to make sure that this use case does not regress again.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Looks good to me.

Reviewed-by: Josh Steadmon <steadmon@google.com>