Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames

[Elijah Newren <newren@gmail.com>]

On Fri, Jan 25, 2019 at 2:21 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > Also, my first version of the patch actually showed all names, on all
> > lines, but I found the heavy repetition really annoying, and not in
> > keeping with how non-merge commits are handled (where original
> > filenames are only shown when they differ).  Granted, my change isn't
> > the only one.  We could just have all names shown if they are not all
> > identical, as you suggest and I also considered, but I liked this way
> > slightly better.  If others feel strongly, I can change it, that was
> > just my gut feel and preference.
>
> I do not have a strong _preference_; I am only worried about ease of
> use by consumers of the raw output, i.e. scripts.  From that point
> of view, --all-names showing all names always even when there is no
> rename is involved would certainly be the easiest to write a parser
> for, and lines being overly repetitive is a non-issue.  For human
> consumption, it would be a separate matter, but they won't be
> reading from the --raw output that lists blob object names
> repetitively (the object names may or may not be different, but they
> are all equally gibberish) before showing the human-readble part,
> which are names.

Okay, I've switched it over.

> > You could kind of ask the same question of -c -p, actually.  I looked
> > into that, but I was only interested in raw format output and --cc is
> > only about coalescing uninteresting hunks in patches.  Whenever git
> > shows a combined diff in patch format, it always lists two files in
> > the header, e.g.:
> >   a/foo.c
> >   b/foo.c
>
> Yeah, but with a new option, it does not have to stay that way.  It
> is OK to show a/foo.c, b/bar.c, c/baz.c, or perhaps an easy way to
> avoid overlong unreadable lines in these human readable output may
> be to only show b/foo.c to report the result and list the original
> names using the "rename from" "rename to" extended diff-headers
> shown in normal diff output with rename detection enabled.
>
> > ...  I guess the "combined" patch is already kind of
> > special, so it could make sense, ...
>
> Absolutely.  -c/--cc output with -p departs from the usual "diff is
> to compare two things" by having two +/- indicator columns and more
> than two '@'s in hunk header to begin with.  It is only natural if
> we are showing more/all names in --raw, we should avoid losing info
> in the same way.

This turned out to be easier to implement than I expected, with one
small wrinkle: We have the "a/" and "b/" prefixes, and switching to
"a/", "b/" and "c/" doesn't work due to things like the
diff.mnemonicPrefix setting -- plus I didn't want to plumb multiple
prefix options everywhere they'd be needed.  Instead, I decided that
instead of having a "from" and a "to" header, we would have N "from"
headers (since all parents are what we came from) and 1 "to" header.
I hope that makes sense, because otherwise I think this become to
unwieldy of a change.