# Minor bug in git-apply's patch-cleaning 4 messages from 2007-07-07 to 2007-07-08. Participants: Daniel Barkalow, Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/8911 ## Daniel Barkalow, 2007-07-07 02:21 Subject: Minor bug in git-apply's patch-cleaning Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0707062155170.6977%40iabervon.org ``` 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* ``` ## Johannes Schindelin, 2007-07-07 17:50 Subject: [PATCH] Fix "apply --reverse" with regard to whitespace Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0707071849430.4093%40racer.site In-Reply-To: ``` "git apply" used to take check the whitespace in the wrong direction. Noticed by Daniel Barkalow. Signed-off-by: Johannes Schindelin --- 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 ``` ## Junio C Hamano, 2007-07-08 09:02 Subject: Re: [PATCH] Fix "apply --reverse" with regard to whitespace Message-ID: <7vd4z32rqc.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vd4z32rqc.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin writes: > "git apply" used to take check the whitespace in the wrong > direction. > > Noticed by Daniel Barkalow. > > Signed-off-by: Johannes Schindelin > --- > > 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? ``` ## Johannes Schindelin, 2007-07-08 12:47 Subject: Re: [PATCH] Fix "apply --reverse" with regard to whitespace Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0707081344000.4248%40racer.site In-Reply-To: <7vd4z32rqc.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Sun, 8 Jul 2007, Junio C Hamano wrote: > Johannes Schindelin writes: > > > "git apply" used to take check the whitespace in the wrong > > direction. > > > > Noticed by Daniel Barkalow. > > > > Signed-off-by: Johannes Schindelin > > --- > > > > 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 -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 ```