Web lists-archives.com

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




Elijah Newren <newren@xxxxxxxxx> writes:

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