[PATCH] xdiff/xemit.c (xdl_find_func): Elide trailing white space in a context header.

[PATCH] xdiff/xemit.c (xdl_find_func): Elide trailing white space in a context header.

[Jim Meyering]

This removes trailing blanks from git-generated diff headers
the same way a similar patch did that for GNU diff:

  http://article.gmane.org/gmane.comp.gnu.utils.bugs/13839

Signed-off-by: Jim Meyering <jim@meyering.net>
---
 xdiff/xemit.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 714c563..1bca639 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -92,6 +92,8 @@ static void xdl_find_func(xdfile_t *xf,
 				len = sz;
 			if (len && rec[len - 1] == '\n')
 				len--;
+			while (0 < len && isspace ((unsigned char) rec[len - 1]))
+				len--;
 			memcpy(buf, rec, len);
 			*ll = len;
 			return;
--
1.4.3.1.g178e-dirty

Re: [PATCH] xdiff/xemit.c (xdl_find_func): Elide trailing white space in a context header.

[Linus Torvalds]

On Mon, 23 Oct 2006, Jim Meyering wrote:
>
> This removes trailing blanks from git-generated diff headers
> the same way a similar patch did that for GNU diff:

NO!

This is _wrong_

You should only remove the space IF IT IS THE ONLY THING ON THE WHOLE 
LINE!

You must not remove white-space in general.

So the patch should check something like

	if (len == 1 && rec[0] == ' ')
		len = 0;

and not like you did it.

Otherwise the patch will simply not even _apply_.

		Linus

Re: [PATCH] xdiff/xemit.c (xdl_find_func): Elide trailing white space in a context header.

[Petr Baudis]

Dear diary, on Mon, Oct 23, 2006 at 10:49:37PM CEST, I got a letter
where Linus Torvalds <torvalds@osdl.org> said that...
> 
> 
> On Mon, 23 Oct 2006, Jim Meyering wrote:
> >
> > This removes trailing blanks from git-generated diff headers
> > the same way a similar patch did that for GNU diff:
> 
> NO!
> 
> This is _wrong_
> 
> You should only remove the space IF IT IS THE ONLY THING ON THE WHOLE 
> LINE!
> 
> You must not remove white-space in general.
> 
> So the patch should check something like
> 
> 	if (len == 1 && rec[0] == ' ')
> 		len = 0;
> 
> and not like you did it.
> 
> Otherwise the patch will simply not even _apply_.

Not really - mind you, this is only about the diff headers. Actually, it
is only about the hunk headers, specifically when we append the function
name to the hunk header. A trailing whitespace can come out of that even
if the source does not have any trailing whitespaces since we trim the
line at 40 bytes.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Re: [PATCH] xdiff/xemit.c (xdl_find_func): Elide trailing white space in a context header.

[Linus Torvalds]

On Mon, 23 Oct 2006, Petr Baudis wrote:
> 
> Not really - mind you, this is only about the diff headers. Actually, it
> is only about the hunk headers, specifically when we append the function
> name to the hunk header.

Ahh. I totally misunderstood, looking at just the diff, and not the bigger 
context of it.

Removing whitespace from the "comment" in the hunk header is obviously 
fine.

Sorry for the noise.

		Linus

Re: [PATCH] xdiff/xemit.c (xdl_find_func): Elide trailing white space in a context header.

[Jakub Narebski]

Linus Torvalds wrote:

> On Mon, 23 Oct 2006, Petr Baudis wrote:
>> 
>> Not really - mind you, this is only about the diff headers. Actually, it
>> is only about the hunk headers, specifically when we append the function
>> name to the hunk header.
> 
> Ahh. I totally misunderstood, looking at just the diff, and not the bigger 
> context of it.
> 
> Removing whitespace from the "comment" in the hunk header is obviously 
> fine.
> 
> Sorry for the noise.

Well, this points out that commit message was not clear enough.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] xdiff/xemit.c (xdl_find_func): Elide trailing white space in a context header.

[Jakub Narebski]

Linus Torvalds wrote:

> On Mon, 23 Oct 2006, Jim Meyering wrote:
>>
>> This removes trailing blanks from git-generated diff headers
>> the same way a similar patch did that for GNU diff:
> 
> NO!
> 
> This is _wrong_
> 
> You should only remove the space IF IT IS THE ONLY THING ON THE WHOLE 
> LINE!

If I understand the description of this patch given by the mentioned article
and example therein (Jim, you should include appropriate parts of mentioned
email in commit message, not just provide link to it), trailing spaces are
removed from extended diff header, i.e. the "which function are we in",
diff -p header. Not from diff body.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git