Re: [PATCH v4] log,diff-tree: add --combined-all-names option

From: Elijah Newren <newren@gmail.com>
Date: Feb 7, 2019, 19:50 UTC
Message-ID: <CABPp-BG4WftXZgbzN48zSo1Z5BcWYjdbz+8hGSGBNJbSsYUzAA@mail.gmail.com>
In-Reply-To: <xmqqa7jarmvo.fsf@gitster-ct.c.googlers.com>

On Tue, Feb 5, 2019 at 12:39 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 34 quoted lines

>
> Elijah Newren <newren@gmail.com> writes:
>
> >> > Further, in patch format, this changes the from/to headers so that
> >> > instead of just having one "from" header, we get one for each parent.
> >> > For example, instead of having
> >> >
> >> >   --- a/phooey.c
> >> >   +++ b/phooey.c
> >> >
> >> > we would see
> >> >
> >> >   --- a/fooey.c
> >> >   --- a/fuey.c
> >> >   +++ b/phooey.c
> >>
> >> Do we have the three "rename from fooey.c", "rename from fuey.c" and
> >> "rename to "phooey.c" extended headers, too?  That's what I meant in
> >> my response, but I do like what I see in the above example ;-)
> >
> > Ah, gotcha.  I'll look into whether it's possible to hook it up to
> > diff.c's fill_metainfo() .
>
> Just to clarify.  I do not think these extended headers are "must
> have"; the "--cc" output is not meant for machine consumption, as it
> simplifies the output by omitting hunks that preimage trees agree
> with each other etc., and making the resulting "patch" not showing
> the whole picture, and these extended header lines might only become
> waste of the screen real estate.
>
> So, do not spend too much effort to emit these textual info that can
> be easily seen with the N+1 plus/minus header lines.
>
> Thanks.

Understood, thanks.

I think something can be done here, but I'm unsure exactly what. From Documentation/diff-generate-patch.txt, the extended headers for normal diff mode are:

       old mode <mode>
       new mode <mode>
       deleted file mode <mode>
       new file mode <mode>
       copy from <path>
       copy to <path>
       rename from <path>
       rename to <path>
       similarity index <number>
       dissimilarity index <number>
       index <hash>..<hash> <mode>

and for combined diffs they are:

       index <hash>,<hash>..<hash>
       mode <mode>,<mode>..<mode>
       new file mode <mode>
       deleted file mode <mode>,<mode>

meaning that the ones we are missing are:

       copy from <path>
       copy to <path>
       rename from <path>
       rename to <path>
       similarity index <number>
       dissimilarity index <number>

I think "copy from" and "rename from" should be relatively
straightforward.  However, in a combined diff, we could have both a
modified status, a renamed status, and a copied status, meaning that
we'll need an array of both similarity and dissimilarity indexes...and
trying to present that to the user in a way that makes sense seems
like a lost cause to me.  Does anyone else know how to represent that?
 I'm inclined to just leave it out.

Also, I'm afraid "copy to" and "rename to" could be confusing if both appeared, since there's only one "to" path. If there is both a copy and a rename involved relative to different parents, should these be coalesced into a "copy/rename to" line?

Thanks, Elijah