From: Philip Oakley <philipoakley@iee.org>
To: Antoine Pelisse <apelisse@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git <git@vger.kernel.org>
Subject: Re: [PATCH] diff: Fix rename pretty-print when suffix and prefix overlap
Date: Mon, 25 Feb 2013 22:36:55 +0000 [thread overview]
Message-ID: <512BE787.1090901@iee.org> (raw)
In-Reply-To: <CALWbr2yviqF68zF7mBbhaXW7oFar0YRqROBWXwqjo7UNgZNVBQ@mail.gmail.com>
On 25/02/13 19:50, Antoine Pelisse wrote:
> On Sun, Feb 24, 2013 at 10:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Antoine Pelisse <apelisse@gmail.com> 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
next prev parent reply other threads:[~2013-02-25 22:37 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-23 16:48 [PATCH] diff: Fix rename pretty-print when suffix and prefix overlap Antoine Pelisse
2013-02-24 9:15 ` Junio C Hamano
2013-02-25 19:50 ` Antoine Pelisse
2013-02-25 22:36 ` Philip Oakley [this message]
2013-02-25 22:41 ` Antoine Pelisse
2013-02-28 21:55 ` Antoine Pelisse
2013-02-28 22:14 ` Thomas Rast
2013-02-28 22:22 ` Antoine Pelisse
2013-02-28 22:29 ` Junio C Hamano
2013-03-02 14:38 ` [PATCH] tests: make sure rename pretty print works Antoine Pelisse
2013-03-03 6:50 ` Junio C Hamano
2013-03-06 21:36 ` [PATCH v2] " Antoine Pelisse
2013-03-06 22:03 ` Junio C Hamano
2013-02-26 20:47 ` [PATCH] diff: prevent pprint_rename from underrunning input Thomas Rast
2013-02-26 21:44 ` Junio C Hamano
2013-02-27 13:27 ` Antoine Pelisse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=512BE787.1090901@iee.org \
--to=philipoakley@iee.org \
--cc=apelisse@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.