Antoine Pelisse [off-list ref] writes:
On Sun, Jun 9, 2013 at 10:07 PM, Junio C Hamano [off-list ref] wrote:
quoted
When any ignore blank option is used, there will be lines that
actually has changes (hence should be shown with +/-) but we
deliberately ignore their changes (hence, if they ever appear in the
hunk, they do so as context lines prefixed with SP not +/-). When
we do so, we show the lines from the postimage in the context.
Don't we actually use preimage (see below) ? I think using pre-image
allows the patch to be applicable to another tree (but ignoring the
space changes).
But the result of such patch application is not usually what you
want to use. If we use postimage (which by the way was a deliberate
design decision we made earlier), at least the review of the patch
is easier because you would see the end result more clearly.
If we actually hide new blank lines that are in the context, it means
that we won't be able to apply a patch with 2 new blank lines in the 3
line context.
Yes, but I do not think the point of --ignore-blank-lines is to
produce a patch that can be applied in the first place. It is to
allow easier eyeballing.
Anyway, I'm starting to think that "show blank lines changes near
other changes" makes sense more and more sense.
Probably.
On Mon, Jun 10, 2013 at 11:43 PM, Junio C Hamano [off-list ref] wrote:
Antoine Pelisse [off-list ref] writes:
quoted
On Sun, Jun 9, 2013 at 10:07 PM, Junio C Hamano [off-list ref] wrote:
quoted
When any ignore blank option is used, there will be lines that
actually has changes (hence should be shown with +/-) but we
deliberately ignore their changes (hence, if they ever appear in the
hunk, they do so as context lines prefixed with SP not +/-). When
we do so, we show the lines from the postimage in the context.
Don't we actually use preimage (see below) ? I think using pre-image
allows the patch to be applicable to another tree (but ignoring the
space changes).
Answering to myself: OK, my package version of git is 1.7.9.5 while
the post-image is used since 1.7.10 or something. That explains my
confusion.
But the result of such patch application is not usually what you
want to use. If we use postimage (which by the way was a deliberate
design decision we made earlier), at least the review of the patch
is easier because you would see the end result more clearly.
I've found the patch and discussion [1] about that switch from
pre-image to post-image, so I can understand the motives (and see that
you actually considered problems for applying such a patch). I always
felt confident that running "git send-email -w" would send a patch
(that can be applied) without the potential space errors/changes I
would have added.
I think it's unfortunate that Git does generate patches with git-diff
that can't be applied if any space option is used. I'm still not
really convinced by the pre-image to post-image change, and maybe I
would have made it a non-default option. What is done is done, but I'd
rather like not do the same here, if possible.
quoted
If we actually hide new blank lines that are in the context, it means
that we won't be able to apply a patch with 2 new blank lines in the 3
line context.
Yes, but I do not think the point of --ignore-blank-lines is to
produce a patch that can be applied in the first place. It is to
allow easier eyeballing.
I think it can not be applied because it's *hard* for a computer to
actually find the correct location, and it may be equally hard for the
reader to evaluate the change with removed/different context.
quoted
Anyway, I'm starting to think that "show blank lines changes near
other changes" makes sense more and more sense.
Probably.
I'm glad to see how convinced you are ;)
I will send my patch and see what makes more sense.
[1]: $gmane/188305