From: Philip Oakley Date: Mon, 25 Feb 2013 22:36:55 GMT Subject: Re: [PATCH] diff: Fix rename pretty-print when suffix and prefix overlap Message-ID: <512BE787.1090901@iee.org> In-Reply-To: On 25/02/13 19:50, Antoine Pelisse wrote: > On Sun, Feb 24, 2013 at 10:15 AM, Junio C Hamano wrote: >> Antoine Pelisse 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