Re: [PATCH] diff: Fix rename pretty-print when suffix and prefix overlap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/02/13 19:50, Antoine Pelisse wrote:
On Sun, Feb 24, 2013 at 10:15 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
Antoine Pelisse <apelisse@xxxxxxxxx> writes:

When considering a rename for two files that have a suffix and a prefix
that can overlap, a confusing line is shown. As an example, renaming
"a/b/b/c" to "a/b/c" shows "a/b/{ => }/b/c".

This would be vastly more readable if it had "It should show XXX
instead" somewhere in the description, perhaps at the end of this
sentence.  It can also be after "thus the { => }" below, but I think
giving the expected output earlier would be more appropriate.

Good catch, this would probably be better:

     When considering a rename for two files that have a suffix and a prefix
     that can overlap, a confusing line is shown. As an example, renaming
     "a/b/b/c" to "a/b/c" shows "a/b/{ => }/b/c", instead of "a/b/{ => b}/c"

Currently, what we do is calculate the common prefix ("a/b/"), and the
common suffix ("/b/c"), but the same "/b/" is actually counted both in
prefix and suffix. Then when calculating the size of the non-common part,
we end-up with a negative value which is reset to 0, thus the "{ => }".

In this example, the common prefix would be "a/b/" and the common
suffix that does not overlap with the prefix part would be "/c", so
I am imagining that "a/b/{ => b}/c" would be the desired output?

Yes, at least that's what I expected.

Surely it would be "a/b/{b => }/c", that is, we have reduced the number of b's by one. Or am I misunderstanding something?
(I'm guessing it was an all too obvious typo that was misread)


This is a really old thinko (dating back to June 2005).  I'll queue
the patch on maint-1.7.6 (because 1.7.6.6 is slightly more than one
year old while 1.7.5.4 is a lot older) to allow distros that issue
incremental fixes on top of ancient versions of Git to pick up the
fix if they wanted to.  Perhaps we would want to add a few tests?

I can easily understand why that was missed.
I will try to resubmit with tests very soon.
Philip
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]