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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 25, 2019, 22:21 UTC
Message-ID: <xmqq8sz8fkfi.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CABPp-BFjUgY5htSbPpnamPDRM3bCLNsQv-K35uXoRzaT2f85tA@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 8 quoted lines

> 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.

Show 7 quoted lines

> 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.