Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file

From: Junio C Hamano <junkio@cox.net>
Date: May 19, 2007, 23:03 UTC
Message-ID: <7vsl9stnzp.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <e5bfff550705191458o3fa1d1a0ndd916d1efafdd2a7@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

> What about this?
>
> builtin-apply.c |   13 +++++++++----
> 1 files changed, 9 insertions(+), 4 deletions(-)

You count the trailing blank lines in new and old, and if new one has more you strip them out, which _sounds_ sane.

But it is unclear to me how you are limiting the processing to the very end of the file. The "new" and "old" essentially is a patch fragment that is separated into two, and the part you modified with your patch does not know if the hunk applies at the end of the patch yet.

That is, given this patch:

diff --git a/builtin-apply.c b/builtin-apply.c
index 9e82757..113c71f 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1746,10 +1746,15 @@ static int apply_one_fragment(struct buffer_desc *des..
                newsize--;
        }

-       if (new_whitespace == strip_whitespace)
-               while (newsize > 1 && !strncmp(new + newsize - 2, "\n\n", 2))
-                       newsize--;
-
+       if (new_whitespace == strip_whitespace) {
+               int cnt1 =  1, cnt2 = 1;
+               while (newsize -  cnt1 > 1 && new[newsize - cnt1] == '\n')
+                       cnt1++;
+               while (oldsize -  cnt2 > 1 && new[newsize - cnt2] == '\n')
+                       cnt2++;
+               if (cnt1 > cnt2 && cnt1 > 2)
+                       newsize -= cnt1 - cnt2;
+       }
        oldlines = old;
        newlines = new;
        leading = frag->leading;

"new" has these lines

                newsize--;
        }

        if (new_whitespace == strip_whitespace) {
                int cnt1 =  1, cnt2 = 1;
                while (newsize -  cnt1 > 1 && new[newsize - cnt1] == '\n')
                        cnt1++;
                while (oldsize -  cnt2 > 1 && new[newsize - cnt2] == '\n')
                        cnt2++;
                if (cnt1 > cnt2 && cnt1 > 2)
                        newsize -= cnt1 - cnt2;
        }
        oldlines = old;
        newlines = new;
        leading = frag->leading;

while "old" has this:

                newsize--;
        }

        if (new_whitespace == strip_whitespace)
                while (newsize > 1 && !strncmp(new + newsize - 2, "\n\n", 2))
                        newsize--;

        oldlines = old;
        newlines = new;
        leading = frag->leading;

and these may or may not be at the end of the file, so
inspecting what blank lines they have at the end is not
sufficient.  If "new" does not introduce new blank lines at its
end, then you can be sure that you are not adding trailing blank
lines, but even if "new" does introduce a new blank line at the
end, you do not know if that is adding it to the end of the
file, or in the middle.

You do not know where the hunk is applied until you do the loop
that follows the part your patch we are discussing.