{"thread":{"id":"8234","subject":"[PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","startedAt":"2007-05-20T09:51:41Z","lastAt":"2007-05-22T11:13:21Z","messageCount":16,"participants":["Marco Costalba","Junio C Hamano","Frank Lichtenheld","Josef Weidendorfer"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"42681","messageId":"e5bfff550705200251j3dd9b377je7ae5bafac988060@mail.gmail.com","threadId":"8234","inReplyTo":null,"subject":"[PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-20T09:51:41Z","receivedAt":"2007-05-20T09:51:41Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"Signed-off-by: Marco Costalba \n---\n\nThis one seems to pass all the tests.\n\n builtin-apply.c | 22 ++++++++++++++++++++++\n 1 files changed, 22 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 0399743..a96f669 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1671,6 +1671,7 @@ static int apply_one_fragment(struct buffer_desc *desc,\n \tchar *new = xmalloc(size);\n \tconst char *oldlines, *newlines;\n \tint oldsize = 0, newsize = 0;\n+\tint trailing_added_lines = 0;\n \tunsigned long leading, trailing;\n \tint pos, lines;\n\n@@ -1699,6 +1700,15 @@ static int apply_one_fragment(struct buffer_desc *desc,\n \t\t\telse if (first == '+')\n \t\t\t\tfirst = '-';\n \t\t}\n+\t\t/*\n+\t\t * Only fragments that add lines at the bottom\n+\t\t * of a file end with a list of '+' lines\n+\t\t*/\n+\t\tif (first == '+')\n+\t\t\ttrailing_added_lines++;\n+\t\telse\n+\t\t\ttrailing_added_lines = 0;\n+\n \t\tswitch (first) {\n \t\tcase '\\n':\n \t\t\t/* Newer GNU diff, empty context line */\n@@ -1738,6 +1748,18 @@ static int apply_one_fragment(struct buffer_desc *desc,\n \t\tnewsize--;\n \t}\n\n+\tif (new_whitespace == strip_whitespace) {\n+\t\t/* Any added empty lines is already cleaned-up here\n+\t\t * becuase of 'strip_whitespace' flag, so just count '\\n'\n+\t\t*/\n+\t\tint empty = 0;\n+\t\twhile ( empty < trailing_added_lines\n+\t\t && newsize - empty - 2 > 0\n+\t\t && new[newsize - empty - 2] == '\\n')\n+\t\t\tempty++;\n+\n+\t\tnewsize -= empty;\n+\t}\n \toldlines = old;\n \tnewlines = new;\n \tleading = frag->leading;\n-- \n1.5.2.rc3.90.gf33e-dirty\n"},{"id":"42686","messageId":"7vabvzq0bb.fsf@assigned-by-dhcp.cox.net","threadId":"8234","inReplyTo":"e5bfff550705200251j3dd9b377je7ae5bafac988060@mail.gmail.com","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-20T10:03:20Z","receivedAt":"2007-05-20T10:03:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> Signed-off-by: Marco Costalba \n> ---\n>\n> This one seems to pass all the tests.\n\nI think this happens to work because you are not feeding -u0\npatch; if you have more than one context, then a hunk that ends\nwith + line is guaranteed to apply only at the end, With a\ndiff prepared with -u0, that is not true anymore, is it?\n\nWe can argue that -u0 patch is crazy but we do support them.\n"},{"id":"42697","messageId":"e5bfff550705200334pef694cn1a7842c23e2672f5@mail.gmail.com","threadId":"8234","inReplyTo":"7vabvzq0bb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-20T10:34:06Z","receivedAt":"2007-05-20T10:34:06Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/20/07, Junio C Hamano wrote:\n> \"Marco Costalba\" writes:\n>\n> > Signed-off-by: Marco Costalba \n> > ---\n> >\n> > This one seems to pass all the tests.\n>\n> I think this happens to work because you are not feeding -u0\n> patch; if you have more than one context, then a hunk that ends\n> with + line is guaranteed to apply only at the end, With a\n> diff prepared with -u0, that is not true anymore, is it?\n>\n\nI don't know much about this -u0 thing, could you please point me to\nan example so I can try to fix also this case?\n\nThanks\nMarco\n"},{"id":"42699","messageId":"7vabvzoij8.fsf@assigned-by-dhcp.cox.net","threadId":"8234","inReplyTo":"e5bfff550705200334pef694cn1a7842c23e2672f5@mail.gmail.com","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-20T11:12:43Z","receivedAt":"2007-05-20T11:12:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> On 5/20/07, Junio C Hamano wrote:\n>> \"Marco Costalba\" writes:\n>>\n>> > Signed-off-by: Marco Costalba \n>> > ---\n>> >\n>> > This one seems to pass all the tests.\n>>\n>> I think this happens to work because you are not feeding -u0\n>> patch; if you have more than one context, then a hunk that ends\n>> with + line is guaranteed to apply only at the end, With a\n>> diff prepared with -u0, that is not true anymore, is it?\n>\n> I don't know much about this -u0 thing, could you please point me to\n> an example so I can try to fix also this case?\n\nI was starting to suspect that I misunderstood what you were\ntrying to do. I thought you were trying to avoid a patch that\nadds (one or more) blank line(s) at the end of the file, but is\nit that you do not want to have a hunk that adds more than one\nblank line anywhere? However, the comment \"Only fragments that\nadd lines at the bottom ends with '+' lines\" suggests otherwise.\n\nBut.\n\nIf you start with this file:\n\n$ git init\n$ cat >AAA <<\\EOF\na\nb\nc\nd\nEOF\n$ git add AAA\n\nand modify it by adding three blank lines between b and c, like\nthis:\n\n$ cat >AAA <<\\EOF\na\nb\n\n\n\nc\nd\nEOF\n\nIf you say \"give me zero lines of context\" (again, I think use\nof -u0 is insane, but we got complaints in the past that we did\nnot get this right), you would get this:\n\n$ git diff --unified=0 >P.diff\n$ cat P.diff\ndiff --git a/AAA b/AAA\nindex d68dd40..8410b89 100644\n--- a/AAA\n+++ b/AAA\n@@ -2,0 +3,3 @@ b\n+\n+\n+\n\nBecause we had the same mistake in our earlier code as you made\nin this patch, which assumed that a hunk that ends with '+' only\napply at the end (and we still assume that by default), if you\napply this with patch git-apply without --unidiff-zero option,\nyou get an error. If you use the option this patch can be\napplied correctly.\n\n$ git checkout -- AAA ;# to go back to the original a/b/c/d\n$ git apply --unidiff-zero P.diff\n\nNow, with --unidiff-zero option, I think your patch will mistake\nthat this hunk adds _trailing_ blank lines, because it does not\nsee anything that comes after the '+'.\n\n I think it should notice that it adds three trailing\n blank lines and should reduce \"new\" to zero lines, but\n somehow it does not seem to do so. You start with\n newsize == 3 and do not allow (newsize-empty) to go\n below 2, so you would get only 1 in empty, not 3, and\n end up reducing this hunk by only one line.\n\n Which may or may not be a bug, but that is besides the\n point.\n\nThe point is that this hunk does not apply to the end of the\nfile, and I do not think you should even be attempting to reduce\n\"new\" at all.\n\nBut the code to determine where in the dest buffer the hunk\napplies to exists way after the point you patched (inside of the\nfor(;;) loop, where we have memmove and memcpy). The memmove is\nto move away the later part of the file to make room if \"new\" is\nlarger than \"old\" (if the hunk deletes more than it adds, the\nmemmove would move the remainder up, otherwise down), and I\nthink that should be the place you would first decide if you are\napplying at the end, and reduce \"new\" only if that is the case.\n\nAm I misreading your patch?\n"},{"id":"42708","messageId":"e5bfff550705200545kcf1f7f9n4f3f6d7d25955e1@mail.gmail.com","threadId":"8234","inReplyTo":"7vabvzoij8.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-20T12:45:59Z","receivedAt":"2007-05-20T12:45:59Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/20/07, Junio C Hamano wrote:\n>\n> I was starting to suspect that I misunderstood what you were\n> trying to do. I thought you were trying to avoid a patch that\n> adds (one or more) blank line(s) at the end of the file, but is\n> it that you do not want to have a hunk that adds more than one\n> blank line anywhere? However, the comment \"Only fragments that\n> add lines at the bottom ends with '+' lines\" suggests otherwise.\n>\n\nNo, you understand right.\n\n>\n> Because we had the same mistake in our earlier code as you made\n> in this patch, which assumed that a hunk that ends with '+' only\n> apply at the end (and we still assume that by default), if you\n> apply this with patch git-apply without --unidiff-zero option,\n> you get an error. If you use the option this patch can be\n> applied correctly.\n>\n\nOk. This is take 3. It works correctly on standard patches and also on\nu0 example that you gave above.\n\nThis patch is on top of git 1.5.2\n\nPlease check it.\n\nbuiltin-apply.c | 34 ++++++++++++++++++++++++++++++++++\n 1 files changed, 34 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 0399743..6032f78 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1671,6 +1671,7 @@ static int apply_one_fragment(struct buffer_desc *desc,\n \tchar *new = xmalloc(size);\n \tconst char *oldlines, *newlines;\n \tint oldsize = 0, newsize = 0;\n+\tint trailing_added_lines = 0;\n \tunsigned long leading, trailing;\n \tint pos, lines;\n\n@@ -1699,6 +1700,17 @@ static int apply_one_fragment(struct buffer_desc *desc,\n \t\t\telse if (first == '+')\n \t\t\t\tfirst = '-';\n \t\t}\n+\t\t/*\n+\t\t * Count lines added at the end of the file.\n+\t\t * This is not enough to get things right in case of\n+\t\t * patches generated with --unified=0, but it's a\n+\t\t * useful upper bound.\n+\t\t*/\n+\t\tif (first == '+')\n+\t\t\ttrailing_added_lines++;\n+\t\telse\n+\t\t\ttrailing_added_lines = 0;\n+\n \t\tswitch (first) {\n \t\tcase '\\n':\n \t\t\t/* Newer GNU diff, empty context line */\n@@ -1738,6 +1750,24 @@ static int apply_one_fragment(struct buffer_desc *desc,\n \t\tnewsize--;\n \t}\n\n+\tif (new_whitespace == strip_whitespace) {\n+\t\t/* Any added empty lines is already cleaned-up here\n+\t\t * becuase of 'strip_whitespace' flag, so just count '\\n'\n+\t\t*/\n+\t\tint empty = 0;\n+\t\twhile ( empty < trailing_added_lines\n+\t\t && newsize - empty > 0\n+\t\t && new[newsize - empty - 1] == '\\n')\n+\t\t\tempty++;\n+\n+\t\tif (empty < trailing_added_lines)\n+\t\t\tempty--;\n+\n+\t\t/* these are the empty lines added at\n+\t\t * the end of the file, modulo u0 patches.\n+\t\t */\n+\t\ttrailing_added_lines = empty;\n+\t}\n \toldlines = old;\n \tnewlines = new;\n \tleading = frag->leading;\n@@ -1770,6 +1800,10 @@ static int apply_one_fragment(struct buffer_desc *desc,\n \t\tif (match_beginning && offset)\n \t\t\toffset = -1;\n \t\tif (offset >= 0) {\n+\n+\t\t\tif (desc->size - oldsize - offset == 0) /* end of file? */\n+\t\t\t\tnewsize -= trailing_added_lines;\n+\n \t\t\tint diff = newsize - oldsize;\n \t\t\tunsigned long size = desc->size + diff;\n \t\t\tunsigned long alloc = desc->alloc;\n"},{"id":"42758","messageId":"7v1whbmjel.fsf@assigned-by-dhcp.cox.net","threadId":"8234","inReplyTo":"e5bfff550705200545kcf1f7f9n4f3f6d7d25955e1@mail.gmail.com","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-20T18:36:50Z","receivedAt":"2007-05-20T18:36:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> Ok. This is take 3. It works correctly on standard patches and also on\n> u0 example that you gave above.\n>\n> This patch is on top of git 1.5.2\n>\n> Please check it.\n\nI think the checks and actions are at the right places (I\nhaven't looked very closely nor tried to run it yet).\n\n> builtin-apply.c | 34 ++++++++++++++++++++++++++++++++++\n> 1 files changed, 34 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-apply.c b/builtin-apply.c\n> index 0399743..6032f78 100644\n> --- a/builtin-apply.c\n> +++ b/builtin-apply.c\n> ...\n> @@ -1770,6 +1800,10 @@ static int apply_one_fragment(struct buffer_desc *desc,\n> \t\tif (match_beginning && offset)\n> \t\t\toffset = -1;\n> \t\tif (offset >= 0) {\n> +\n> +\t\t\tif (desc->size - oldsize - offset == 0) /* end of file? */\n> +\t\t\t\tnewsize -= trailing_added_lines;\n> +\n> \t\t\tint diff = newsize - oldsize;\n> \t\t\tunsigned long size = desc->size + diff;\n> \t\t\tunsigned long alloc = desc->alloc;\n\nBut we have kept our sources -Wdeclaration-after-statement\nclean so far, and this hunk needs a trivial adjustment.\n"},{"id":"42762","messageId":"e5bfff550705201156m244e1cf0v7e6b3ab43fa3b47b@mail.gmail.com","threadId":"8234","inReplyTo":"7v1whbmjel.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-20T18:56:49Z","receivedAt":"2007-05-20T18:56:49Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/20/07, Junio C Hamano wrote:\n> \"Marco Costalba\" writes:\n>\n> > Ok. This is take 3. It works correctly on standard patches and also on\n> > u0 example that you gave above.\n> >\n> > This patch is on top of git 1.5.2\n> >\n> > Please check it.\n>\n> I think the checks and actions are at the right places (I\n> haven't looked very closely nor tried to run it yet).\n>\n> > builtin-apply.c | 34 ++++++++++++++++++++++++++++++++++\n> > 1 files changed, 34 insertions(+), 0 deletions(-)\n> >\n> > diff --git a/builtin-apply.c b/builtin-apply.c\n> > index 0399743..6032f78 100644\n> > --- a/builtin-apply.c\n> > +++ b/builtin-apply.c\n> > ...\n> > @@ -1770,6 +1800,10 @@ static int apply_one_fragment(struct buffer_desc *desc,\n> > if (match_beginning && offset)\n> > offset = -1;\n> > if (offset >= 0) {\n> > +\n> > + if (desc->size - oldsize - offset == 0) /* end of file? */\n> > + newsize -= trailing_added_lines;\n> > +\n> > int diff = newsize - oldsize;\n> > unsigned long size = desc->size + diff;\n> > unsigned long alloc = desc->alloc;\n>\n> But we have kept our sources -Wdeclaration-after-statement\n> clean so far\n\n??????\n\nWie bitte?\n"},{"id":"42766","messageId":"7vd50vl30r.fsf@assigned-by-dhcp.cox.net","threadId":"8234","inReplyTo":"e5bfff550705201156m244e1cf0v7e6b3ab43fa3b47b@mail.gmail.com","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-20T19:16:04Z","receivedAt":"2007-05-20T19:16:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> On 5/20/07, Junio C Hamano wrote:\n> ...\n>> > diff --git a/builtin-apply.c b/builtin-apply.c\n>> > index 0399743..6032f78 100644\n>> > --- a/builtin-apply.c\n>> > +++ b/builtin-apply.c\n>> > ...\n>> > @@ -1770,6 +1800,10 @@ static int apply_one_fragment(struct buffer_desc *desc,\n>> > if (match_beginning && offset)\n>> > offset = -1;\n>> > if (offset >= 0) {\n>> > +\n>> > + if (desc->size - oldsize - offset == 0) /* end of file? */\n>> > + newsize -= trailing_added_lines;\n>> > +\n>> > int diff = newsize - oldsize;\n>> > unsigned long size = desc->size + diff;\n>> > unsigned long alloc = desc->alloc;\n>>\n>> But we have kept our sources -Wdeclaration-after-statement\n>> clean so far\n>\n> ??????\n>\n> Wie bitte?\n\nSorry I forgot to mention that that is \"trivial\" so there is no\nreason to resend. I don't expect me doing much git stuff for\nthe rest of the day, but you'll hear from me about this patch\nlater (hopefully it would appear on 'next' -- we'll see).\n"},{"id":"42767","messageId":"20070520191718.GI4085@planck.djpig.de","threadId":"8234","inReplyTo":"e5bfff550705201156m244e1cf0v7e6b3ab43fa3b47b@mail.gmail.com","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-05-20T19:17:18Z","receivedAt":"2007-05-20T19:17:18Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Sun, May 20, 2007 at 08:56:49PM +0200, Marco Costalba wrote:\n> On 5/20/07, Junio C Hamano wrote:\n> >> if (offset >= 0) {\n> >> +\n> >> + if (desc->size - oldsize - offset == 0) /* end of \n> >file? */\n> >> + newsize -= trailing_added_lines;\n> >> +\n> >> int diff = newsize - oldsize;\n> >> unsigned long size = desc->size + diff;\n> >> unsigned long alloc = desc->alloc;\n> >\n> >But we have kept our sources -Wdeclaration-after-statement\n> >clean so far\n> \n> ??????\n> \n> Wie bitte?\n\nman gcc:\n\n-Wdeclaration-after-statement (C only)\n Warn when a declaration is found after a statement in a block.\n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"},{"id":"42785","messageId":"e5bfff550705201344r274ac9f4g9ca5e1fefe7c12cd@mail.gmail.com","threadId":"8234","inReplyTo":"20070520191718.GI4085@planck.djpig.de","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-20T20:44:27Z","receivedAt":"2007-05-20T20:44:27Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/20/07, Frank Lichtenheld wrote:\n> > >\n> > >But we have kept our sources -Wdeclaration-after-statement\n> > >clean so far\n> >\n> > ??????\n> >\n> > Wie bitte?\n>\n> man gcc:\n>\n> -Wdeclaration-after-statement (C only)\n> Warn when a declaration is found after a statement in a block.\n>\n\nJust for my personal knowledge, what's the meaning of this apparently\nnon-sense kind of warning?\n\nThanks\nMarco\n"},{"id":"42790","messageId":"e5bfff550705201355x7025cef1lb1bcf0566cfffc3f@mail.gmail.com","threadId":"8234","inReplyTo":"7vd50vl30r.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-20T20:55:17Z","receivedAt":"2007-05-20T20:55:17Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/20/07, Junio C Hamano wrote:\n> \"Marco Costalba\" writes:\n>\n> >> > if (offset >= 0) {\n> >> > +\n> >> > + if (desc->size - oldsize - offset == 0) /* end of file? */\n> >> > + newsize -= trailing_added_lines;\n> >> > +\n> >> > int diff = newsize - oldsize;\n> >> > unsigned long size = desc->size + diff;\n> >> > unsigned long alloc = desc->alloc;\n> >>\n\n>\n> Sorry I forgot to mention that that is \"trivial\" so there is no\n> reason to resend. I don't expect me doing much git stuff for\n> the rest of the day, but you'll hear from me about this patch\n> later (hopefully it would appear on 'next' -- we'll see).\n>\n\nOk. Thanks for your help.\n\nP.S: I don't find a trivial way to avoid adding more lines then\nremoved, the shortest trick I can find is\n\nint eof = (desc->size - oldsize - offset == 0);\nint diff = newsize - oldsize - eof * trailing_added_lines;\nunsigned long size = desc->size + diff;\nunsigned long alloc = desc->alloc;\n\nnewsize -= eof * trailing_added_lines;\n\n\nBut is not as elegant as the original.\n"},{"id":"42852","messageId":"7vmyzyhdfh.fsf@assigned-by-dhcp.cox.net","threadId":"8234","inReplyTo":"7v1whbmjel.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-21T06:57:06Z","receivedAt":"2007-05-21T06:57:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Marco Costalba\" writes:\n>\n>> Ok. This is take 3. It works correctly on standard patches and also on\n>> u0 example that you gave above.\n>>\n>> This patch is on top of git 1.5.2\n>>\n>> Please check it.\n>\n> I think the checks and actions are at the right places (I\n> haven't looked very closely nor tried to run it yet).\n\nAfter fixing it up a bit to actually perform the removal only\nunder --whitespace=strip option, I merged it to 'next' and\npushed the result out. Then I found a slight breakage, when I\ntried to reproduce your 6 \"whitespace fix\" series using that\nfamous procedure:\n\n $ git checkout master\n $ rm -f .git/index\n $ git checkout HEAD -- t/ Documentation/\n $ git clean -x -d\n $ git diff -R --binary HEAD >P.diff\n $ git apply --index --whitespace=strip P.diff\n\nWe somehow end up removing one LF too many, like this:\n\n diff --git a/contrib/emacs/.gitignore b/contrib/emacs/.gitignore\n index c531d98..016d3b1 100644\n --- a/contrib/emacs/.gitignore\n +++ b/contrib/emacs/.gitignore\n @@ -1 +1 @@\n -*.elc\n +*.elc\n \\ No newline at end of file\n\nHere is a fix on top of what's in 'next'. I think this is a lot\ncloser to what I outlined originally. Passes the testsuite but\nthat does not tell us much, as they did not catch the breakage\nin your version.\n\nCare to add a few tests for this new feature? Hint, hint...\n\n-- >8 --\n[PATCH] git-apply: Fix removal of new trailing blank lines.\n\nThe earlier code removed one newline too many from the hunk that\nadds new lines at the end of the file. Also the way the code\ncounted the added blank lines was somewhat roundabout; I think\nthe way updated code does it is more direct and easier to\nfollow:\n\n * We keep track of the number of blank lines added;\n\n * While processing each line, we notice if it adds a blank\n line, and increment the counter, or reset it to zero\n otherwise;\n\n * When actually we apply the data, we remove the empty lines we\n counted earlier if we are applying it at the end of the\n file.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-apply.c | 48 +++++++++++++++---------------------------------\n 1 files changed, 15 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex ac7c824..e717898 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1671,7 +1671,7 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \tchar *new = xmalloc(size);\n \tconst char *oldlines, *newlines;\n \tint oldsize = 0, newsize = 0;\n-\tint trailing_added_lines = 0;\n+\tint new_blank_lines_at_end = 0;\n \tunsigned long leading, trailing;\n \tint pos, lines;\n \n@@ -1679,6 +1679,7 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \t\tchar first;\n \t\tint len = linelen(patch, size);\n \t\tint plen;\n+\t\tint added_blank_line = 0;\n \n \t\tif (!len)\n \t\t\tbreak;\n@@ -1700,16 +1701,6 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \t\t\telse if (first == '+')\n \t\t\t\tfirst = '-';\n \t\t}\n-\t\t/*\n-\t\t * Count lines added at the end of the file.\n-\t\t * This is not enough to get things right in case of\n-\t\t * patches generated with --unified=0, but it's a\n-\t\t * useful upper bound.\n-\t\t*/\n-\t\tif (first == '+')\n-\t\t\ttrailing_added_lines++;\n-\t\telse\n-\t\t\ttrailing_added_lines = 0;\n \n \t\tswitch (first) {\n \t\tcase '\\n':\n@@ -1728,9 +1719,14 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \t\t\t\tbreak;\n \t\t/* Fall-through for ' ' */\n \t\tcase '+':\n-\t\t\tif (first != '+' || !no_add)\n-\t\t\t\tnewsize += apply_line(new + newsize, patch,\n-\t\t\t\t\t\t plen);\n+\t\t\tif (first != '+' || !no_add) {\n+\t\t\t\tint added = apply_line(new + newsize, patch,\n+\t\t\t\t\t\t plen);\n+\t\t\t\tnewsize += added;\n+\t\t\t\tif (first == '+' &&\n+\t\t\t\t added == 1 && new[newsize-1] == '\\n')\n+\t\t\t\t\tadded_blank_line = 1;\n+\t\t\t}\n \t\t\tbreak;\n \t\tcase '@': case '\\\\':\n \t\t\t/* Ignore it, we already handled it */\n@@ -1740,6 +1736,10 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \t\t\t\terror(\"invalid start of line: '%c'\", first);\n \t\t\treturn -1;\n \t\t}\n+\t\tif (added_blank_line)\n+\t\t\tnew_blank_lines_at_end++;\n+\t\telse\n+\t\t\tnew_blank_lines_at_end = 0;\n \t\tpatch += len;\n \t\tsize -= len;\n \t}\n@@ -1750,24 +1750,6 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \t\tnewsize--;\n \t}\n \n-\tif (new_whitespace == strip_whitespace) {\n-\t\t/* Any added empty lines is already cleaned-up here\n-\t\t * becuase of 'strip_whitespace' flag, so just count '\\n'\n-\t\t*/\n-\t\tint empty = 0;\n-\t\twhile ( empty < trailing_added_lines\n-\t\t && newsize - empty > 0\n-\t\t && new[newsize - empty - 1] == '\\n')\n-\t\t\tempty++;\n-\n-\t\tif (empty < trailing_added_lines)\n-\t\t\tempty--;\n-\n-\t\t/* these are the empty lines added at\n-\t\t * the end of the file, modulo u0 patches.\n-\t\t */\n-\t\ttrailing_added_lines = empty;\n-\t}\n \toldlines = old;\n \tnewlines = new;\n \tleading = frag->leading;\n@@ -1805,7 +1787,7 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \n \t\t\tif (new_whitespace == strip_whitespace &&\n \t\t\t (desc->size - oldsize - offset == 0)) /* end of file? */\n-\t\t\t\tnewsize -= trailing_added_lines;\n+\t\t\t\tnewsize -= new_blank_lines_at_end;\n \n \t\t\tdiff = newsize - oldsize;\n \t\t\tsize = desc->size + diff;\n-- \n1.5.2.24.g93d4\n"},{"id":"42871","messageId":"200705211059.46678.Josef.Weidendorfer@gmx.de","threadId":"8234","inReplyTo":"e5bfff550705201344r274ac9f4g9ca5e1fefe7c12cd@mail.gmail.com","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Josef Weidendorfer","fromEmail":"josef.weidendorfer@gmx.de","sentAt":"2007-05-21T08:59:46Z","receivedAt":"2007-05-21T08:59:46Z","isPatch":true,"sender":{"key":"josef.weidendorfer@gmx.de","avatar":null},"body":"On Sunday 20 May 2007, Marco Costalba wrote:\n> On 5/20/07, Frank Lichtenheld wrote:\n> > > >\n> > > >But we have kept our sources -Wdeclaration-after-statement\n> > > >clean so far\n> > >\n> > > ??????\n> > >\n> > > Wie bitte?\n> >\n> > man gcc:\n> >\n> > -Wdeclaration-after-statement (C only)\n> > Warn when a declaration is found after a statement in a block.\n> >\n> \n> Just for my personal knowledge, what's the meaning of this apparently\n> non-sense kind of warning?\n\nman gcc:\n\n -Wdeclaration-after-statement (C only)\n Warn when a declaration is found after a statement in a block. This con-\n struct, known from C++, was introduced with ISO C99 and is by default allowed\n in GCC. It is not supported by ISO C90 and was not supported by GCC versions\n before GCC 3.0.\n\nThere are some C compilers out there which break out with an error when\nusing declaration after a statement; however, we want git code to compile\neven using these compilers.\n\nJosef\n\n> \n> Thanks\n> Marco\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"42883","messageId":"e5bfff550705210423i34dc481es61d3b886ae77c5f7@mail.gmail.com","threadId":"8234","inReplyTo":"7vmyzyhdfh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-21T11:23:59Z","receivedAt":"2007-05-21T11:23:59Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/21/07, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>\n> We somehow end up removing one LF too many, like this:\n>\n> diff --git a/contrib/emacs/.gitignore b/contrib/emacs/.gitignore\n> index c531d98..016d3b1 100644\n> --- a/contrib/emacs/.gitignore\n> +++ b/contrib/emacs/.gitignore\n> @@ -1 +1 @@\n> -*.elc\n> +*.elc\n> \\ No newline at end of file\n>\n\nI also had that, but after adding\n\n+\n+ if (empty < trailing_added_lines)\n+ empty--;\n+\n\neverything worked correctly. I made again the same test myself without problems.\n\nI really don't understand how could be broken.\n\n\nFor me it's OK if you don't like my patch, but I would really\nunderstand why that very strange error.\n\nThanks\n Marco\n"},{"id":"42942","messageId":"7vbqgdbq5j.fsf@assigned-by-dhcp.cox.net","threadId":"8234","inReplyTo":"e5bfff550705210423i34dc481es61d3b886ae77c5f7@mail.gmail.com","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-22T01:31:04Z","receivedAt":"2007-05-22T01:31:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> On 5/21/07, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>>\n>>\n>> We somehow end up removing one LF too many, like this:\n>>\n>> diff --git a/contrib/emacs/.gitignore b/contrib/emacs/.gitignore\n>> index c531d98..016d3b1 100644\n>> --- a/contrib/emacs/.gitignore\n>> +++ b/contrib/emacs/.gitignore\n>> @@ -1 +1 @@\n>> -*.elc\n>> +*.elc\n>> \\ No newline at end of file\n>>\n>\n> I also had that, but after adding\n>\n> +\n> + if (empty < trailing_added_lines)\n> + empty--;\n> +\n>\n> everything worked correctly. I made again the same test myself\n> without problems.\n>\n> I really don't understand how could be broken.\n\nHmmm. Puzzled.\n\nLet's say that the patch is to create a file that has a single\nline, like this:\n\ndiff --git a/contrib/emacs/.gitignore b/contrib/emacs/.gitignore\nnew file mode 100644\nindex 0000000..c531d98\n--- /dev/null\n+++ b/contrib/emacs/.gitignore\n@@ -0,0 +1 @@\n+*.elc\n\nThe function \"apply_one_fragment\" gets two lines ('@' and '+').\n\nWe come to \"while (size > 0)\" loop. During the first round,\n'first' is '@' and the line is ignored. In the second round,\n'first' is '+', so apply_line appends the contents to 'new'\nbuffer, while we count trailing_added_lines.\n\nEnd result is\n\n - newsize = 6, new has \"*.elc\\n\";\n - oldsize = 0, and old has \"\";\n - trailing_added_lines = 1;\n\nwhen we get to the \"empty\" counting code.\n\nThen you count empty up to trailing_added_lines. When we get to\nthe \"if (empty < trailing_added_lines)\" code, empty is 1. You\ndo not decrement this, and take that number in\ntrailing_added_lines, to be used to strip the trailing run of\nnewlines in the for (;;) loop later. That's how you can lose\nthe last newline that is not on a blank line.\n"},{"id":"42967","messageId":"e5bfff550705220413v261e1543s220d97ce4b9da07b@mail.gmail.com","threadId":"8234","inReplyTo":"7vbqgdbq5j.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at the end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-22T11:13:21Z","receivedAt":"2007-05-22T11:13:21Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/22/07, Junio C Hamano wrote:\n> \"Marco Costalba\" writes:\n>\n> > On 5/21/07, Junio C Hamano wrote:\n> >> Junio C Hamano writes:\n> >>\n> >>\n> >> We somehow end up removing one LF too many, like this:\n> >>\n> >> diff --git a/contrib/emacs/.gitignore b/contrib/emacs/.gitignore\n> >> index c531d98..016d3b1 100644\n> >> --- a/contrib/emacs/.gitignore\n> >> +++ b/contrib/emacs/.gitignore\n> >> @@ -1 +1 @@\n> >> -*.elc\n> >> +*.elc\n> >> \\ No newline at end of file\n> >>\n> >\n\nThe final, and correct version is:\n\n if (new_whitespace == strip_whitespace && trailing_added_lines) {\n\n\tint n = 0;\n\tfor ( ; n <= trailing_added_lines; n++) { /* counting trailing '\\n' */\n\n\t\tif (newsize == n) {\n\t\t\tn++;\n\t\t\tbreak;\n\t\t}\n\t\tif (new[newsize - 1 - n] != '\\n')\n\t\t\tbreak;\n\t}\n trailing_added_lines = (n>0) ? --n : 0;\n } else\n\ttrailing_added_lines = 0;\n\n\nbut I understand is ugly as hell. The fact is, it is far easier to\ncount '\\n' *while* they are created then after at the end.\n\n\nSo no problem for me if you drop my patch.\n\n\n Marco\n"}]}