Björn Gustavsson <bgustavsson@xxxxxxxxx> writes:
> I see at least two possible ways to implement that:
>
> 1. Have "git rebase" give "git apply" a special option so that it
> will apply patches beyond the end of file (and trusting the
> line numbers in the chunks).
>
> 2. Having "git rebase" remember the number of blanks line that
> was removed in each previous file in previous fixed commits
> and add them back just before invoking "git apply".
I actually changed my mind after thinking about it a bit more.
I think you should be able to do the same thing as we already do in
git-apply inside match_fragment(), and without involving any "rebase"
specific hacks nor options, the result will cover most of the real-life
use cases.
The existing logic in the function says "We were told to find a location
in the img (i.e. the text being patched) where preimage appears. We will
replace the copy of the preimage in img with the corresponding postimage.
Unfortunately, the preimage does not exactly match, but if we consider
that we may have applied earlier patches with whitespace=fix, we can see
that the given preimage matches with this location in our img except for
whitespace differences."
When it finds the place that "fuzzily" matches the preimage, it adjusts
the given preimage to match what we have (i.e. pretends that the patch
sender sent a patch based on a version of the text with whitespace fixes
we made in ours already applied), and then let the common logic replace
that copy of preimage with postimage in img.
The hunk is applied successfully, and we are happy.
Using the same logic, your "a/b/c trail" -> "a/b/c gap d/e/f" example
would work like this.
After applying the first patch with blank-at-eof, we will have "a/b/c" (no
trailing blanks) in img.
The second patch comes. It looks like this:
@@ -l,3 +k,6 @@
b
c
+d
+e
+f
This tells you to match "'b c blank' at the end" (lack of trailing context
would trigger match_end hint, I think). So you go ahead and look at your
img, which is "a/b/c" (no trailing blanks). It does not match.
Just like existing logic in match_fragment() knows that the img might have
undergone whitespace fixes, your new logic would realize that with an
addition of one empty line to make your img "a/b/c blank", the preimage of
this hunk matches the way as you are told to search.
At that point, you adjust the preimage to match what we have (the logic is
exactly the same as blank-at-eol case---adjust the patch to pretend that
they started with a whitespace-corrected version we have). This would
make the hunk look like this:
@@ -l,2 +k,6 @@
b
c
+
+d
+e
+f
and by applying that adjusted hunk, you will get the expected result,
namely, "a/b/c gap d/e/f".
Notice that we did all that without ever looking at 'l' and 'k' (hunk
offsets)?
I'd like to see this logic (and only this logic, without relying on the
diff hunk offset numbers at all) done first, because it is very much in
line with what we already do, and more importantly, because it is a more
general solution that is applicable outside the context of rebase.
This of course will not catch the case where you used to have added tons
of blanks at the end in the earlier patch (and losing sight of how many
blanks you removed while applying it). You would need to build a special
case that probably relies on the diff hunk offset, and trigger that
additional logic in rebase (i.e. the caller that _knows_ the hunk offsets
are reliable). That special case may not involve a change to "git apply"
at all, as you suggested as an alternative (2), as you can do that all
inside "git rebase". But I'd rather like to see a solution that does not
rely on the special case as one patch that is independent from rebase, and
the special case built on top of it, as a separate patch.
After all, if you _are_ applying with whitespace=fix and blank-at-eof is
in effect, it is very likely that the nature of the contents in that path
is something that blank lines in the middle does not matter except for
readability (e.g. something like .git/config file format); the fact that
it is safe to strip the blank-at-eof strongly suggests that blanks do not
have semantic meaning and are there purely for readability. In contents
of such a nature, it would not matter if you lost indeterminate number of
blank lines in the middle by applying two patches, the first one leaving
100 blank lines at the end and then the second one adding some non-blank
lines at the end. It might even turn out to be a moot point to worry
about the special case hack if that is the case.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html