Minor bug in git-apply's patch-cleaning

Minor bug in git-apply's patch-cleaning

[Daniel Barkalow]

If you apply in reverse a patch which adds junk (e.g., terminal 
whitespace), it complains about the junk you're adding, even though (since 
it's in reverse) you're actually removing that junk.

It's arguable as to whether it should complain about junk in - lines with 
--reverse; it's likely that you care more about getting the patch 
unapplied exactly than not reintroducing removed whitespace. But 
complaining about junk in + lines is actually confusing.

	-Daniel
*This .sig left intentionally blank*

[PATCH] Fix "apply --reverse" with regard to whitespace

[Johannes Schindelin]

"git apply" used to take check the whitespace in the wrong
direction.

Noticed by Daniel Barkalow.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Fri, 6 Jul 2007, Daniel Barkalow wrote:

	> If you apply in reverse a patch which adds junk (e.g., terminal
	> whitespace), it complains about the junk you're adding, even 
	> though (since it's in reverse) you're actually removing that 
	> junk.

	This fixes it.

 builtin-apply.c          |    6 +++++-
 t/t4116-apply-reverse.sh |    6 ++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index c6f736c..0a0b4a9 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1003,12 +1003,16 @@ static int parse_fragment(char *line, unsigned long size, struct patch *patch, s
 			trailing++;
 			break;
 		case '-':
+			if (apply_in_reverse &&
+					new_whitespace != nowarn_whitespace)
+				check_whitespace(line, len);
 			deleted++;
 			oldlines--;
 			trailing = 0;
 			break;
 		case '+':
-			if (new_whitespace != nowarn_whitespace)
+			if (!apply_in_reverse &&
+					new_whitespace != nowarn_whitespace)
 				check_whitespace(line, len);
 			added++;
 			newlines--;
diff --git a/t/t4116-apply-reverse.sh b/t/t4116-apply-reverse.sh
index a7f5905..9ae2b3a 100755
--- a/t/t4116-apply-reverse.sh
+++ b/t/t4116-apply-reverse.sh
@@ -82,4 +82,10 @@ test_expect_success 'apply in reverse without postimage' '
 	)
 '
 
+test_expect_success 'reversing a whitespace introduction' '
+	sed "s/a/a /" < file1 > file1.new &&
+	mv file1.new file1 &&
+	git diff | git apply --reverse --whitespace=error
+'
+
 test_done
-- 
1.5.3.rc0.2712.g125b7f

Re: [PATCH] Fix "apply --reverse" with regard to whitespace

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> "git apply" used to take check the whitespace in the wrong
> direction.
>
> Noticed by Daniel Barkalow.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	On Fri, 6 Jul 2007, Daniel Barkalow wrote:
>
> 	> If you apply in reverse a patch which adds junk (e.g., terminal
> 	> whitespace), it complains about the junk you're adding, even 
> 	> though (since it's in reverse) you're actually removing that 
> 	> junk.
>
> 	This fixes it.

Hmm.  Does this cover the "trailing blank lines removal" as
well?

Re: [PATCH] Fix "apply --reverse" with regard to whitespace

[Johannes Schindelin]

Hi,

On Sun, 8 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > "git apply" used to take check the whitespace in the wrong
> > direction.
> >
> > Noticed by Daniel Barkalow.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >
> > 	On Fri, 6 Jul 2007, Daniel Barkalow wrote:
> >
> > 	> If you apply in reverse a patch which adds junk (e.g., terminal
> > 	> whitespace), it complains about the junk you're adding, even 
> > 	> though (since it's in reverse) you're actually removing that 
> > 	> junk.
> >
> > 	This fixes it.
> 
> Hmm.  Does this cover the "trailing blank lines removal" as
> well?

-- snip --
diff --git a/TODO b/TODO
index ecd2b85..8935376 100644
--- a/TODO
+++ b/TODO
@@ -1,6 +1,5 @@
+interdiff between commits
 diff with file/symlink
-remote remove <nick>
-libification: setjmp() error handler with xmalloc(), xcalloc(), xfree() handling
 fix diff_filepair() mess in diff.c
 merge -s rebase
 build in ls-remote
diff --git a/t/t4116-apply-reverse.sh b/t/t4116-apply-reverse.sh
index 9ae2b3a..f3f9181 100755
--- a/t/t4116-apply-reverse.sh
+++ b/t/t4116-apply-reverse.sh
@@ -84,6 +84,7 @@ test_expect_success 'apply in reverse without postimage' '
 
 test_expect_success 'reversing a whitespace introduction' '
 	sed "s/a/a /" < file1 > file1.new &&
+	echo >> file1.new &&
 	mv file1.new file1 &&
 	git diff | git apply --reverse --whitespace=error
 '
-- snap --

Apparently.

Or not.  Since a patch adding empty lines does not trigger any error with 
--whitespace=error.  I just tested that.

Ciao,
Dscho