threads / patch / 6018

patchxdiff/xemit.c (xdl_find_func): Elide trailing white space in a context header.

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

## tl;dr

6 messages between Oct 23, 2006 and Oct 23, 2006. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Jim Meyering· Oct 23, 2006, 20:22 UTC · lore

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(-)
Show changes to xdiff/xemit.c +2 −1
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
Linus Torvalds· Oct 23, 2006, 20:49 UTC · re: Jim Meyering · lore

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

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
Petr Baudis· Oct 23, 2006, 21:03 UTC · re: Linus Torvalds · lore

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

Dear diary, on Mon, Oct 23, 2006 at 10:49:37PM CEST, I got a letter where Linus Torvalds <torvalds@osdl.org> said that...

Show 24 quoted lines
> 
> 
> 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*',/((..)*)$/)
Linus Torvalds· Oct 23, 2006, 21:10 UTC · re: Petr Baudis · lore

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

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
Jakub Narebski· Oct 23, 2006, 21:15 UTC · re: Linus Torvalds · lore

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

Linus Torvalds wrote:
Show 13 quoted lines
> 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
Jakub Narebski· Oct 23, 2006, 21:06 UTC · re: Linus Torvalds · lore

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

Linus Torvalds wrote:
Show 11 quoted lines
> 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

← back to recent threads