Re: [PATCH v5 2/2] squash! log,diff-tree: add --combined-all-paths option

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 8, 2019, 04:14 UTC
Message-ID: <xmqq4l9ench8.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190208011247.21021-3-newren@gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 14 quoted lines

> +	for (i = 0; i < num_parent; i++) {
> +		switch (elem->parent[i].status) {
> +		case DIFF_STATUS_COPIED:
> +			dump_quoted_path("copy from ", "",
> +					 elem->parent[i].path.buf,
> +					 line_prefix, c_meta, c_reset);
> +			break;
> +		case DIFF_STATUS_RENAMED:
> +			dump_quoted_path("rename from ", "",
> +					 elem->parent[i].path.buf,
> +					 line_prefix, c_meta, c_reset);
> +			break;
> +		}
> +	}

The explanation for this addition was that it is hard to tell from which side a rename happened in the three-dash lines alone:

    --- a/packages/search/ete/src/test/resources/test-suite.yml
    --- a/packages/search/src/geb/resources/test-suite.yml
    +++ b/packages/search/ete/src/test/resources/test-suite.yml

and your hope was that adding:

    rename from packages/search/src/geb/resources/test-suite.yml

would help especially when the path is overly long.

But I am not sure if that single "rename from" is all that helpful. You cannot tell relative to which parent the rename happened without going back to the three-dash lines. A loop that iterates over all parents but shows only a line for a parent that actually had copy or rename loses "the line is talking about the change from this parent" which is a fairly important piece of information, doesn't it?

If we attempt to clarify it by adding some more information on the line, e.g.

    rename relative to parent #1 from packages/search/src/geb/...

the line gets even longer, and going back to look at

    --- a/packages/search/src/geb/resources/test-suite.yml

may turn out to be an easier way to learn that information. So,... I dunno.