{"thread":{"id":"8223","subject":"[PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file","startedAt":"2007-05-19T13:12:50Z","lastAt":"2007-05-19T23:18:00Z","messageCount":5,"participants":["Marco Costalba","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"42623","messageId":"464EF7D2.4030406@gmail.com","threadId":"8223","inReplyTo":null,"subject":"[PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-19T13:12:50Z","receivedAt":"2007-05-19T13:12:50Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"Signed-off-by: Marco Costalba \n---\n builtin-apply.c | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 0399743..f17f838 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1738,6 +1738,10 @@ 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\twhile (newsize > 1 && !strncmp(new + newsize - 2, \"\\n\\n\", 2))\n+\t\t\tnewsize--;\n+\n \toldlines = old;\n \tnewlines = new;\n \tleading = frag->leading;\n-- \n1.5.2.rc3.87.g404fd-dirty\n"},{"id":"42649","messageId":"7vhcq8v97k.fsf@assigned-by-dhcp.cox.net","threadId":"8223","inReplyTo":"464EF7D2.4030406@gmail.com","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-19T20:39:59Z","receivedAt":"2007-05-19T20:39:59Z","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> builtin-apply.c | 4 ++++\n> 1 files changed, 4 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-apply.c b/builtin-apply.c\n> index 0399743..f17f838 100644\n> --- a/builtin-apply.c\n> +++ b/builtin-apply.c\n> @@ -1738,6 +1738,10 @@ 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\twhile (newsize > 1 && !strncmp(new + newsize - 2, \"\\n\\n\", 2))\n> +\t\t\tnewsize--;\n> +\n> \toldlines = old;\n> \tnewlines = new;\n> \tleading = frag->leading;\n\nI agree to what you are trying to do, but this patch is wrong.\nYou are stripping trailing newlines that were NOT introduced by\nthe patch, but happened to be present in the preimage (and in\nthe context).\n\nTry it on this test vector:\n\ncat >AAA <<\\EOF\na\nb\nc\nd\n\n\ne\nf\ng\nh\ni\n\n\nj\nk\n\n\nEOF\ncat >P.diff <<\\EOF\ndiff --git a/AAA b/AAA\nindex 59f6a9c..ffb28f5 100644\n--- a/AAA\n+++ b/AAA\n@@ -1,4 +1,4 @@\n-a\n+A\n b\n c\n d\n@@ -6,12 +6,11 @@ d\n \n e\n f\n+\n+\n g\n h\n-i\n-\n \n-j\n k\n \n \nEOF\n"},{"id":"42653","messageId":"e5bfff550705191458o3fa1d1a0ndd916d1efafdd2a7@mail.gmail.com","threadId":"8223","inReplyTo":"7vhcq8v97k.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-19T21:58:54Z","receivedAt":"2007-05-19T21:58:54Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/19/07, Junio C Hamano wrote:\n> Marco Costalba writes:\n>\n> > Signed-off-by: Marco Costalba \n> > ---\n> > builtin-apply.c | 4 ++++\n> > 1 files changed, 4 insertions(+), 0 deletions(-)\n> >\n> > diff --git a/builtin-apply.c b/builtin-apply.c\n> > index 0399743..f17f838 100644\n> > --- a/builtin-apply.c\n> > +++ b/builtin-apply.c\n> > @@ -1738,6 +1738,10 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n> > newsize--;\n> > }\n> >\n> > + if (new_whitespace == strip_whitespace)\n> > + while (newsize > 1 && !strncmp(new + newsize - 2, \"\\n\\n\", 2))\n> > + newsize--;\n> > +\n> > oldlines = old;\n> > newlines = new;\n> > leading = frag->leading;\n>\n> I agree to what you are trying to do, but this patch is wrong.\n> You are stripping trailing newlines that were NOT introduced by\n> the patch, but happened to be present in the preimage (and in\n> the context).\n>\n> Try it on this test vector:\n>\n> cat >AAA <<\\EOF\n> a\n> b\n> c\n> d\n>\n>\n> e\n> f\n> g\n> h\n> i\n>\n>\n> j\n> k\n>\n>\n> EOF\n> cat >P.diff <<\\EOF\n> diff --git a/AAA b/AAA\n> index 59f6a9c..ffb28f5 100644\n> --- a/AAA\n> +++ b/AAA\n> @@ -1,4 +1,4 @@\n> -a\n> +A\n> b\n> c\n> d\n> @@ -6,12 +6,11 @@ d\n>\n> e\n> f\n> +\n> +\n> g\n> h\n> -i\n> -\n>\n> -j\n> k\n>\n>\n> EOF\n>\n>\n\nWhat about this?\n\n builtin-apply.c | 13 +++++++++----\n 1 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 9e82757..113c71f 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1746,10 +1746,15 @@ static int apply_one_fragment(struct\nbuffer_desc *desc, struct fragment *frag, i\n \t\tnewsize--;\n \t}\n\n-\tif (new_whitespace == strip_whitespace)\n-\t\twhile (newsize > 1 && !strncmp(new + newsize - 2, \"\\n\\n\", 2))\n-\t\t\tnewsize--;\n-\n+\tif (new_whitespace == strip_whitespace) {\n+\t\tint cnt1 = 1, cnt2 = 1;\n+\t\twhile (newsize - cnt1 > 1 && new[newsize - cnt1] == '\\n')\n+\t\t\tcnt1++;\n+\t\twhile (oldsize - cnt2 > 1 && new[newsize - cnt2] == '\\n')\n+\t\t\tcnt2++;\n+\t\tif (cnt1 > cnt2 && cnt1 > 2)\n+\t\t\tnewsize -= cnt1 - cnt2;\n+\t}\n \toldlines = old;\n \tnewlines = new;\n \tleading = frag->leading;\n"},{"id":"42654","messageId":"7vsl9stnzp.fsf@assigned-by-dhcp.cox.net","threadId":"8223","inReplyTo":"e5bfff550705191458o3fa1d1a0ndd916d1efafdd2a7@mail.gmail.com","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-19T23:03:38Z","receivedAt":"2007-05-19T23:03:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> What about this?\n>\n> builtin-apply.c | 13 +++++++++----\n> 1 files changed, 9 insertions(+), 4 deletions(-)\n\nYou count the trailing blank lines in new and old, and if new\none has more you strip them out, which _sounds_ sane.\n\nBut it is unclear to me how you are limiting the processing to\nthe very end of the file. The \"new\" and \"old\" essentially is a\npatch fragment that is separated into two, and the part you\nmodified with your patch does not know if the hunk applies at\nthe end of the patch yet.\n\nThat is, given this patch:\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 9e82757..113c71f 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1746,10 +1746,15 @@ static int apply_one_fragment(struct buffer_desc *des..\n newsize--;\n }\n\n- if (new_whitespace == strip_whitespace)\n- while (newsize > 1 && !strncmp(new + newsize - 2, \"\\n\\n\", 2))\n- newsize--;\n-\n+ if (new_whitespace == strip_whitespace) {\n+ int cnt1 = 1, cnt2 = 1;\n+ while (newsize - cnt1 > 1 && new[newsize - cnt1] == '\\n')\n+ cnt1++;\n+ while (oldsize - cnt2 > 1 && new[newsize - cnt2] == '\\n')\n+ cnt2++;\n+ if (cnt1 > cnt2 && cnt1 > 2)\n+ newsize -= cnt1 - cnt2;\n+ }\n oldlines = old;\n newlines = new;\n leading = frag->leading;\n\n\"new\" has these lines\n\n newsize--;\n }\n\n if (new_whitespace == strip_whitespace) {\n int cnt1 = 1, cnt2 = 1;\n while (newsize - cnt1 > 1 && new[newsize - cnt1] == '\\n')\n cnt1++;\n while (oldsize - cnt2 > 1 && new[newsize - cnt2] == '\\n')\n cnt2++;\n if (cnt1 > cnt2 && cnt1 > 2)\n newsize -= cnt1 - cnt2;\n }\n oldlines = old;\n newlines = new;\n leading = frag->leading;\n\nwhile \"old\" has this:\n\n newsize--;\n }\n\n if (new_whitespace == strip_whitespace)\n while (newsize > 1 && !strncmp(new + newsize - 2, \"\\n\\n\", 2))\n newsize--;\n\n oldlines = old;\n newlines = new;\n leading = frag->leading;\n\nand these may or may not be at the end of the file, so\ninspecting what blank lines they have at the end is not\nsufficient. If \"new\" does not introduce new blank lines at its\nend, then you can be sure that you are not adding trailing blank\nlines, but even if \"new\" does introduce a new blank line at the\nend, you do not know if that is adding it to the end of the\nfile, or in the middle.\n\nYou do not know where the hunk is applied until you do the loop\nthat follows the part your patch we are discussing.\n"},{"id":"42657","messageId":"7vabw0tnbr.fsf@assigned-by-dhcp.cox.net","threadId":"8223","inReplyTo":"7vsl9stnzp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-19T23:18:00Z","receivedAt":"2007-05-19T23:18:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> ... these may or may not be at the end of the file, so\n> inspecting what blank lines they have at the end is not\n> sufficient. If \"new\" does not introduce new blank lines at its\n> end, then you can be sure that you are not adding trailing blank\n> lines, but even if \"new\" does introduce a new blank line at the\n> end, you do not know if that is adding it to the end of the\n> file, or in the middle.\n>\n> You do not know where the hunk is applied until you do the loop\n> that follows the part your patch we are discussing.\n\nIf I were doing this, I would probably do it this way:\n\n (1) Inside apply_one_fragment(), where \"case '+':\" appears, count\n the blank (not just '\\n', but matches /^\\s*$/) lines at the\n end of \"new\" side. As soon as you fall into \"case ' ':\" or\n \"case '-':\" or non-blank line in \"case '+':\", you reset the\n counter to zero, so that what you are counting is the\n number of blank lines that would have get added, if the\n hunk were to be applied at the end of the file. Keep that\n number ofter you separated the fragment into new and old.\n\n (2) In the same function, inside the big \"for (;;)\" loop that\n figures out where to apply that \"old\" => \"new\" change, use\n the number you gathered in the step (1) to trim what is\n applied, where the real application happens, which is the\n part that has memmove()/memcpy(), only when you know you\n are applying the hunk at the end of the file. That is the\n only place in the function that knows where the hunk is\n being applied.\n"}]}