| Message ID | 20190208011247.21021-3-newren@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] log,diff-tree: add --combined-all-names option | expand |
Elijah Newren <newren@gmail.com> 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.
On Thu, Feb 7, 2019 at 8:14 PM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> 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. Yeah, fair enough. I'll drop the patch.
diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt index f10ca410ad..f0c2a17aef 100644 --- a/Documentation/diff-generate-patch.txt +++ b/Documentation/diff-generate-patch.txt @@ -128,12 +128,11 @@ or like this (when `--cc` option is used): mode <mode>,<mode>..<mode> new file mode <mode> deleted file mode <mode>,<mode> + copy from <path> + rename from <path> + The `mode <mode>,<mode>..<mode>` line appears only if at least one of -the <mode> is different from the rest. Extended headers with -information about detected contents movement (renames and -copying detection) are designed to work with diff of two -<tree-ish> and are not used by combined diff format. +the <mode> is different from the rest. 3. It is followed by two-line from-file/to-file header diff --git a/combine-diff.c b/combine-diff.c index 54cb892ae5..04a139ea03 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -981,6 +981,21 @@ static void show_combined_header(struct combine_diff_path *elem, printf("%s\n", c_reset); } + 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; + } + } + if (!show_file_header) return;
This also adds "rename from <oldpath>" and "copy from <oldpath>" extended headers when renames or copies are involved. Signed-off-by: Elijah Newren <newren@gmail.com> --- Documentation/diff-generate-patch.txt | 7 +++---- combine-diff.c | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-)