{"thread":{"id":"50310","subject":"[PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","startedAt":"2019-01-24T17:09:08Z","lastAt":"2019-02-08T17:50:50Z","messageCount":39,"participants":["Elijah Newren","brian m. carlson","Derrick Stolee","Junio C Hamano","Eric Sunshine","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"367563","messageId":"20190124164654.1923-1-newren@gmail.com","threadId":"50310","inReplyTo":null,"subject":"[PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-01-24T16:46:54Z","receivedAt":"2019-01-24T17:09:08Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The raw diff format for merges with -c or --cc will only list one\nfilename, even if rename detection is active and a rename was detected\nfor the given path. Examples:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n\nThis doesn't let us know what the original name of bar.sh was in the\nfirst parent, and doesn't let us know what either of the original\nnames of phooey.c were in either of the parents. In contrast, for\nnon-merge commits, raw format does provide original filenames (and a\nrename score to boot). In order to also provide original filenames\nfor merge commits, add a --combined-with-paths option (which is only\nuseful in conjunction with -c, --raw, and -M and thus implies all\nthose options) so that we can print tab-separated filenames when\nrenames are involved. This transforms the above output to:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n\nSigned-off-by: Elijah Newren \n---\n\nThe critical part of the patch is the few line change to\nshow_raw_diff(), the rest is plumbing to set that up.\n\nThis patch was based out of Peff's suggestion[1] to fix diff-tree to\ndo what I needed rather than bending fast-export to cover my usecase;\nI've been running with a hacky version of this patch for a while and\nfinally cleaned it up.\n\n[1] https://public-inbox.org/git/20181114071454.GB19904@sigill.intra.peff.net/\n\nAs an alternative, I considered perhaps trying to sell it as a bugfix\n(how often do people use -M, -c, and --raw together and have renames\nin merge commits -- can I just change the format to include the old\nnames), but was worried that since diff-tree is plumbing and that the\nformat was documented to not include original filename(s), that I'd be\nbreaking backward compatibility in an important way for someone and\nthus opted for a new flag to get the behavior I needed.\n\nI did struggle a bit to come up with a name for the option; if others\nhave better suggestions, I'm happy to switch.\n\n\nRange-diff:\n1: 29e9ddf532 = 1: 29e9ddf532 log,diff-tree: add --combined-with-paths options for merges with renames\n\n Documentation/diff-format.txt | 23 +++++++++++++---\n Documentation/git-diff-tree.txt | 9 +++++--\n Documentation/rev-list-options.txt | 5 ++++\n combine-diff.c | 42 ++++++++++++++++++++++++++----\n diff.h | 1 +\n revision.c | 7 +++++\n revision.h | 1 +\n 7 files changed, 78 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt\nindex cdcc17f0ad..6e6cfc5d56 100644\n--- a/Documentation/diff-format.txt\n+++ b/Documentation/diff-format.txt\n@@ -87,7 +87,7 @@ diff format for merges\n ----------------------\n \n \"git-diff-tree\", \"git-diff-files\" and \"git-diff --raw\"\n-can take `-c` or `--cc` option\n+can take `-c`, `--cc`, or `--combined-with-paths` option\n to generate diff output also for merge commits. The output differs\n from the format described above in the following way:\n \n@@ -95,12 +95,29 @@ from the format described above in the following way:\n . there are more \"src\" modes and \"src\" sha1\n . status is concatenated status characters for each parent\n . no optional \"score\" number\n-. single path, only for \"dst\"\n+. tab-separated pathname(s) of the file\n \n-Example:\n+For `-c` and `--cc`, only the destination or final path is shown even\n+if the file was renamed on any side of history. With\n+`--combined-with-paths`, the number of paths printed will be one more\n+than the number of 'R' characters in the concatenated status. For\n+each 'R' in the concatenated status characters, the original pathname\n+on that side of history will be shown, and the final path shown on the\n+line will be the path used in the merge.\n+\n+Examples for `-c` and `-cc`:\n+------------------------------------------------\n+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n+------------------------------------------------\n+\n+Examples for `--combined-with-paths`:\n \n ------------------------------------------------\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n ------------------------------------------------\n \n Note that 'combined diff' lists only files which were modified from\ndiff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt\nindex 2319b2b192..2d9ba382bc 100644\n--- a/Documentation/git-diff-tree.txt\n+++ b/Documentation/git-diff-tree.txt\n@@ -10,8 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git diff-tree' [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty]\n-\t [-t] [-r] [-c | --cc] [--root] []\n-\t [] [...]\n+\t [-t] [-r] [-c | --cc | --combined-with-paths] [--root]\n+\t [] [] [...]\n \n DESCRIPTION\n -----------\n@@ -108,6 +108,11 @@ include::pretty-options.txt[]\n \titself and the commit log message is not shown, just like in any other\n \t\"empty diff\" case.\n \n+--combined-with-paths::\n+\tThis flag is similar to -c, but modifies the raw output format for\n+\tmerges to also show the original paths when renames are found.\n+\tImplies\t-c, -M, and --raw.\n+\n --always::\n \tShow the commit itself and the commit log message even\n \tif the diff itself is empty.\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 98b538bc77..4fdcaef52d 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -948,6 +948,11 @@ options may be given. See linkgit:git-diff-files[1] for more options.\n \tthe parents have only two variants and the merge result picks\n \tone of them without modification.\n \n+--combined-with-paths::\n+\tThis flag is similar to -c, but modifies the raw output format for\n+\tmerges to also show the original paths when renames are found.\n+\tImplies\t-c, -M, and --raw.\n+\n -m::\n \tThis flag makes the merge commits show the full diff like\n \tregular commits; for each merge parent, a separate log entry\ndiff --git a/combine-diff.c b/combine-diff.c\nindex a143c00634..77f7de14c9 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -23,11 +23,15 @@ static int compare_paths(const struct combine_diff_path *one,\n \t\t\t\t two->path, strlen(two->path), two->mode);\n }\n \n-static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n+static struct combine_diff_path *intersect_paths(\n+\tstruct combine_diff_path *curr,\n+\tint n,\n+\tint num_parent,\n+\tint combined_with_paths)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct combine_diff_path *p, **tail = &curr;\n-\tint i, cmp;\n+\tint i, j, cmp;\n \n \tif (!n) {\n \t\tfor (i = 0; i < q->nr; i++) {\n@@ -50,6 +54,13 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\t\tp->parent[n].status = q->queue[i]->status;\n+\n+\t\t\tif (combined_with_paths &&\n+\t\t\t p->parent[n].status == 'R') {\n+\t\t\t\tstrbuf_init(&p->parent[n].path, 0);\n+\t\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t\t q->queue[i]->one->path);\n+\t\t\t}\n \t\t\t*tail = p;\n \t\t\ttail = &p->next;\n \t\t}\n@@ -68,6 +79,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\tif (cmp < 0) {\n \t\t\t/* p->path not in q->queue[]; drop it */\n \t\t\t*tail = p->next;\n+\t\t\tfor (j = 0; j < num_parent; j++)\n+\t\t\t\tif (combined_with_paths &&\n+\t\t\t\t p->parent[j].status == 'R')\n+\t\t\t\t\tstrbuf_release(&p->parent[j].path);\n \t\t\tfree(p);\n \t\t\tcontinue;\n \t\t}\n@@ -81,6 +96,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\tp->parent[n].status = q->queue[i]->status;\n+\t\tif (combined_with_paths &&\n+\t\t p->parent[n].status == 'R')\n+\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t q->queue[i]->one->path);\n \n \t\ttail = &p->next;\n \t\ti++;\n@@ -1227,6 +1246,11 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re\n \t\tputchar(inter_name_termination);\n \t}\n \n+\tfor (i = 0; i < num_parent; i++)\n+\t\tif (rev->combined_with_paths &&\n+\t\t p->parent[i].status == 'R')\n+\t\t\twrite_name_quoted(p->parent[i].path.buf, stdout,\n+\t\t\t\t\t inter_name_termination);\n \twrite_name_quoted(p->path, stdout, line_termination);\n }\n \n@@ -1324,7 +1348,9 @@ static const char *path_path(void *obj)\n \n /* find set of paths that every parent touches */\n static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n-\tconst struct oid_array *parents, struct diff_options *opt)\n+\tconst struct oid_array *parents,\n+\tstruct diff_options *opt,\n+\tint combined_with_paths)\n {\n \tstruct combine_diff_path *paths = NULL;\n \tint i, num_parent = parents->nr;\n@@ -1350,7 +1376,8 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n \t\t\topt->output_format = DIFF_FORMAT_NO_OUTPUT;\n \t\tdiff_tree_oid(&parents->oid[i], oid, \"\", opt);\n \t\tdiffcore_std(opt);\n-\t\tpaths = intersect_paths(paths, i, num_parent);\n+\t\tpaths = intersect_paths(paths, i, num_parent,\n+\t\t\t\t\tcombined_with_paths);\n \n \t\t/* if showing diff, show it in requested order */\n \t\tif (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&\n@@ -1460,7 +1487,8 @@ void diff_tree_combined(const struct object_id *oid,\n \t\t * diff(sha1,parent_i) for all i to do the job, specifically\n \t\t * for parent0.\n \t\t */\n-\t\tpaths = find_paths_generic(oid, parents, &diffopts);\n+\t\tpaths = find_paths_generic(oid, parents, &diffopts,\n+\t\t\t\t\t rev->combined_with_paths);\n \t}\n \telse {\n \t\tint stat_opt;\n@@ -1535,6 +1563,10 @@ void diff_tree_combined(const struct object_id *oid,\n \twhile (paths) {\n \t\tstruct combine_diff_path *tmp = paths;\n \t\tpaths = paths->next;\n+\t\tfor (i = 0; i < num_parent; i++)\n+\t\t\tif (rev->combined_with_paths &&\n+\t\t\t tmp->parent[i].status == 'R')\n+\t\t\t\tstrbuf_release(&tmp->parent[i].path);\n \t\tfree(tmp);\n \t}\n \ndiff --git a/diff.h b/diff.h\nindex b512d0477a..90ea0256a5 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -294,6 +294,7 @@ struct combine_diff_path {\n \t\tchar status;\n \t\tunsigned int mode;\n \t\tstruct object_id oid;\n+\t\tstruct strbuf path;\n \t} parent[FLEX_ARRAY];\n };\n #define combine_diff_path_size(n, l) \\\ndiff --git a/revision.c b/revision.c\nindex 13cfb59b38..21c818ba11 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1995,6 +1995,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 0;\n \t\trevs->combine_merges = 1;\n+\t} else if (!strcmp(arg, \"--combined-with-paths\")) {\n+\t\trevs->diff = 1;\n+\t\trevs->dense_combined_merges = 0;\n+\t\trevs->combine_merges = 1;\n+\t\trevs->combined_with_paths = 1;\n+\t\trevs->diffopt.detect_rename = DIFF_DETECT_RENAME;\n+\t\trevs->diffopt.output_format |= DIFF_FORMAT_RAW;\n \t} else if (!strcmp(arg, \"--cc\")) {\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 1;\ndiff --git a/revision.h b/revision.h\nindex 52e5a88ff5..59a15b0ed0 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -171,6 +171,7 @@ struct rev_info {\n \t\t\tverbose_header:1,\n \t\t\tignore_merges:1,\n \t\t\tcombine_merges:1,\n+\t\t\tcombined_with_paths:1,\n \t\t\tdense_combined_merges:1,\n \t\t\talways_show_header:1;\n \n-- \n2.20.1.310.g29e9ddf532\n\n"},{"id":"367636","messageId":"20190125021950.GV423984@genre.crustytoothpaste.net","threadId":"50310","inReplyTo":"20190124164654.1923-1-newren@gmail.com","subject":"Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-01-25T02:19:50Z","receivedAt":"2019-01-25T02:19:59Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Jan 24, 2019 at 08:46:54AM -0800, Elijah Newren wrote:\n> The critical part of the patch is the few line change to\n> show_raw_diff(), the rest is plumbing to set that up.\n> \n> This patch was based out of Peff's suggestion[1] to fix diff-tree to\n> do what I needed rather than bending fast-export to cover my usecase;\n> I've been running with a hacky version of this patch for a while and\n> finally cleaned it up.\n> \n> [1] https://public-inbox.org/git/20181114071454.GB19904@sigill.intra.peff.net/\n> \n> As an alternative, I considered perhaps trying to sell it as a bugfix\n> (how often do people use -M, -c, and --raw together and have renames\n> in merge commits -- can I just change the format to include the old\n> names), but was worried that since diff-tree is plumbing and that the\n> format was documented to not include original filename(s), that I'd be\n> breaking backward compatibility in an important way for someone and\n> thus opted for a new flag to get the behavior I needed.\n> \n> I did struggle a bit to come up with a name for the option; if others\n> have better suggestions, I'm happy to switch.\n\nMaybe --all-names? You should definitely let other people chime in on\nthis as well; it should be obvious by now that I'm no expert on naming\nthings.\n\n> Range-diff:\n> 1: 29e9ddf532 = 1: 29e9ddf532 log,diff-tree: add --combined-with-paths options for merges with renames\n> \n> Documentation/diff-format.txt | 23 +++++++++++++---\n> Documentation/git-diff-tree.txt | 9 +++++--\n> Documentation/rev-list-options.txt | 5 ++++\n> combine-diff.c | 42 ++++++++++++++++++++++++++----\n> diff.h | 1 +\n> revision.c | 7 +++++\n> revision.h | 1 +\n> 7 files changed, 78 insertions(+), 10 deletions(-)\n\nI think it might be nice to see a test for this option so that we avoid\nbreaking it in the future. I'm also curious how this works with -z, and\na test for that would be interesting as well (as well as illustrative of\nthe format). For example, is it still unambiguous for machine parsing,\neven with oddly named files?\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"367662","messageId":"e9dd9394-e34d-e948-9a9c-83338b672d65@gmail.com","threadId":"50310","inReplyTo":"20190124164654.1923-1-newren@gmail.com","subject":"Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-01-25T14:45:27Z","receivedAt":"2019-01-25T14:45:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/24/2019 11:46 AM, Elijah Newren wrote:\n> As an alternative, I considered perhaps trying to sell it as a bugfix\n> (how often do people use -M, -c, and --raw together and have renames\n> in merge commits -- can I just change the format to include the old\n> names), but was worried that since diff-tree is plumbing and that the\n> format was documented to not include original filename(s), that I'd be\n> breaking backward compatibility in an important way for someone and\n> thus opted for a new flag to get the behavior I needed.\n\nThis is wise. Changing the format is likely to break at least one GUI\nclient or tool, especially because many depend on the installed version\nof Git instead of shipping with one \"blessed\" client.\n\nIn addition, there may be whitespace in the filenames. It appears the\ndiff format separates the filenames with tab characters. Is it\npossible to have tab character in a file name? That would make the output\nambiguous, but is no worse than our current output in a non-combined\ndiff.\n\nI'll repeat Brian's request for tests. I trust the compiler and the\ntest suite more than I trust my ability to read code.\n\nThanks,\n-Stolee\n\n"},{"id":"367665","messageId":"CABPp-BH-7iuSYmxA080UUbuKSEy32-T58oDKNajmFyeFWfauEA@mail.gmail.com","threadId":"50310","inReplyTo":"20190125021950.GV423984@genre.crustytoothpaste.net","subject":"Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-01-25T16:27:36Z","receivedAt":"2019-01-25T16:27:51Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Thu, Jan 24, 2019 at 6:19 PM brian m. carlson\n wrote:\n>\n> On Thu, Jan 24, 2019 at 08:46:54AM -0800, Elijah Newren wrote:\n> > The critical part of the patch is the few line change to\n> > show_raw_diff(), the rest is plumbing to set that up.\n> >\n> > This patch was based out of Peff's suggestion[1] to fix diff-tree to\n> > do what I needed rather than bending fast-export to cover my usecase;\n> > I've been running with a hacky version of this patch for a while and\n> > finally cleaned it up.\n> >\n> > [1] https://public-inbox.org/git/20181114071454.GB19904@sigill.intra.peff.net/\n> >\n> > As an alternative, I considered perhaps trying to sell it as a bugfix\n> > (how often do people use -M, -c, and --raw together and have renames\n> > in merge commits -- can I just change the format to include the old\n> > names), but was worried that since diff-tree is plumbing and that the\n> > format was documented to not include original filename(s), that I'd be\n> > breaking backward compatibility in an important way for someone and\n> > thus opted for a new flag to get the behavior I needed.\n> >\n> > I did struggle a bit to come up with a name for the option; if others\n> > have better suggestions, I'm happy to switch.\n>\n> Maybe --all-names? You should definitely let other people chime in on\n> this as well; it should be obvious by now that I'm no expert on naming\n> things.\n\n--all-names may work. Two possible worries:\n\n1) Would people expect the output when using this option to be of the form:\n\n::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c\ndescribe.c describe.c\n\nIn other words, even if there are no renames involved, would they\nexpect us to show the filename from each side of the merge? I was\ntrying to make the combined diff output format more similar to how we\nhandle non-merge commits; we only show more than one filename when a\nrename is involved.\n\n2) When looking through manpages I have often found an option that I\nthought was related to what I wanted, only to discover that a short\nsemi-ambiguous option had a much different context in mind than I did\nand thus performed some operation entirely unrelated to anything I was\ninterested in. I suspect that this won't be a highly used option, as\nsuch, a longer name to disambiguate seemed reasonable.\n\n\nOf course --combined-with-paths isn't fully descriptive either, and\nmaking it even longer might be annoying, so... *shrug*\n\n>\n> > Range-diff:\n> > 1: 29e9ddf532 = 1: 29e9ddf532 log,diff-tree: add --combined-with-paths options for merges with renames\n> >\n> > Documentation/diff-format.txt | 23 +++++++++++++---\n> > Documentation/git-diff-tree.txt | 9 +++++--\n> > Documentation/rev-list-options.txt | 5 ++++\n> > combine-diff.c | 42 ++++++++++++++++++++++++++----\n> > diff.h | 1 +\n> > revision.c | 7 +++++\n> > revision.h | 1 +\n> > 7 files changed, 78 insertions(+), 10 deletions(-)\n>\n> I think it might be nice to see a test for this option so that we avoid\n> breaking it in the future. I'm also curious how this works with -z, and\n> a test for that would be interesting as well (as well as illustrative of\n> the format). For example, is it still unambiguous for machine parsing,\n> even with oddly named files?\n\nI'll add a test for both with and without -z.\n"},{"id":"367666","messageId":"CABPp-BE9qFHGmJ-qO4Bx+wmO8N7pO_hdros5Q2hxd32=zU0F5A@mail.gmail.com","threadId":"50310","inReplyTo":"e9dd9394-e34d-e948-9a9c-83338b672d65@gmail.com","subject":"Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-01-25T16:30:23Z","receivedAt":"2019-01-25T16:30:37Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Fri, Jan 25, 2019 at 6:45 AM Derrick Stolee wrote:\n>\n> On 1/24/2019 11:46 AM, Elijah Newren wrote:\n> > As an alternative, I considered perhaps trying to sell it as a bugfix\n> > (how often do people use -M, -c, and --raw together and have renames\n> > in merge commits -- can I just change the format to include the old\n> > names), but was worried that since diff-tree is plumbing and that the\n> > format was documented to not include original filename(s), that I'd be\n> > breaking backward compatibility in an important way for someone and\n> > thus opted for a new flag to get the behavior I needed.\n>\n> This is wise. Changing the format is likely to break at least one GUI\n> client or tool, especially because many depend on the installed version\n> of Git instead of shipping with one \"blessed\" client.\n>\n> In addition, there may be whitespace in the filenames. It appears the\n> diff format separates the filenames with tab characters. Is it\n> possible to have tab character in a file name? That would make the output\n> ambiguous, but is no worse than our current output in a non-combined\n> diff.\n\nNo, it's actually unambiguous with or without the -z option. Without\n-z, pathnames with \"unusual\" characters (whose definition depends on\ncore.quotePath) will cause the pathname to be enclosed in\ndouble-quotes with the unusual characters handed with backslashes and\ncontrol characters. With -z, it separates pathnames with a nul\ncharacter instead.\n\n> I'll repeat Brian's request for tests. I trust the compiler and the\n> test suite more than I trust my ability to read code.\n\nYep, I'll add a pair and use filenames with multiple tabs.\n"},{"id":"367667","messageId":"20190125165416.17473-1-newren@gmail.com","threadId":"50310","inReplyTo":"20190124164654.1923-1-newren@gmail.com","subject":"[PATCH v2] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-01-25T16:54:16Z","receivedAt":"2019-01-25T17:12:23Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The raw diff format for merges with -c or --cc will only list one\nfilename, even if rename detection is active and a rename was detected\nfor the given path. Examples:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n\nThis doesn't let us know what the original name of bar.sh was in the\nfirst parent, and doesn't let us know what either of the original\nnames of phooey.c were in either of the parents. In contrast, for\nnon-merge commits, raw format does provide original filenames (and a\nrename score to boot). In order to also provide original filenames\nfor merge commits, add a --combined-with-paths option (which is only\nuseful in conjunction with -c, --raw, and -M and thus implies all\nthose options) so that we can print tab-separated filenames when\nrenames are involved. This transforms the above output to:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n\nSigned-off-by: Elijah Newren \n---\n\nDifferences since v1: added a testcase, with and without -z\n\n Documentation/diff-format.txt | 23 +++++++++++++---\n Documentation/git-diff-tree.txt | 9 +++++--\n Documentation/rev-list-options.txt | 5 ++++\n combine-diff.c | 42 ++++++++++++++++++++++++++----\n diff.h | 1 +\n revision.c | 7 +++++\n revision.h | 1 +\n t/t4038-diff-combined.sh | 36 +++++++++++++++++++++++++\n 8 files changed, 114 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt\nindex cdcc17f0ad..6e6cfc5d56 100644\n--- a/Documentation/diff-format.txt\n+++ b/Documentation/diff-format.txt\n@@ -87,7 +87,7 @@ diff format for merges\n ----------------------\n \n \"git-diff-tree\", \"git-diff-files\" and \"git-diff --raw\"\n-can take `-c` or `--cc` option\n+can take `-c`, `--cc`, or `--combined-with-paths` option\n to generate diff output also for merge commits. The output differs\n from the format described above in the following way:\n \n@@ -95,12 +95,29 @@ from the format described above in the following way:\n . there are more \"src\" modes and \"src\" sha1\n . status is concatenated status characters for each parent\n . no optional \"score\" number\n-. single path, only for \"dst\"\n+. tab-separated pathname(s) of the file\n \n-Example:\n+For `-c` and `--cc`, only the destination or final path is shown even\n+if the file was renamed on any side of history. With\n+`--combined-with-paths`, the number of paths printed will be one more\n+than the number of 'R' characters in the concatenated status. For\n+each 'R' in the concatenated status characters, the original pathname\n+on that side of history will be shown, and the final path shown on the\n+line will be the path used in the merge.\n+\n+Examples for `-c` and `-cc`:\n+------------------------------------------------\n+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n+------------------------------------------------\n+\n+Examples for `--combined-with-paths`:\n \n ------------------------------------------------\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n ------------------------------------------------\n \n Note that 'combined diff' lists only files which were modified from\ndiff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt\nindex 2319b2b192..2d9ba382bc 100644\n--- a/Documentation/git-diff-tree.txt\n+++ b/Documentation/git-diff-tree.txt\n@@ -10,8 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git diff-tree' [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty]\n-\t [-t] [-r] [-c | --cc] [--root] []\n-\t [] [...]\n+\t [-t] [-r] [-c | --cc | --combined-with-paths] [--root]\n+\t [] [] [...]\n \n DESCRIPTION\n -----------\n@@ -108,6 +108,11 @@ include::pretty-options.txt[]\n \titself and the commit log message is not shown, just like in any other\n \t\"empty diff\" case.\n \n+--combined-with-paths::\n+\tThis flag is similar to -c, but modifies the raw output format for\n+\tmerges to also show the original paths when renames are found.\n+\tImplies\t-c, -M, and --raw.\n+\n --always::\n \tShow the commit itself and the commit log message even\n \tif the diff itself is empty.\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 98b538bc77..4fdcaef52d 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -948,6 +948,11 @@ options may be given. See linkgit:git-diff-files[1] for more options.\n \tthe parents have only two variants and the merge result picks\n \tone of them without modification.\n \n+--combined-with-paths::\n+\tThis flag is similar to -c, but modifies the raw output format for\n+\tmerges to also show the original paths when renames are found.\n+\tImplies\t-c, -M, and --raw.\n+\n -m::\n \tThis flag makes the merge commits show the full diff like\n \tregular commits; for each merge parent, a separate log entry\ndiff --git a/combine-diff.c b/combine-diff.c\nindex a143c00634..77f7de14c9 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -23,11 +23,15 @@ static int compare_paths(const struct combine_diff_path *one,\n \t\t\t\t two->path, strlen(two->path), two->mode);\n }\n \n-static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n+static struct combine_diff_path *intersect_paths(\n+\tstruct combine_diff_path *curr,\n+\tint n,\n+\tint num_parent,\n+\tint combined_with_paths)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct combine_diff_path *p, **tail = &curr;\n-\tint i, cmp;\n+\tint i, j, cmp;\n \n \tif (!n) {\n \t\tfor (i = 0; i < q->nr; i++) {\n@@ -50,6 +54,13 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\t\tp->parent[n].status = q->queue[i]->status;\n+\n+\t\t\tif (combined_with_paths &&\n+\t\t\t p->parent[n].status == 'R') {\n+\t\t\t\tstrbuf_init(&p->parent[n].path, 0);\n+\t\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t\t q->queue[i]->one->path);\n+\t\t\t}\n \t\t\t*tail = p;\n \t\t\ttail = &p->next;\n \t\t}\n@@ -68,6 +79,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\tif (cmp < 0) {\n \t\t\t/* p->path not in q->queue[]; drop it */\n \t\t\t*tail = p->next;\n+\t\t\tfor (j = 0; j < num_parent; j++)\n+\t\t\t\tif (combined_with_paths &&\n+\t\t\t\t p->parent[j].status == 'R')\n+\t\t\t\t\tstrbuf_release(&p->parent[j].path);\n \t\t\tfree(p);\n \t\t\tcontinue;\n \t\t}\n@@ -81,6 +96,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\tp->parent[n].status = q->queue[i]->status;\n+\t\tif (combined_with_paths &&\n+\t\t p->parent[n].status == 'R')\n+\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t q->queue[i]->one->path);\n \n \t\ttail = &p->next;\n \t\ti++;\n@@ -1227,6 +1246,11 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re\n \t\tputchar(inter_name_termination);\n \t}\n \n+\tfor (i = 0; i < num_parent; i++)\n+\t\tif (rev->combined_with_paths &&\n+\t\t p->parent[i].status == 'R')\n+\t\t\twrite_name_quoted(p->parent[i].path.buf, stdout,\n+\t\t\t\t\t inter_name_termination);\n \twrite_name_quoted(p->path, stdout, line_termination);\n }\n \n@@ -1324,7 +1348,9 @@ static const char *path_path(void *obj)\n \n /* find set of paths that every parent touches */\n static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n-\tconst struct oid_array *parents, struct diff_options *opt)\n+\tconst struct oid_array *parents,\n+\tstruct diff_options *opt,\n+\tint combined_with_paths)\n {\n \tstruct combine_diff_path *paths = NULL;\n \tint i, num_parent = parents->nr;\n@@ -1350,7 +1376,8 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n \t\t\topt->output_format = DIFF_FORMAT_NO_OUTPUT;\n \t\tdiff_tree_oid(&parents->oid[i], oid, \"\", opt);\n \t\tdiffcore_std(opt);\n-\t\tpaths = intersect_paths(paths, i, num_parent);\n+\t\tpaths = intersect_paths(paths, i, num_parent,\n+\t\t\t\t\tcombined_with_paths);\n \n \t\t/* if showing diff, show it in requested order */\n \t\tif (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&\n@@ -1460,7 +1487,8 @@ void diff_tree_combined(const struct object_id *oid,\n \t\t * diff(sha1,parent_i) for all i to do the job, specifically\n \t\t * for parent0.\n \t\t */\n-\t\tpaths = find_paths_generic(oid, parents, &diffopts);\n+\t\tpaths = find_paths_generic(oid, parents, &diffopts,\n+\t\t\t\t\t rev->combined_with_paths);\n \t}\n \telse {\n \t\tint stat_opt;\n@@ -1535,6 +1563,10 @@ void diff_tree_combined(const struct object_id *oid,\n \twhile (paths) {\n \t\tstruct combine_diff_path *tmp = paths;\n \t\tpaths = paths->next;\n+\t\tfor (i = 0; i < num_parent; i++)\n+\t\t\tif (rev->combined_with_paths &&\n+\t\t\t tmp->parent[i].status == 'R')\n+\t\t\t\tstrbuf_release(&tmp->parent[i].path);\n \t\tfree(tmp);\n \t}\n \ndiff --git a/diff.h b/diff.h\nindex b512d0477a..90ea0256a5 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -294,6 +294,7 @@ struct combine_diff_path {\n \t\tchar status;\n \t\tunsigned int mode;\n \t\tstruct object_id oid;\n+\t\tstruct strbuf path;\n \t} parent[FLEX_ARRAY];\n };\n #define combine_diff_path_size(n, l) \\\ndiff --git a/revision.c b/revision.c\nindex 13cfb59b38..21c818ba11 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1995,6 +1995,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 0;\n \t\trevs->combine_merges = 1;\n+\t} else if (!strcmp(arg, \"--combined-with-paths\")) {\n+\t\trevs->diff = 1;\n+\t\trevs->dense_combined_merges = 0;\n+\t\trevs->combine_merges = 1;\n+\t\trevs->combined_with_paths = 1;\n+\t\trevs->diffopt.detect_rename = DIFF_DETECT_RENAME;\n+\t\trevs->diffopt.output_format |= DIFF_FORMAT_RAW;\n \t} else if (!strcmp(arg, \"--cc\")) {\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 1;\ndiff --git a/revision.h b/revision.h\nindex 52e5a88ff5..59a15b0ed0 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -171,6 +171,7 @@ struct rev_info {\n \t\t\tverbose_header:1,\n \t\t\tignore_merges:1,\n \t\t\tcombine_merges:1,\n+\t\t\tcombined_with_paths:1,\n \t\t\tdense_combined_merges:1,\n \t\t\talways_show_header:1;\n \ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex e2824d3437..9420c1318a 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -435,4 +435,40 @@ test_expect_success 'combine diff gets tree sorting right' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup for --combined-with-paths' '\n+\tgit branch side1c &&\n+\tgit branch side2c &&\n+\tgit checkout side1c &&\n+\ttest_seq 1 10 >$(printf \"file\\twith\\ttabs\") &&\n+\tgit add file* &&\n+\tgit commit -m with &&\n+\tgit checkout side2c &&\n+\ttest_seq 1 9 >$(printf \"i\\tam\\ttabbed\") &&\n+\techo ten >>$(printf \"i\\tam\\ttabbed\") &&\n+\tgit add *tabbed &&\n+\tgit commit -m iam &&\n+\tgit checkout -b mergery side1c &&\n+\tgit merge --no-commit side2c &&\n+\tgit rm *tabs &&\n+\techo eleven >>$(printf \"i\\tam\\ttabbed\") &&\n+\tgit mv \"$(printf \"i\\tam\\ttabbed\")\" \"$(printf \"fickle\\tnaming\")\" &&\n+\tgit add fickle* &&\n+\tgit commit\n+'\n+\n+test_expect_success '--combined-with-paths shows all paths involved' '\n+\tcat <<-\\EOF >expect &&\n+\t::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\t\"file\\twith\\ttabs\"\t\"i\\tam\\ttabbed\"\t\"fickle\\tnaming\"\n+\tEOF\n+\tgit diff-tree --combined-with-paths HEAD >actual.tmp &&\n+\tsed 1d actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--combined-with-paths works with -z as well' '\n+\tprintf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n+\tgit diff-tree --combined-with-paths -z HEAD >actual &&\n+\ttest_cmp -a expect actual\n+'\n+\n test_done\n-- \n2.20.1.310.g29e9ddf532\n\n"},{"id":"367669","messageId":"2f785c2d-161c-b6a3-7743-b071969d60d7@gmail.com","threadId":"50310","inReplyTo":"20190125165416.17473-1-newren@gmail.com","subject":"Re: [PATCH v2] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-01-25T17:40:58Z","receivedAt":"2019-01-25T17:41:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/25/2019 11:54 AM, Elijah Newren wrote:\n> +test_expect_success '--combined-with-paths works with -z as well' '\n> +\tprintf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n\nI'm guessing that you use printf here because the\n'cat <<-\\EOF' approach doesn't work with the special\ntabs? Kudos for putting in the extra effort here for\nthe special formatting!\n\nThe rest of the patch looks good.\n\nThanks,\n-Stolee\n"},{"id":"367670","messageId":"CABPp-BHTsZBO11ykfTaZ_dzpvrww5D3A57qBM05EgP_wK-iM9g@mail.gmail.com","threadId":"50310","inReplyTo":"2f785c2d-161c-b6a3-7743-b071969d60d7@gmail.com","subject":"Re: [PATCH v2] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-01-25T17:52:07Z","receivedAt":"2019-01-25T17:52:25Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 25, 2019 at 9:41 AM Derrick Stolee wrote:\n>\n> On 1/25/2019 11:54 AM, Elijah Newren wrote:\n> > +test_expect_success '--combined-with-paths works with -z as well' '\n> > + printf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n>\n> I'm guessing that you use printf here because the\n> 'cat <<-\\EOF' approach doesn't work with the special\n> tabs? Kudos for putting in the extra effort here for\n> the special formatting!\n\nYeah, I didn't know how to easily get NUL bytes in the stream without\nprintf, and once I was using printf the EOF HEREDOC no longer had a\nuseful purpose. In the first testcase, since there were only\nprintable characters in the expected output, a HEREDOC worked well. I\nguess I could have just used printf for both testcases, but having the\nliteral output shown where it's possible for a human to read it seemed\nlike an advantage worth capitalizing on. If anyone feels strongly\nthat I should use printf for both tests, I can switch them though.\n\n> The rest of the patch looks good.\n\nThanks for taking a look!\n"},{"id":"367676","messageId":"xmqqmunofsdv.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"20190124164654.1923-1-newren@gmail.com","subject":"Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-25T19:29:16Z","receivedAt":"2019-01-25T19:29:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> The raw diff format for merges with -c or --cc will only list one\n> filename, even if rename detection is active and a rename was detected\n> for the given path. Examples:\n>\n> ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n> ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n> ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n>\n> This doesn't let us know what the original name of bar.sh was in the\n> first parent, and doesn't let us know what either of the original\n> names of phooey.c were in either of the parents. In contrast, for\n> non-merge commits, raw format does provide original filenames (and a\n> rename score to boot). In order to also provide original filenames\n> for merge commits, add a --combined-with-paths option (which is only\n> useful in conjunction with -c, --raw, and -M and thus implies all\n> those options) so that we can print tab-separated filenames when\n> renames are involved. This transforms the above output to:\n>\n> ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n> ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\n> ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n\nI admit that I designed the original without too much thought.\nPerhaps we should have avoided discarding info, but it is way too\nlate to fix with a default behaviour change.\n\nI am not sure if it is easy for consumers to guess which name on the\noutput line corresponds to which input tree from the status letter,\nthough. Would it make it easier for consumers if this showed names\nin all input trees if any of them is different from the name in the\nresulting tree, I wonder? Even in that case, the consumer must know\nsome rule like \"if R or C appears in the status column, then we have\nN preimage names plus the name in the result for N-way merge\", so it\nmay not be too bad to force them to know \"for each of R or C in the\nstatus column, the name in the preimage tree is emitted, and the\nlast name is the name in the result\". I dunno.\n\n> +For `-c` and `--cc`, only the destination or final path is shown even\n> +if the file was renamed on any side of history. With\n> +`--combined-with-paths`, the number of paths printed will be one more\n> +than the number of 'R' characters in the concatenated status. For\n> +each 'R' in the concatenated status characters, the original pathname\n> +on that side of history will be shown, and the final path shown on the\n> +line will be the path used in the merge.\n\nIs it safe for readers to pay attention to only 'R'? Will it stay\nforever that way? My immediate worry is 'C', but there might be\nother cases that original and result have different names.\n\n> +--combined-with-paths::\n> +\tThis flag is similar to -c, but modifies the raw output format for\n> +\tmerges to also show the original paths when renames are found.\n> +\tImplies\t-c, -M, and --raw.\n\nSo, --cc -p is not allowed to use this? I was wondering if we want\nto have a separate \"even though traditionally we did not show\npreimage names in combined output, this option tells Git to do so,\nregardless of output format used, as long as 'combine-diff' is in\neffect\".\n"},{"id":"367679","messageId":"CAPig+cRefjWr3gNOfQou9JyW=yfACEvbXPPRzXU+VkJN_gCTzA@mail.gmail.com","threadId":"50310","inReplyTo":"CABPp-BHTsZBO11ykfTaZ_dzpvrww5D3A57qBM05EgP_wK-iM9g@mail.gmail.com","subject":"Re: [PATCH v2] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-01-25T19:51:45Z","receivedAt":"2019-01-25T19:51:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 25, 2019 at 12:52 PM Elijah Newren wrote:\n> On Fri, Jan 25, 2019 at 9:41 AM Derrick Stolee wrote:\n> > On 1/25/2019 11:54 AM, Elijah Newren wrote:\n> > > + printf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n> >\n> > I'm guessing that you use printf here because the\n> > 'cat <<-\\EOF' approach doesn't work with the special\n> > tabs? Kudos for putting in the extra effort here for\n> > the special formatting!\n>\n> Yeah, I didn't know how to easily get NUL bytes in the stream without\n> printf, and once I was using printf the EOF HEREDOC no longer had a\n> useful purpose. In the first testcase, since there were only\n> printable characters in the expected output, a HEREDOC worked well. I\n> guess I could have just used printf for both testcases, but having the\n> literal output shown where it's possible for a human to read it seemed\n> like an advantage worth capitalizing on.\n\nIf the readability of a here-doc is preferred, you should be able to\nachieve the desired result with the q_to_tab() and lf_to_nul()\nfunctions. For instance:\n\n q_to_tab <<-\\EOF | lf_to_nul >expect &&\n ...Q...Q...\n EOF\n"},{"id":"367682","messageId":"CABPp-BFjUgY5htSbPpnamPDRM3bCLNsQv-K35uXoRzaT2f85tA@mail.gmail.com","threadId":"50310","inReplyTo":"xmqqmunofsdv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-01-25T20:04:13Z","receivedAt":"2019-01-25T20:04:28Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 25, 2019 at 11:29 AM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > The raw diff format for merges with -c or --cc will only list one\n> > filename, even if rename detection is active and a rename was detected\n> > for the given path. Examples:\n> >\n> > ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c\n> > ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM bar.sh\n> > ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR phooey.c\n> >\n> > This doesn't let us know what the original name of bar.sh was in the\n> > first parent, and doesn't let us know what either of the original\n> > names of phooey.c were in either of the parents. In contrast, for\n> > non-merge commits, raw format does provide original filenames (and a\n> > rename score to boot). In order to also provide original filenames\n> > for merge commits, add a --combined-with-paths option (which is only\n> > useful in conjunction with -c, --raw, and -M and thus implies all\n> > those options) so that we can print tab-separated filenames when\n> > renames are involved. This transforms the above output to:\n> >\n> > ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c\n> > ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM foo.sh bar.sh\n> > ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR fooey.c fuey.c phooey.c\n>\n> I admit that I designed the original without too much thought.\n> Perhaps we should have avoided discarding info, but it is way too\n> late to fix with a default behaviour change.\n>\n> I am not sure if it is easy for consumers to guess which name on the\n> output line corresponds to which input tree from the status letter,\n> though. Would it make it easier for consumers if this showed names\n> in all input trees if any of them is different from the name in the\n> resulting tree, I wonder? Even in that case, the consumer must know\n> some rule like \"if R or C appears in the status column, then we have\n> N preimage names plus the name in the result for N-way merge\", so it\n> may not be too bad to force them to know \"for each of R or C in the\n> status column, the name in the preimage tree is emitted, and the\n> last name is the name in the result\". I dunno.\n\nI wasn't able to guess the other fields in the raw format (\"which of\nthe three blob shas is the final one and which ones were for parents\")\nwithout going and reading the diff-format.txt documentation. Unless\nwe always list all names on all lines (even e.g. for 'MM' changes\nwhich will list the same filename three times), we have a more\ncomplicated case where people will have to refer to that\ndocumentation. I hope the extra paragraphs and examples I added there\nare sufficient to spell it out.\n\nAlso, my first version of the patch actually showed all names, on all\nlines, but I found the heavy repetition really annoying, and not in\nkeeping with how non-merge commits are handled (where original\nfilenames are only shown when they differ). Granted, my change isn't\nthe only one. We could just have all names shown if they are not all\nidentical, as you suggest and I also considered, but I liked this way\nslightly better. If others feel strongly, I can change it, that was\njust my gut feel and preference.\n\n> > +For `-c` and `--cc`, only the destination or final path is shown even\n> > +if the file was renamed on any side of history. With\n> > +`--combined-with-paths`, the number of paths printed will be one more\n> > +than the number of 'R' characters in the concatenated status. For\n> > +each 'R' in the concatenated status characters, the original pathname\n> > +on that side of history will be shown, and the final path shown on the\n> > +line will be the path used in the merge.\n>\n> Is it safe for readers to pay attention to only 'R'? Will it stay\n> forever that way? My immediate worry is 'C', but there might be\n> other cases that original and result have different names.\n\nOops, yeah, it should also handle 'C' and be worded so that if any\nfuture change type comes along involving different names then it'd be\nincluded.\n\n> > +--combined-with-paths::\n> > + This flag is similar to -c, but modifies the raw output format for\n> > + merges to also show the original paths when renames are found.\n> > + Implies -c, -M, and --raw.\n>\n> So, --cc -p is not allowed to use this? I was wondering if we want\n> to have a separate \"even though traditionally we did not show\n> preimage names in combined output, this option tells Git to do so,\n> regardless of output format used, as long as 'combine-diff' is in\n> effect\".\n\nYou could kind of ask the same question of -c -p, actually. I looked\ninto that, but I was only interested in raw format output and --cc is\nonly about coalescing uninteresting hunks in patches. Whenever git\nshows a combined diff in patch format, it always lists two files in\nthe header, e.g.:\n a/foo.c\n b/foo.c\nperhaps because people have a built-in expectation that a diff has to\ninvolve exactly two files. I wasn't sure how hard-baked in that\nassumption is, but as I was only interested in raw format I didn't\nmess with it. We'd have to switch to showing three or more if we want\nthis to be relevant to such a mode. Would that make sense for users\nto show in a patch? I guess the \"combined\" patch is already kind of\nspecial, so it could make sense, but it kind of feels like a follow-on\nfeature for someone else to implement...unless leaving it out now\nsomehow boxes us in.\n"},{"id":"367691","messageId":"xmqq8sz8fkfi.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"CABPp-BFjUgY5htSbPpnamPDRM3bCLNsQv-K35uXoRzaT2f85tA@mail.gmail.com","subject":"Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-25T22:21:05Z","receivedAt":"2019-01-25T22:21:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> Also, my first version of the patch actually showed all names, on all\n> lines, but I found the heavy repetition really annoying, and not in\n> keeping with how non-merge commits are handled (where original\n> filenames are only shown when they differ). Granted, my change isn't\n> the only one. We could just have all names shown if they are not all\n> identical, as you suggest and I also considered, but I liked this way\n> slightly better. If others feel strongly, I can change it, that was\n> just my gut feel and preference.\n\nI do not have a strong _preference_; I am only worried about ease of\nuse by consumers of the raw output, i.e. scripts. From that point\nof view, --all-names showing all names always even when there is no\nrename is involved would certainly be the easiest to write a parser\nfor, and lines being overly repetitive is a non-issue. For human\nconsumption, it would be a separate matter, but they won't be\nreading from the --raw output that lists blob object names\nrepetitively (the object names may or may not be different, but they\nare all equally gibberish) before showing the human-readble part,\nwhich are names.\n\n> You could kind of ask the same question of -c -p, actually. I looked\n> into that, but I was only interested in raw format output and --cc is\n> only about coalescing uninteresting hunks in patches. Whenever git\n> shows a combined diff in patch format, it always lists two files in\n> the header, e.g.:\n> a/foo.c\n> b/foo.c\n\nYeah, but with a new option, it does not have to stay that way. It\nis OK to show a/foo.c, b/bar.c, c/baz.c, or perhaps an easy way to\navoid overlong unreadable lines in these human readable output may\nbe to only show b/foo.c to report the result and list the original\nnames using the \"rename from\" \"rename to\" extended diff-headers\nshown in normal diff output with rename detection enabled.\n\n> ... I guess the \"combined\" patch is already kind of\n> special, so it could make sense, ...\n\nAbsolutely. -c/--cc output with -p departs from the usual \"diff is\nto compare two things\" by having two +/- indicator columns and more\nthan two '@'s in hunk header to begin with. It is only natural if\nwe are showing more/all names in --raw, we should avoid losing info\nin the same way.\n"},{"id":"367720","messageId":"CABPp-BHU9QUQrr9F9ncQSn-yeBhMUZiRO78jzX+C42zrg55tzA@mail.gmail.com","threadId":"50310","inReplyTo":"CAPig+cRefjWr3gNOfQou9JyW=yfACEvbXPPRzXU+VkJN_gCTzA@mail.gmail.com","subject":"Re: [PATCH v2] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-01-26T22:07:52Z","receivedAt":"2019-01-26T22:08:07Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 25, 2019 at 11:51 AM Eric Sunshine wrote:\n>\n> On Fri, Jan 25, 2019 at 12:52 PM Elijah Newren wrote:\n> > On Fri, Jan 25, 2019 at 9:41 AM Derrick Stolee wrote:\n> > > On 1/25/2019 11:54 AM, Elijah Newren wrote:\n> > > > + printf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n> > >\n> > > I'm guessing that you use printf here because the\n> > > 'cat <<-\\EOF' approach doesn't work with the special\n> > > tabs? Kudos for putting in the extra effort here for\n> > > the special formatting!\n> >\n> > Yeah, I didn't know how to easily get NUL bytes in the stream without\n> > printf, and once I was using printf the EOF HEREDOC no longer had a\n> > useful purpose. In the first testcase, since there were only\n> > printable characters in the expected output, a HEREDOC worked well. I\n> > guess I could have just used printf for both testcases, but having the\n> > literal output shown where it's possible for a human to read it seemed\n> > like an advantage worth capitalizing on.\n>\n> If the readability of a here-doc is preferred, you should be able to\n> achieve the desired result with the q_to_tab() and lf_to_nul()\n> functions. For instance:\n>\n> q_to_tab <<-\\EOF | lf_to_nul >expect &&\n> ...Q...Q...\n> EOF\n\nI like the idea and I tried it out...but it actually looks harder to read to me:\n\ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex 5bccc323f6..ddbd27d16a 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -457,8 +457,8 @@ test_expect_success 'setup for --combined-with-paths' '\n '\n\n test_expect_success '--combined-all-names and --raw' '\n- cat <<-\\EOF >expect &&\n- ::100644 100644 100644 f00c965d8307308469e537302baa73048488f162\n088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7\n333b9c62519f285e1854830ade0fe1ef1d40ee1b RR \"file\\twith\\ttabs\"\n\"i\\tam\\ttabbed\" \"fickle\\tnaming\"\n+ q_to_tab <<-\\EOF >expect &&\n+ ::100644 100644 100644 f00c965d8307308469e537302baa73048488f162\n088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7\n333b9c62519f285e1854830ade0fe1ef1d40ee1b\nRRQ\"file\\twith\\ttabs\"Q\"i\\tam\\ttabbed\"Q\"fickle\\tnaming\"\n EOF\n git diff-tree -c -M --raw --combined-all-names HEAD >actual.tmp &&\n sed 1d actual &&\n@@ -466,7 +466,13 @@ test_expect_success '--combined-all-names and --raw' '\n '\n\n test_expect_success '--combined-all-names and --raw -and -z' '\n- printf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644\n100644 f00c965d8307308469e537302baa73048488f162\n088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7\n333b9c62519f285e1854830ade0fe1ef1d40ee1b\nRR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n+ q_to_tab <<-\\EOF | lf_to_nul >expect &&\n+ 0f9645804ebb04cc3eef91f799eb7fb54d70cefb\n+ ::100644 100644 100644 f00c965d8307308469e537302baa73048488f162\n088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7\n333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\n+ fileQwithQtabs\n+ iQamQtabbed\n+ fickleQnaming\n+ EOF\n git diff-tree -c -M --raw --combined-all-names -z HEAD >actual &&\n test_cmp -a expect actual\n '\n\nSo I'm going to stick with the original. Thanks for pointing out\nq_to_tab and lf_to_nul, though; they may come in handy in the future.\n"},{"id":"367721","messageId":"CABPp-BFaVwwM17Oe+-RoP5wt2V58AD=dr01uNPuV=9r4yoF3Qg@mail.gmail.com","threadId":"50310","inReplyTo":"xmqq8sz8fkfi.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-01-26T22:12:41Z","receivedAt":"2019-01-26T22:12:55Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 25, 2019 at 2:21 PM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > Also, my first version of the patch actually showed all names, on all\n> > lines, but I found the heavy repetition really annoying, and not in\n> > keeping with how non-merge commits are handled (where original\n> > filenames are only shown when they differ). Granted, my change isn't\n> > the only one. We could just have all names shown if they are not all\n> > identical, as you suggest and I also considered, but I liked this way\n> > slightly better. If others feel strongly, I can change it, that was\n> > just my gut feel and preference.\n>\n> I do not have a strong _preference_; I am only worried about ease of\n> use by consumers of the raw output, i.e. scripts. From that point\n> of view, --all-names showing all names always even when there is no\n> rename is involved would certainly be the easiest to write a parser\n> for, and lines being overly repetitive is a non-issue. For human\n> consumption, it would be a separate matter, but they won't be\n> reading from the --raw output that lists blob object names\n> repetitively (the object names may or may not be different, but they\n> are all equally gibberish) before showing the human-readble part,\n> which are names.\n\nOkay, I've switched it over.\n\n> > You could kind of ask the same question of -c -p, actually. I looked\n> > into that, but I was only interested in raw format output and --cc is\n> > only about coalescing uninteresting hunks in patches. Whenever git\n> > shows a combined diff in patch format, it always lists two files in\n> > the header, e.g.:\n> > a/foo.c\n> > b/foo.c\n>\n> Yeah, but with a new option, it does not have to stay that way. It\n> is OK to show a/foo.c, b/bar.c, c/baz.c, or perhaps an easy way to\n> avoid overlong unreadable lines in these human readable output may\n> be to only show b/foo.c to report the result and list the original\n> names using the \"rename from\" \"rename to\" extended diff-headers\n> shown in normal diff output with rename detection enabled.\n>\n> > ... I guess the \"combined\" patch is already kind of\n> > special, so it could make sense, ...\n>\n> Absolutely. -c/--cc output with -p departs from the usual \"diff is\n> to compare two things\" by having two +/- indicator columns and more\n> than two '@'s in hunk header to begin with. It is only natural if\n> we are showing more/all names in --raw, we should avoid losing info\n> in the same way.\n\nThis turned out to be easier to implement than I expected, with one\nsmall wrinkle: We have the \"a/\" and \"b/\" prefixes, and switching to\n\"a/\", \"b/\" and \"c/\" doesn't work due to things like the\ndiff.mnemonicPrefix setting -- plus I didn't want to plumb multiple\nprefix options everywhere they'd be needed. Instead, I decided that\ninstead of having a \"from\" and a \"to\" header, we would have N \"from\"\nheaders (since all parents are what we came from) and 1 \"to\" header.\nI hope that makes sense, because otherwise I think this become to\nunwieldy of a change.\n"},{"id":"367722","messageId":"20190126221811.20241-1-newren@gmail.com","threadId":"50310","inReplyTo":"20190125165416.17473-1-newren@gmail.com","subject":"[PATCH v3] log,diff-tree: add --combined-all-names option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-01-26T22:18:11Z","receivedAt":"2019-01-26T22:18:41Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The combined diff format for merges will only list one filename, even if\nrename or copy detection is active. For example, with raw format one\nmight see:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n\nThis doesn't let us know what the original name of bar.sh was in the\nfirst parent, and doesn't let us know what either of the original names\nof phooey.c were in either of the parents. In contrast, for non-merge\ncommits, raw format does provide original filenames (and a rename score\nto boot). In order to also provide original filenames for merge\ncommits, add a --combined-all-names option (which must be used with\neither -c or --cc, and is likely only useful with rename or copy\ndetection active) so that we can print tab-separated filenames when\nrenames are involved. This transforms the above output to:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\tdesc.c\tdesc.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n\nFurther, in patch format, this changes the from/to headers so that\ninstead of just having one \"from\" header, we get one for each parent.\nFor example, instead of having\n\n --- a/phooey.c\n +++ b/phooey.c\n\nwe would see\n\n --- a/fooey.c\n --- a/fuey.c\n +++ b/phooey.c\n\nSigned-off-by: Elijah Newren \n---\nChanges since v2:\n * Renamed --combined-with-paths to --combined-all-names\n * No longer implies -c, --raw, or -M\n * Works with either -c or --cc, with either --raw or -p, and with\n either -M or -C\n * Always shows the name in each parent even if it doesn't differ\n * additional testcase checking patch format header with --cc\n\n Documentation/diff-format.txt | 20 +++++--\n Documentation/diff-generate-patch.txt | 13 +++++\n Documentation/git-diff-tree.txt | 11 +++-\n Documentation/rev-list-options.txt | 7 +++\n builtin/diff-tree.c | 6 ++-\n combine-diff.c | 76 +++++++++++++++++++++++----\n diff.h | 1 +\n revision.c | 6 +++\n revision.h | 1 +\n t/t4038-diff-combined.sh | 47 +++++++++++++++++\n 10 files changed, 171 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt\nindex cdcc17f0ad..8fe800cd9b 100644\n--- a/Documentation/diff-format.txt\n+++ b/Documentation/diff-format.txt\n@@ -95,12 +95,26 @@ from the format described above in the following way:\n . there are more \"src\" modes and \"src\" sha1\n . status is concatenated status characters for each parent\n . no optional \"score\" number\n-. single path, only for \"dst\"\n+. tab-separated pathname(s) of the file\n \n-Example:\n+For `-c` and `--cc`, only the destination or final path is shown even\n+if the file was renamed on any side of history. With\n+`--combined-all-names`, the name of the path in each parent is shown\n+followed by the name of the path in the merge commit.\n+\n+Examples for `-c` and `-cc` without `--combined-all-names`:\n+------------------------------------------------\n+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n+------------------------------------------------\n+\n+Examples when `--combined-all-names` added to either `-c` or `--cc`:\n \n ------------------------------------------------\n-::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\tdesc.c\tdesc.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n ------------------------------------------------\n \n Note that 'combined diff' lists only files which were modified from\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 231105cff4..69cb3b0349 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -144,6 +144,19 @@ Similar to two-line header for traditional 'unified' diff\n format, `/dev/null` is used to signal created or deleted\n files.\n \n+However, if the --combined-all-paths option is provided, instead of a\n+two-line from-file/to-file you get a N+1 line from-file/to-file header,\n+where N is the number of parents in the merge commit\n+\n+ --- a/file\n+ --- a/file\n+ --- a/file\n+ +++ b/file\n++\n+This extended format can be useful if rename or copy detection is\n+active, to allow you to see the original name of the file in different\n+parents.\n+\n 4. Chunk header format is modified to prevent people from\n accidentally feeding it to `patch -p1`. Combined diff format\n was created for review of merge commit changes, and was not\ndiff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt\nindex 2319b2b192..7b6dcbd7a8 100644\n--- a/Documentation/git-diff-tree.txt\n+++ b/Documentation/git-diff-tree.txt\n@@ -10,8 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git diff-tree' [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty]\n-\t [-t] [-r] [-c | --cc] [--root] []\n-\t [] [...]\n+\t [-t] [-r] [-c | --cc] [--combined-all-names] [--root]\n+\t [] [] [...]\n \n DESCRIPTION\n -----------\n@@ -108,6 +108,13 @@ include::pretty-options.txt[]\n \titself and the commit log message is not shown, just like in any other\n \t\"empty diff\" case.\n \n+--combined-all-names::\n+\tThis flag causes combined diffs (used for merge commits) to\n+\tlist the name of the file from all parents. It thus only has\n+\teffect when -c or --cc are specified, and is likely only\n+\tuseful if filename changes are detected (i.e. when either\n+\trename or copy detection have been requested).\n+\n --always::\n \tShow the commit itself and the commit log message even\n \tif the diff itself is empty.\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 98b538bc77..97a6651bae 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -948,6 +948,13 @@ options may be given. See linkgit:git-diff-files[1] for more options.\n \tthe parents have only two variants and the merge result picks\n \tone of them without modification.\n \n+--combined-all-names::\n+\tThis flag causes combined diffs (used for merge commits) to\n+\tlist the name of the file from all parents. It thus only has\n+\teffect when -c or --cc are specified, and is likely only\n+\tuseful if filename changes are detected (i.e. when either\n+\trename or copy detection have been requested).\n+\n -m::\n \tThis flag makes the merge commits show the full diff like\n \tregular commits; for each merge parent, a separate log entry\ndiff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\nindex ef996126d7..bbeebc833f 100644\n--- a/builtin/diff-tree.c\n+++ b/builtin/diff-tree.c\n@@ -82,9 +82,13 @@ static int diff_tree_stdin(char *line)\n }\n \n static const char diff_tree_usage[] =\n-\"git diff-tree [--stdin] [-m] [-c] [--cc] [-s] [-v] [--pretty] [-t] [-r] [--root] \"\n+\"git diff-tree [--stdin] [-m] [-c | --cc] [-s] [-v] [--pretty] [-t] [-r] [--root] \"\n \"[] [] [...]\\n\"\n \" -r diff recursively\\n\"\n+\" -c show combined diff for merge commits\\n\"\n+\" --cc show combined diff for merge commits removing uninteresting hunks\\n\"\n+\" --combined-all-names\\n\"\n+\" show name of file in all parents for combined diffs\\n\"\n \" --root include the initial commit as diff against /dev/null\\n\"\n COMMON_DIFF_OPTIONS_HELP;\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex a143c00634..339daac02b 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -23,11 +23,20 @@ static int compare_paths(const struct combine_diff_path *one,\n \t\t\t\t two->path, strlen(two->path), two->mode);\n }\n \n-static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n+static int filename_changed(char status)\n+{\n+\treturn status == 'R' || status == 'C';\n+}\n+\n+static struct combine_diff_path *intersect_paths(\n+\tstruct combine_diff_path *curr,\n+\tint n,\n+\tint num_parent,\n+\tint combined_all_names)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct combine_diff_path *p, **tail = &curr;\n-\tint i, cmp;\n+\tint i, j, cmp;\n \n \tif (!n) {\n \t\tfor (i = 0; i < q->nr; i++) {\n@@ -50,6 +59,13 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\t\tp->parent[n].status = q->queue[i]->status;\n+\n+\t\t\tif (combined_all_names &&\n+\t\t\t filename_changed(p->parent[n].status)) {\n+\t\t\t\tstrbuf_init(&p->parent[n].path, 0);\n+\t\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t\t q->queue[i]->one->path);\n+\t\t\t}\n \t\t\t*tail = p;\n \t\t\ttail = &p->next;\n \t\t}\n@@ -68,6 +84,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\tif (cmp < 0) {\n \t\t\t/* p->path not in q->queue[]; drop it */\n \t\t\t*tail = p->next;\n+\t\t\tfor (j = 0; j < num_parent; j++)\n+\t\t\t\tif (combined_all_names &&\n+\t\t\t\t filename_changed(p->parent[j].status))\n+\t\t\t\t\tstrbuf_release(&p->parent[j].path);\n \t\t\tfree(p);\n \t\t\tcontinue;\n \t\t}\n@@ -81,6 +101,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\tp->parent[n].status = q->queue[i]->status;\n+\t\tif (combined_all_names &&\n+\t\t filename_changed(p->parent[n].status))\n+\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t q->queue[i]->one->path);\n \n \t\ttail = &p->next;\n \t\ti++;\n@@ -960,12 +984,25 @@ static void show_combined_header(struct combine_diff_path *elem,\n \tif (!show_file_header)\n \t\treturn;\n \n-\tif (added)\n-\t\tdump_quoted_path(\"--- \", \"\", \"/dev/null\",\n-\t\t\t\t line_prefix, c_meta, c_reset);\n-\telse\n-\t\tdump_quoted_path(\"--- \", a_prefix, elem->path,\n-\t\t\t\t line_prefix, c_meta, c_reset);\n+\tif (rev->combined_all_names) {\n+\t\tfor (i = 0; i < num_parent; i++) {\n+\t\t\tchar *path = filename_changed(elem->parent[i].status)\n+\t\t\t\t? elem->parent[i].path.buf : elem->path;\n+\t\t\tif (elem->parent[i].status == DIFF_STATUS_ADDED)\n+\t\t\t\tdump_quoted_path(\"--- \", \"\", \"/dev/null\",\n+\t\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\t\telse\n+\t\t\t\tdump_quoted_path(\"--- \", a_prefix, path,\n+\t\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\t}\n+\t} else {\n+\t\tif (added)\n+\t\t\tdump_quoted_path(\"--- \", \"\", \"/dev/null\",\n+\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\telse\n+\t\t\tdump_quoted_path(\"--- \", a_prefix, elem->path,\n+\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t}\n \tif (deleted)\n \t\tdump_quoted_path(\"+++ \", \"\", \"/dev/null\",\n \t\t\t\t line_prefix, c_meta, c_reset);\n@@ -1227,6 +1264,15 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re\n \t\tputchar(inter_name_termination);\n \t}\n \n+\tfor (i = 0; i < num_parent; i++)\n+\t\tif (rev->combined_all_names) {\n+\t\t\tif (filename_changed(p->parent[i].status))\n+\t\t\t\twrite_name_quoted(p->parent[i].path.buf, stdout,\n+\t\t\t\t\t\t inter_name_termination);\n+\t\t\telse\n+\t\t\t\twrite_name_quoted(p->path, stdout,\n+\t\t\t\t\t\t inter_name_termination);\n+\t\t}\n \twrite_name_quoted(p->path, stdout, line_termination);\n }\n \n@@ -1324,7 +1370,9 @@ static const char *path_path(void *obj)\n \n /* find set of paths that every parent touches */\n static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n-\tconst struct oid_array *parents, struct diff_options *opt)\n+\tconst struct oid_array *parents,\n+\tstruct diff_options *opt,\n+\tint combined_all_names)\n {\n \tstruct combine_diff_path *paths = NULL;\n \tint i, num_parent = parents->nr;\n@@ -1350,7 +1398,8 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n \t\t\topt->output_format = DIFF_FORMAT_NO_OUTPUT;\n \t\tdiff_tree_oid(&parents->oid[i], oid, \"\", opt);\n \t\tdiffcore_std(opt);\n-\t\tpaths = intersect_paths(paths, i, num_parent);\n+\t\tpaths = intersect_paths(paths, i, num_parent,\n+\t\t\t\t\tcombined_all_names);\n \n \t\t/* if showing diff, show it in requested order */\n \t\tif (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&\n@@ -1460,7 +1509,8 @@ void diff_tree_combined(const struct object_id *oid,\n \t\t * diff(sha1,parent_i) for all i to do the job, specifically\n \t\t * for parent0.\n \t\t */\n-\t\tpaths = find_paths_generic(oid, parents, &diffopts);\n+\t\tpaths = find_paths_generic(oid, parents, &diffopts,\n+\t\t\t\t\t rev->combined_all_names);\n \t}\n \telse {\n \t\tint stat_opt;\n@@ -1535,6 +1585,10 @@ void diff_tree_combined(const struct object_id *oid,\n \twhile (paths) {\n \t\tstruct combine_diff_path *tmp = paths;\n \t\tpaths = paths->next;\n+\t\tfor (i = 0; i < num_parent; i++)\n+\t\t\tif (rev->combined_all_names &&\n+\t\t\t filename_changed(tmp->parent[i].status))\n+\t\t\t\tstrbuf_release(&tmp->parent[i].path);\n \t\tfree(tmp);\n \t}\n \ndiff --git a/diff.h b/diff.h\nindex b512d0477a..90ea0256a5 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -294,6 +294,7 @@ struct combine_diff_path {\n \t\tchar status;\n \t\tunsigned int mode;\n \t\tstruct object_id oid;\n+\t\tstruct strbuf path;\n \t} parent[FLEX_ARRAY];\n };\n #define combine_diff_path_size(n, l) \\\ndiff --git a/revision.c b/revision.c\nindex 13cfb59b38..a608884aad 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1995,6 +1995,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 0;\n \t\trevs->combine_merges = 1;\n+\t} else if (!strcmp(arg, \"--combined-all-names\")) {\n+\t\trevs->diff = 1;\n+\t\trevs->combined_all_names = 1;\n \t} else if (!strcmp(arg, \"--cc\")) {\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 1;\n@@ -2491,6 +2494,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t}\n \tif (revs->combine_merges)\n \t\trevs->ignore_merges = 0;\n+\tif (revs->combined_all_names && !revs->combine_merges)\n+\t\tdie(\"--combined-all-names makes no sense without -c or --cc\");\n+\n \trevs->diffopt.abbrev = revs->abbrev;\n \n \tif (revs->line_level_traverse) {\ndiff --git a/revision.h b/revision.h\nindex 52e5a88ff5..074a584410 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -171,6 +171,7 @@ struct rev_info {\n \t\t\tverbose_header:1,\n \t\t\tignore_merges:1,\n \t\t\tcombine_merges:1,\n+\t\t\tcombined_all_names:1,\n \t\t\tdense_combined_merges:1,\n \t\t\talways_show_header:1;\n \ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex e2824d3437..5bccc323f6 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -435,4 +435,51 @@ test_expect_success 'combine diff gets tree sorting right' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup for --combined-with-paths' '\n+\tgit branch side1c &&\n+\tgit branch side2c &&\n+\tgit checkout side1c &&\n+\ttest_seq 1 10 >$(printf \"file\\twith\\ttabs\") &&\n+\tgit add file* &&\n+\tgit commit -m with &&\n+\tgit checkout side2c &&\n+\ttest_seq 1 9 >$(printf \"i\\tam\\ttabbed\") &&\n+\techo ten >>$(printf \"i\\tam\\ttabbed\") &&\n+\tgit add *tabbed &&\n+\tgit commit -m iam &&\n+\tgit checkout -b mergery side1c &&\n+\tgit merge --no-commit side2c &&\n+\tgit rm *tabs &&\n+\techo eleven >>$(printf \"i\\tam\\ttabbed\") &&\n+\tgit mv \"$(printf \"i\\tam\\ttabbed\")\" \"$(printf \"fickle\\tnaming\")\" &&\n+\tgit add fickle* &&\n+\tgit commit\n+'\n+\n+test_expect_success '--combined-all-names and --raw' '\n+\tcat <<-\\EOF >expect &&\n+\t::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\t\"file\\twith\\ttabs\"\t\"i\\tam\\ttabbed\"\t\"fickle\\tnaming\"\n+\tEOF\n+\tgit diff-tree -c -M --raw --combined-all-names HEAD >actual.tmp &&\n+\tsed 1d actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--combined-all-names and --raw -and -z' '\n+\tprintf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n+\tgit diff-tree -c -M --raw --combined-all-names -z HEAD >actual &&\n+\ttest_cmp -a expect actual\n+'\n+\n+test_expect_success '--combined-all-names and --cc' '\n+\tcat <<-\\EOF >expect &&\n+\t--- \"a/file\\twith\\ttabs\"\n+\t--- \"a/i\\tam\\ttabbed\"\n+\t+++ \"b/fickle\\tnaming\"\n+\tEOF\n+\tgit diff-tree --cc -M --combined-all-names HEAD >actual.tmp &&\n+\tgrep ^[-+][-+][-+] actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.20.1.310.gf2549f3af6\n\n"},{"id":"367744","messageId":"20190127015216.GX423984@genre.crustytoothpaste.net","threadId":"50310","inReplyTo":"CABPp-BHTsZBO11ykfTaZ_dzpvrww5D3A57qBM05EgP_wK-iM9g@mail.gmail.com","subject":"Re: [PATCH v2] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-01-27T01:52:17Z","receivedAt":"2019-01-27T01:52:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Jan 25, 2019 at 09:52:07AM -0800, Elijah Newren wrote:\n> On Fri, Jan 25, 2019 at 9:41 AM Derrick Stolee wrote:\n> >\n> > On 1/25/2019 11:54 AM, Elijah Newren wrote:\n> > > +test_expect_success '--combined-with-paths works with -z as well' '\n> > > + printf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n> >\n> > I'm guessing that you use printf here because the\n> > 'cat <<-\\EOF' approach doesn't work with the special\n> > tabs? Kudos for putting in the extra effort here for\n> > the special formatting!\n> \n> Yeah, I didn't know how to easily get NUL bytes in the stream without\n> printf, and once I was using printf the EOF HEREDOC no longer had a\n> useful purpose. In the first testcase, since there were only\n> printable characters in the expected output, a HEREDOC worked well. I\n> guess I could have just used printf for both testcases, but having the\n> literal output shown where it's possible for a human to read it seemed\n> like an advantage worth capitalizing on. If anyone feels strongly\n> that I should use printf for both tests, I can switch them though.\n\nI'm fine with the patch as it stands, but just for future reference (for\nyou and anyone else), we have functions in the test library called\nnul_to_q and q_to_nul that convert NUL back and forth to the letter \"Q\".\nThey can make it a bit easier to write tests where we expect some format\nto have NULs in it.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"367807","messageId":"xmqqd0oheirf.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"CABPp-BFaVwwM17Oe+-RoP5wt2V58AD=dr01uNPuV=9r4yoF3Qg@mail.gmail.com","subject":"Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-28T00:19:16Z","receivedAt":"2019-01-28T00:21:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> ... Instead, I decided that\n> instead of having a \"from\" and a \"to\" header, we would have N \"from\"\n> headers (since all parents are what we came from) and 1 \"to\" header.\n> I hope that makes sense, because otherwise I think this become to\n> unwieldy of a change.\n\nYeah, it makes sense for an N-way merge to have N 'from' pathnames\nthat resulted in a single 'to' pathname in the resulting tree.\n\n"},{"id":"368483","messageId":"20190204200754.16413-1-newren@gmail.com","threadId":"50310","inReplyTo":"20190126221811.20241-1-newren@gmail.com","subject":"[PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-04T20:07:54Z","receivedAt":"2019-02-04T20:08:18Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The combined diff format for merges will only list one filename, even if\nrename or copy detection is active. For example, with raw format one\nmight see:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n\nThis doesn't let us know what the original name of bar.sh was in the\nfirst parent, and doesn't let us know what either of the original names\nof phooey.c were in either of the parents. In contrast, for non-merge\ncommits, raw format does provide original filenames (and a rename score\nto boot). In order to also provide original filenames for merge\ncommits, add a --combined-all-names option (which must be used with\neither -c or --cc, and is likely only useful with rename or copy\ndetection active) so that we can print tab-separated filenames when\nrenames are involved. This transforms the above output to:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\tdesc.c\tdesc.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n\nFurther, in patch format, this changes the from/to headers so that\ninstead of just having one \"from\" header, we get one for each parent.\nFor example, instead of having\n\n --- a/phooey.c\n +++ b/phooey.c\n\nwe would see\n\n --- a/fooey.c\n --- a/fuey.c\n +++ b/phooey.c\n\nSigned-off-by: Elijah Newren \n---\n\nChanges since v3:\n * None, just sending out to maintainer with list cc'ed as per\n SubmittingPatches; v3 had time for review and Junio elsewhere in the\n thread said the strategy of having multiple \"from\" versions makes\n sense.\n\n Documentation/diff-format.txt | 20 +++++--\n Documentation/diff-generate-patch.txt | 13 +++++\n Documentation/git-diff-tree.txt | 11 +++-\n Documentation/rev-list-options.txt | 7 +++\n builtin/diff-tree.c | 6 ++-\n combine-diff.c | 76 +++++++++++++++++++++++----\n diff.h | 1 +\n revision.c | 6 +++\n revision.h | 1 +\n t/t4038-diff-combined.sh | 47 +++++++++++++++++\n 10 files changed, 171 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt\nindex cdcc17f0ad..8fe800cd9b 100644\n--- a/Documentation/diff-format.txt\n+++ b/Documentation/diff-format.txt\n@@ -95,12 +95,26 @@ from the format described above in the following way:\n . there are more \"src\" modes and \"src\" sha1\n . status is concatenated status characters for each parent\n . no optional \"score\" number\n-. single path, only for \"dst\"\n+. tab-separated pathname(s) of the file\n \n-Example:\n+For `-c` and `--cc`, only the destination or final path is shown even\n+if the file was renamed on any side of history. With\n+`--combined-all-names`, the name of the path in each parent is shown\n+followed by the name of the path in the merge commit.\n+\n+Examples for `-c` and `-cc` without `--combined-all-names`:\n+------------------------------------------------\n+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n+------------------------------------------------\n+\n+Examples when `--combined-all-names` added to either `-c` or `--cc`:\n \n ------------------------------------------------\n-::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\tdesc.c\tdesc.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n ------------------------------------------------\n \n Note that 'combined diff' lists only files which were modified from\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 231105cff4..69cb3b0349 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -144,6 +144,19 @@ Similar to two-line header for traditional 'unified' diff\n format, `/dev/null` is used to signal created or deleted\n files.\n \n+However, if the --combined-all-paths option is provided, instead of a\n+two-line from-file/to-file you get a N+1 line from-file/to-file header,\n+where N is the number of parents in the merge commit\n+\n+ --- a/file\n+ --- a/file\n+ --- a/file\n+ +++ b/file\n++\n+This extended format can be useful if rename or copy detection is\n+active, to allow you to see the original name of the file in different\n+parents.\n+\n 4. Chunk header format is modified to prevent people from\n accidentally feeding it to `patch -p1`. Combined diff format\n was created for review of merge commit changes, and was not\ndiff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt\nindex 2319b2b192..7b6dcbd7a8 100644\n--- a/Documentation/git-diff-tree.txt\n+++ b/Documentation/git-diff-tree.txt\n@@ -10,8 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git diff-tree' [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty]\n-\t [-t] [-r] [-c | --cc] [--root] []\n-\t [] [...]\n+\t [-t] [-r] [-c | --cc] [--combined-all-names] [--root]\n+\t [] [] [...]\n \n DESCRIPTION\n -----------\n@@ -108,6 +108,13 @@ include::pretty-options.txt[]\n \titself and the commit log message is not shown, just like in any other\n \t\"empty diff\" case.\n \n+--combined-all-names::\n+\tThis flag causes combined diffs (used for merge commits) to\n+\tlist the name of the file from all parents. It thus only has\n+\teffect when -c or --cc are specified, and is likely only\n+\tuseful if filename changes are detected (i.e. when either\n+\trename or copy detection have been requested).\n+\n --always::\n \tShow the commit itself and the commit log message even\n \tif the diff itself is empty.\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 98b538bc77..97a6651bae 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -948,6 +948,13 @@ options may be given. See linkgit:git-diff-files[1] for more options.\n \tthe parents have only two variants and the merge result picks\n \tone of them without modification.\n \n+--combined-all-names::\n+\tThis flag causes combined diffs (used for merge commits) to\n+\tlist the name of the file from all parents. It thus only has\n+\teffect when -c or --cc are specified, and is likely only\n+\tuseful if filename changes are detected (i.e. when either\n+\trename or copy detection have been requested).\n+\n -m::\n \tThis flag makes the merge commits show the full diff like\n \tregular commits; for each merge parent, a separate log entry\ndiff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\nindex ef996126d7..bbeebc833f 100644\n--- a/builtin/diff-tree.c\n+++ b/builtin/diff-tree.c\n@@ -82,9 +82,13 @@ static int diff_tree_stdin(char *line)\n }\n \n static const char diff_tree_usage[] =\n-\"git diff-tree [--stdin] [-m] [-c] [--cc] [-s] [-v] [--pretty] [-t] [-r] [--root] \"\n+\"git diff-tree [--stdin] [-m] [-c | --cc] [-s] [-v] [--pretty] [-t] [-r] [--root] \"\n \"[] [] [...]\\n\"\n \" -r diff recursively\\n\"\n+\" -c show combined diff for merge commits\\n\"\n+\" --cc show combined diff for merge commits removing uninteresting hunks\\n\"\n+\" --combined-all-names\\n\"\n+\" show name of file in all parents for combined diffs\\n\"\n \" --root include the initial commit as diff against /dev/null\\n\"\n COMMON_DIFF_OPTIONS_HELP;\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex a143c00634..339daac02b 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -23,11 +23,20 @@ static int compare_paths(const struct combine_diff_path *one,\n \t\t\t\t two->path, strlen(two->path), two->mode);\n }\n \n-static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n+static int filename_changed(char status)\n+{\n+\treturn status == 'R' || status == 'C';\n+}\n+\n+static struct combine_diff_path *intersect_paths(\n+\tstruct combine_diff_path *curr,\n+\tint n,\n+\tint num_parent,\n+\tint combined_all_names)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct combine_diff_path *p, **tail = &curr;\n-\tint i, cmp;\n+\tint i, j, cmp;\n \n \tif (!n) {\n \t\tfor (i = 0; i < q->nr; i++) {\n@@ -50,6 +59,13 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\t\tp->parent[n].status = q->queue[i]->status;\n+\n+\t\t\tif (combined_all_names &&\n+\t\t\t filename_changed(p->parent[n].status)) {\n+\t\t\t\tstrbuf_init(&p->parent[n].path, 0);\n+\t\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t\t q->queue[i]->one->path);\n+\t\t\t}\n \t\t\t*tail = p;\n \t\t\ttail = &p->next;\n \t\t}\n@@ -68,6 +84,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\tif (cmp < 0) {\n \t\t\t/* p->path not in q->queue[]; drop it */\n \t\t\t*tail = p->next;\n+\t\t\tfor (j = 0; j < num_parent; j++)\n+\t\t\t\tif (combined_all_names &&\n+\t\t\t\t filename_changed(p->parent[j].status))\n+\t\t\t\t\tstrbuf_release(&p->parent[j].path);\n \t\t\tfree(p);\n \t\t\tcontinue;\n \t\t}\n@@ -81,6 +101,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\tp->parent[n].status = q->queue[i]->status;\n+\t\tif (combined_all_names &&\n+\t\t filename_changed(p->parent[n].status))\n+\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t q->queue[i]->one->path);\n \n \t\ttail = &p->next;\n \t\ti++;\n@@ -960,12 +984,25 @@ static void show_combined_header(struct combine_diff_path *elem,\n \tif (!show_file_header)\n \t\treturn;\n \n-\tif (added)\n-\t\tdump_quoted_path(\"--- \", \"\", \"/dev/null\",\n-\t\t\t\t line_prefix, c_meta, c_reset);\n-\telse\n-\t\tdump_quoted_path(\"--- \", a_prefix, elem->path,\n-\t\t\t\t line_prefix, c_meta, c_reset);\n+\tif (rev->combined_all_names) {\n+\t\tfor (i = 0; i < num_parent; i++) {\n+\t\t\tchar *path = filename_changed(elem->parent[i].status)\n+\t\t\t\t? elem->parent[i].path.buf : elem->path;\n+\t\t\tif (elem->parent[i].status == DIFF_STATUS_ADDED)\n+\t\t\t\tdump_quoted_path(\"--- \", \"\", \"/dev/null\",\n+\t\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\t\telse\n+\t\t\t\tdump_quoted_path(\"--- \", a_prefix, path,\n+\t\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\t}\n+\t} else {\n+\t\tif (added)\n+\t\t\tdump_quoted_path(\"--- \", \"\", \"/dev/null\",\n+\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\telse\n+\t\t\tdump_quoted_path(\"--- \", a_prefix, elem->path,\n+\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t}\n \tif (deleted)\n \t\tdump_quoted_path(\"+++ \", \"\", \"/dev/null\",\n \t\t\t\t line_prefix, c_meta, c_reset);\n@@ -1227,6 +1264,15 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re\n \t\tputchar(inter_name_termination);\n \t}\n \n+\tfor (i = 0; i < num_parent; i++)\n+\t\tif (rev->combined_all_names) {\n+\t\t\tif (filename_changed(p->parent[i].status))\n+\t\t\t\twrite_name_quoted(p->parent[i].path.buf, stdout,\n+\t\t\t\t\t\t inter_name_termination);\n+\t\t\telse\n+\t\t\t\twrite_name_quoted(p->path, stdout,\n+\t\t\t\t\t\t inter_name_termination);\n+\t\t}\n \twrite_name_quoted(p->path, stdout, line_termination);\n }\n \n@@ -1324,7 +1370,9 @@ static const char *path_path(void *obj)\n \n /* find set of paths that every parent touches */\n static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n-\tconst struct oid_array *parents, struct diff_options *opt)\n+\tconst struct oid_array *parents,\n+\tstruct diff_options *opt,\n+\tint combined_all_names)\n {\n \tstruct combine_diff_path *paths = NULL;\n \tint i, num_parent = parents->nr;\n@@ -1350,7 +1398,8 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n \t\t\topt->output_format = DIFF_FORMAT_NO_OUTPUT;\n \t\tdiff_tree_oid(&parents->oid[i], oid, \"\", opt);\n \t\tdiffcore_std(opt);\n-\t\tpaths = intersect_paths(paths, i, num_parent);\n+\t\tpaths = intersect_paths(paths, i, num_parent,\n+\t\t\t\t\tcombined_all_names);\n \n \t\t/* if showing diff, show it in requested order */\n \t\tif (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&\n@@ -1460,7 +1509,8 @@ void diff_tree_combined(const struct object_id *oid,\n \t\t * diff(sha1,parent_i) for all i to do the job, specifically\n \t\t * for parent0.\n \t\t */\n-\t\tpaths = find_paths_generic(oid, parents, &diffopts);\n+\t\tpaths = find_paths_generic(oid, parents, &diffopts,\n+\t\t\t\t\t rev->combined_all_names);\n \t}\n \telse {\n \t\tint stat_opt;\n@@ -1535,6 +1585,10 @@ void diff_tree_combined(const struct object_id *oid,\n \twhile (paths) {\n \t\tstruct combine_diff_path *tmp = paths;\n \t\tpaths = paths->next;\n+\t\tfor (i = 0; i < num_parent; i++)\n+\t\t\tif (rev->combined_all_names &&\n+\t\t\t filename_changed(tmp->parent[i].status))\n+\t\t\t\tstrbuf_release(&tmp->parent[i].path);\n \t\tfree(tmp);\n \t}\n \ndiff --git a/diff.h b/diff.h\nindex b512d0477a..90ea0256a5 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -294,6 +294,7 @@ struct combine_diff_path {\n \t\tchar status;\n \t\tunsigned int mode;\n \t\tstruct object_id oid;\n+\t\tstruct strbuf path;\n \t} parent[FLEX_ARRAY];\n };\n #define combine_diff_path_size(n, l) \\\ndiff --git a/revision.c b/revision.c\nindex 13cfb59b38..a608884aad 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1995,6 +1995,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 0;\n \t\trevs->combine_merges = 1;\n+\t} else if (!strcmp(arg, \"--combined-all-names\")) {\n+\t\trevs->diff = 1;\n+\t\trevs->combined_all_names = 1;\n \t} else if (!strcmp(arg, \"--cc\")) {\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 1;\n@@ -2491,6 +2494,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t}\n \tif (revs->combine_merges)\n \t\trevs->ignore_merges = 0;\n+\tif (revs->combined_all_names && !revs->combine_merges)\n+\t\tdie(\"--combined-all-names makes no sense without -c or --cc\");\n+\n \trevs->diffopt.abbrev = revs->abbrev;\n \n \tif (revs->line_level_traverse) {\ndiff --git a/revision.h b/revision.h\nindex 52e5a88ff5..074a584410 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -171,6 +171,7 @@ struct rev_info {\n \t\t\tverbose_header:1,\n \t\t\tignore_merges:1,\n \t\t\tcombine_merges:1,\n+\t\t\tcombined_all_names:1,\n \t\t\tdense_combined_merges:1,\n \t\t\talways_show_header:1;\n \ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex e2824d3437..5bccc323f6 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -435,4 +435,51 @@ test_expect_success 'combine diff gets tree sorting right' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup for --combined-with-paths' '\n+\tgit branch side1c &&\n+\tgit branch side2c &&\n+\tgit checkout side1c &&\n+\ttest_seq 1 10 >$(printf \"file\\twith\\ttabs\") &&\n+\tgit add file* &&\n+\tgit commit -m with &&\n+\tgit checkout side2c &&\n+\ttest_seq 1 9 >$(printf \"i\\tam\\ttabbed\") &&\n+\techo ten >>$(printf \"i\\tam\\ttabbed\") &&\n+\tgit add *tabbed &&\n+\tgit commit -m iam &&\n+\tgit checkout -b mergery side1c &&\n+\tgit merge --no-commit side2c &&\n+\tgit rm *tabs &&\n+\techo eleven >>$(printf \"i\\tam\\ttabbed\") &&\n+\tgit mv \"$(printf \"i\\tam\\ttabbed\")\" \"$(printf \"fickle\\tnaming\")\" &&\n+\tgit add fickle* &&\n+\tgit commit\n+'\n+\n+test_expect_success '--combined-all-names and --raw' '\n+\tcat <<-\\EOF >expect &&\n+\t::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\t\"file\\twith\\ttabs\"\t\"i\\tam\\ttabbed\"\t\"fickle\\tnaming\"\n+\tEOF\n+\tgit diff-tree -c -M --raw --combined-all-names HEAD >actual.tmp &&\n+\tsed 1d actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--combined-all-names and --raw -and -z' '\n+\tprintf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n+\tgit diff-tree -c -M --raw --combined-all-names -z HEAD >actual &&\n+\ttest_cmp -a expect actual\n+'\n+\n+test_expect_success '--combined-all-names and --cc' '\n+\tcat <<-\\EOF >expect &&\n+\t--- \"a/file\\twith\\ttabs\"\n+\t--- \"a/i\\tam\\ttabbed\"\n+\t+++ \"b/fickle\\tnaming\"\n+\tEOF\n+\tgit diff-tree --cc -M --combined-all-names HEAD >actual.tmp &&\n+\tgrep ^[-+][-+][-+] actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.20.1.310.g17ca096f17\n\n"},{"id":"368490","messageId":"xmqqlg2vtfmo.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"20190204200754.16413-1-newren@gmail.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-04T21:20:31Z","receivedAt":"2019-02-04T21:20:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\nThere is one place that says combined-all-paths, and everywhere else\nit says combined-all-names. The former is probably techincally more\ncorrect, I think ;-)\n\n> The combined diff format for merges will only list one filename, even if\n> rename or copy detection is active. For example, with raw format one\n> might see:\n>\n> ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n> ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n> ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n>\n> This doesn't let us know what the original name of bar.sh was in the\n> first parent, and doesn't let us know what either of the original names\n> of phooey.c were in either of the parents. In contrast, for non-merge\n> commits, raw format does provide original filenames (and a rename score\n> to boot). In order to also provide original filenames for merge\n> commits, add a --combined-all-names option (which must be used with\n> either -c or --cc, and is likely only useful with rename or copy\n> detection active) so that we can print tab-separated filenames when\n> renames are involved. This transforms the above output to:\n>\n> ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\tdesc.c\tdesc.c\n> ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\tbar.sh\n> ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n>\n> Further, in patch format, this changes the from/to headers so that\n> instead of just having one \"from\" header, we get one for each parent.\n> For example, instead of having\n>\n> --- a/phooey.c\n> +++ b/phooey.c\n>\n> we would see\n>\n> --- a/fooey.c\n> --- a/fuey.c\n> +++ b/phooey.c\n\nDo we have the three \"rename from fooey.c\", \"rename from fuey.c\" and\n\"rename to \"phooey.c\" extended headers, too? That's what I meant in\nmy response, but I do like what I see in the above example ;-)\n\n> -. single path, only for \"dst\"\n> +. tab-separated pathname(s) of the file\n> \n> -Example:\n> +For `-c` and `--cc`, only the destination or final path is shown even\n> +if the file was renamed on any side of history. With\n> +`--combined-all-names`, the name of the path in each parent is shown\n> +followed by the name of the path in the merge commit.\n> +\n> +Examples for `-c` and `-cc` without `--combined-all-names`:\n> +------------------------------------------------\n> +::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\n> +::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n> +::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n> +------------------------------------------------\n> +\n> +Examples when `--combined-all-names` added to either `-c` or `--cc`:\n"},{"id":"368514","messageId":"nycvar.QRO.7.76.6.1902051045460.41@tvgsbejvaqbjf.bet","threadId":"50310","inReplyTo":"20190204200754.16413-1-newren@gmail.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-02-05T09:48:42Z","receivedAt":"2019-02-05T09:49:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Mon, 4 Feb 2019, Elijah Newren wrote:\n\n> The combined diff format for merges will only list one filename, even if\n> rename or copy detection is active. For example, with raw format one\n> might see:\n> \n> ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n> ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n> ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n> \n> This doesn't let us know what the original name of bar.sh was in the\n> first parent, and doesn't let us know what either of the original names\n> of phooey.c were in either of the parents. In contrast, for non-merge\n> commits, raw format does provide original filenames (and a rename score\n> to boot). In order to also provide original filenames for merge\n> commits, add a --combined-all-names option (which must be used with\n> either -c or --cc, and is likely only useful with rename or copy\n> detection active) so that we can print tab-separated filenames when\n> renames are involved. This transforms the above output to:\n> \n> ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\tdesc.c\tdesc.c\n> ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\tbar.sh\n> ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n> \n> Further, in patch format, this changes the from/to headers so that\n> instead of just having one \"from\" header, we get one for each parent.\n> For example, instead of having\n> \n> --- a/phooey.c\n> +++ b/phooey.c\n> \n> we would see\n> \n> --- a/fooey.c\n> --- a/fuey.c\n> +++ b/phooey.c\n> \n> Signed-off-by: Elijah Newren \n\nI do not really see where this needs to have tests with filenames\ncontaining tabs, but your test does create such files. Which makes it\nbreak on filesystems that do not allow tabs in file names, such as\nNTFS. I need this to make the test pass again:\n\n-- snip --\ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex 5bccc323f648..596705985baa 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -435,7 +435,7 @@ test_expect_success 'combine diff gets tree sorting right' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'setup for --combined-with-paths' '\n+test_expect_success FUNNYNAMES 'setup for --combined-with-paths' '\n \tgit branch side1c &&\n \tgit branch side2c &&\n \tgit checkout side1c &&\n@@ -456,7 +456,7 @@ test_expect_success 'setup for --combined-with-paths' '\n \tgit commit\n '\n \n-test_expect_success '--combined-all-names and --raw' '\n+test_expect_success FUNNYNAMES '--combined-all-names and --raw' '\n \tcat <<-\\EOF >expect &&\n \t::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\t\"file\\twith\\ttabs\"\t\"i\\tam\\ttabbed\"\t\"fickle\\tnaming\"\n \tEOF\n@@ -465,13 +465,13 @@ test_expect_success '--combined-all-names and --raw' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--combined-all-names and --raw -and -z' '\n+test_expect_success FUNNYNAMES '--combined-all-names and --raw -and -z' '\n \tprintf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n \tgit diff-tree -c -M --raw --combined-all-names -z HEAD >actual &&\n \ttest_cmp -a expect actual\n '\n \n-test_expect_success '--combined-all-names and --cc' '\n+test_expect_success FUNNYNAMES '--combined-all-names and --cc' '\n \tcat <<-\\EOF >expect &&\n \t--- \"a/file\\twith\\ttabs\"\n \t--- \"a/i\\tam\\ttabbed\"\n-- snap --\n\nBut maybe you want to get rid of the funny file names? Or test for them in\na separate, dedicated test case so that we do not have to guard *all* of\nyour added tests behind FUNNYNAMES?\n\nCiao,\nDscho\n"},{"id":"368527","messageId":"CABPp-BGyL5BAejK-P-EdscFdH3C6uR7e6CbgNe-9doy-mkw-vg@mail.gmail.com","threadId":"50310","inReplyTo":"xmqqlg2vtfmo.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-05T15:51:56Z","receivedAt":"2019-02-05T15:52:12Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Feb 4, 2019 at 1:20 PM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> There is one place that says combined-all-paths, and everywhere else\n> it says combined-all-names. The former is probably techincally more\n> correct, I think ;-)\n\nLooks like I had a mixture of my original name (combined-with-paths)\nand the one someone else suggested (combined-all-names). Since you\nlike the hybrid better, I'll switch them all over to it.\n\n> > The combined diff format for merges will only list one filename, even if\n> > rename or copy detection is active. For example, with raw format one\n> > might see:\n> >\n> > ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c\n> > ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM bar.sh\n> > ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR phooey.c\n> >\n> > This doesn't let us know what the original name of bar.sh was in the\n> > first parent, and doesn't let us know what either of the original names\n> > of phooey.c were in either of the parents. In contrast, for non-merge\n> > commits, raw format does provide original filenames (and a rename score\n> > to boot). In order to also provide original filenames for merge\n> > commits, add a --combined-all-names option (which must be used with\n> > either -c or --cc, and is likely only useful with rename or copy\n> > detection active) so that we can print tab-separated filenames when\n> > renames are involved. This transforms the above output to:\n> >\n> > ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM desc.c desc.c desc.c\n> > ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM foo.sh bar.sh bar.sh\n> > ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR fooey.c fuey.c phooey.c\n> >\n> > Further, in patch format, this changes the from/to headers so that\n> > instead of just having one \"from\" header, we get one for each parent.\n> > For example, instead of having\n> >\n> > --- a/phooey.c\n> > +++ b/phooey.c\n> >\n> > we would see\n> >\n> > --- a/fooey.c\n> > --- a/fuey.c\n> > +++ b/phooey.c\n>\n> Do we have the three \"rename from fooey.c\", \"rename from fuey.c\" and\n> \"rename to \"phooey.c\" extended headers, too? That's what I meant in\n> my response, but I do like what I see in the above example ;-)\n\nAh, gotcha. I'll look into whether it's possible to hook it up to\ndiff.c's fill_metainfo() .\n\nElijah\n"},{"id":"368528","messageId":"CABPp-BGQ62Mj1jym8-xu5XKN6mJ65yq-QUqXjR-xx=AWFZgNbw@mail.gmail.com","threadId":"50310","inReplyTo":"nycvar.QRO.7.76.6.1902051045460.41@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-05T15:54:38Z","receivedAt":"2019-02-05T15:54:53Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Dscho,\n\nOn Tue, Feb 5, 2019 at 1:48 AM Johannes Schindelin\n wrote:\n>\n> Hi Elijah,\n>\n> On Mon, 4 Feb 2019, Elijah Newren wrote:\n>\n> > The combined diff format for merges will only list one filename, even if\n> > rename or copy detection is active. For example, with raw format one\n> > might see:\n> >\n> > ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c\n> > ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM bar.sh\n> > ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR phooey.c\n> >\n> > This doesn't let us know what the original name of bar.sh was in the\n> > first parent, and doesn't let us know what either of the original names\n> > of phooey.c were in either of the parents. In contrast, for non-merge\n> > commits, raw format does provide original filenames (and a rename score\n> > to boot). In order to also provide original filenames for merge\n> > commits, add a --combined-all-names option (which must be used with\n> > either -c or --cc, and is likely only useful with rename or copy\n> > detection active) so that we can print tab-separated filenames when\n> > renames are involved. This transforms the above output to:\n> >\n> > ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM desc.c desc.c desc.c\n> > ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM foo.sh bar.sh bar.sh\n> > ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR fooey.c fuey.c phooey.c\n> >\n> > Further, in patch format, this changes the from/to headers so that\n> > instead of just having one \"from\" header, we get one for each parent.\n> > For example, instead of having\n> >\n> > --- a/phooey.c\n> > +++ b/phooey.c\n> >\n> > we would see\n> >\n> > --- a/fooey.c\n> > --- a/fuey.c\n> > +++ b/phooey.c\n> >\n> > Signed-off-by: Elijah Newren \n>\n> I do not really see where this needs to have tests with filenames\n> containing tabs, but your test does create such files. Which makes it\n> break on filesystems that do not allow tabs in file names, such as\n> NTFS. I need this to make the test pass again:\n>\n> -- snip --\n> diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\n> index 5bccc323f648..596705985baa 100755\n> --- a/t/t4038-diff-combined.sh\n> +++ b/t/t4038-diff-combined.sh\n> @@ -435,7 +435,7 @@ test_expect_success 'combine diff gets tree sorting right' '\n> test_cmp expect actual\n> '\n>\n> -test_expect_success 'setup for --combined-with-paths' '\n> +test_expect_success FUNNYNAMES 'setup for --combined-with-paths' '\n> git branch side1c &&\n> git branch side2c &&\n> git checkout side1c &&\n> @@ -456,7 +456,7 @@ test_expect_success 'setup for --combined-with-paths' '\n> git commit\n> '\n>\n> -test_expect_success '--combined-all-names and --raw' '\n> +test_expect_success FUNNYNAMES '--combined-all-names and --raw' '\n> cat <<-\\EOF >expect &&\n> ::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR \"file\\twith\\ttabs\" \"i\\tam\\ttabbed\" \"fickle\\tnaming\"\n> EOF\n> @@ -465,13 +465,13 @@ test_expect_success '--combined-all-names and --raw' '\n> test_cmp expect actual\n> '\n>\n> -test_expect_success '--combined-all-names and --raw -and -z' '\n> +test_expect_success FUNNYNAMES '--combined-all-names and --raw -and -z' '\n> printf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n> git diff-tree -c -M --raw --combined-all-names -z HEAD >actual &&\n> test_cmp -a expect actual\n> '\n>\n> -test_expect_success '--combined-all-names and --cc' '\n> +test_expect_success FUNNYNAMES '--combined-all-names and --cc' '\n> cat <<-\\EOF >expect &&\n> --- \"a/file\\twith\\ttabs\"\n> --- \"a/i\\tam\\ttabbed\"\n> -- snap --\n>\n> But maybe you want to get rid of the funny file names? Or test for them in\n> a separate, dedicated test case so that we do not have to guard *all* of\n> your added tests behind FUNNYNAMES?\n\nI don't want to get rid of them; the initial reviews of my original\npatch thought the format was ambiguous and would mishandle files with\ntabs in them. But yeah, I can definitely add a few testcases without\nFUNNYNAMES. Sorry for the headache; I'll fix this up.\n\nElijah\n"},{"id":"368534","messageId":"xmqqr2cmru0k.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"CABPp-BGQ62Mj1jym8-xu5XKN6mJ65yq-QUqXjR-xx=AWFZgNbw@mail.gmail.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-05T18:04:59Z","receivedAt":"2019-02-05T18:05:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> I don't want to get rid of them; the initial reviews of my original\n> patch thought the format was ambiguous and would mishandle files with\n> tabs in them. But yeah, I can definitely add a few testcases without\n> FUNNYNAMES. Sorry for the headache; I'll fix this up.\n\nThanks, both. I was about to squash the fix from Dscho in, but I\nagree that it is a much better idea to cover the basics in separate\ntests that would run without FUNNYNAMES. Will hold and wait for\nyour updates.\n\nThanks.\n"},{"id":"368544","messageId":"xmqqa7jarmvo.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"CABPp-BGyL5BAejK-P-EdscFdH3C6uR7e6CbgNe-9doy-mkw-vg@mail.gmail.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-05T20:39:07Z","receivedAt":"2019-02-05T20:39:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> > Further, in patch format, this changes the from/to headers so that\n>> > instead of just having one \"from\" header, we get one for each parent.\n>> > For example, instead of having\n>> >\n>> > --- a/phooey.c\n>> > +++ b/phooey.c\n>> >\n>> > we would see\n>> >\n>> > --- a/fooey.c\n>> > --- a/fuey.c\n>> > +++ b/phooey.c\n>>\n>> Do we have the three \"rename from fooey.c\", \"rename from fuey.c\" and\n>> \"rename to \"phooey.c\" extended headers, too? That's what I meant in\n>> my response, but I do like what I see in the above example ;-)\n>\n> Ah, gotcha. I'll look into whether it's possible to hook it up to\n> diff.c's fill_metainfo() .\n\nJust to clarify. I do not think these extended headers are \"must\nhave\"; the \"--cc\" output is not meant for machine consumption, as it\nsimplifies the output by omitting hunks that preimage trees agree\nwith each other etc., and making the resulting \"patch\" not showing\nthe whole picture, and these extended header lines might only become\nwaste of the screen real estate.\n\nSo, do not spend too much effort to emit these textual info that can\nbe easily seen with the N+1 plus/minus header lines.\n\nThanks.\n\n"},{"id":"368728","messageId":"CABPp-BG4WftXZgbzN48zSo1Z5BcWYjdbz+8hGSGBNJbSsYUzAA@mail.gmail.com","threadId":"50310","inReplyTo":"xmqqa7jarmvo.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-07T19:50:15Z","receivedAt":"2019-02-07T19:50:33Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Feb 5, 2019 at 12:39 PM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> >> > Further, in patch format, this changes the from/to headers so that\n> >> > instead of just having one \"from\" header, we get one for each parent.\n> >> > For example, instead of having\n> >> >\n> >> > --- a/phooey.c\n> >> > +++ b/phooey.c\n> >> >\n> >> > we would see\n> >> >\n> >> > --- a/fooey.c\n> >> > --- a/fuey.c\n> >> > +++ b/phooey.c\n> >>\n> >> Do we have the three \"rename from fooey.c\", \"rename from fuey.c\" and\n> >> \"rename to \"phooey.c\" extended headers, too? That's what I meant in\n> >> my response, but I do like what I see in the above example ;-)\n> >\n> > Ah, gotcha. I'll look into whether it's possible to hook it up to\n> > diff.c's fill_metainfo() .\n>\n> Just to clarify. I do not think these extended headers are \"must\n> have\"; the \"--cc\" output is not meant for machine consumption, as it\n> simplifies the output by omitting hunks that preimage trees agree\n> with each other etc., and making the resulting \"patch\" not showing\n> the whole picture, and these extended header lines might only become\n> waste of the screen real estate.\n>\n> So, do not spend too much effort to emit these textual info that can\n> be easily seen with the N+1 plus/minus header lines.\n>\n> Thanks.\n\nUnderstood, thanks.\n\nI think something can be done here, but I'm unsure exactly what. From\nDocumentation/diff-generate-patch.txt, the extended headers for normal\ndiff mode are:\n\n old mode \n new mode \n deleted file mode \n new file mode \n copy from \n copy to \n rename from \n rename to \n similarity index \n dissimilarity index \n index .. \n\nand for combined diffs they are:\n\n index ,..\n mode ,..\n new file mode \n deleted file mode ,\n\nmeaning that the ones we are missing are:\n\n copy from \n copy to \n rename from \n rename to \n similarity index \n dissimilarity index \n\nI think \"copy from\" and \"rename from\" should be relatively\nstraightforward. However, in a combined diff, we could have both a\nmodified status, a renamed status, and a copied status, meaning that\nwe'll need an array of both similarity and dissimilarity indexes...and\ntrying to present that to the user in a way that makes sense seems\nlike a lost cause to me. Does anyone else know how to represent that?\n I'm inclined to just leave it out.\n\nAlso, I'm afraid \"copy to\" and \"rename to\" could be confusing if both\nappeared, since there's only one \"to\" path. If there is both a copy\nand a rename involved relative to different parents, should these be\ncoalesced into a \"copy/rename to\" line?\n\n\nThanks,\nElijah\n"},{"id":"368733","messageId":"xmqqimxvny6s.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"CABPp-BG4WftXZgbzN48zSo1Z5BcWYjdbz+8hGSGBNJbSsYUzAA@mail.gmail.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-07T20:25:15Z","receivedAt":"2019-02-07T20:25:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> I think \"copy from\" and \"rename from\" should be relatively\n> straightforward. However, in a combined diff, we could have both a\n> modified status, a renamed status, and a copied status, meaning that\n> we'll need an array of both similarity and dissimilarity indexes...and\n> trying to present that to the user in a way that makes sense seems\n> like a lost cause to me. Does anyone else know how to represent that?\n> I'm inclined to just leave it out.\n>\n> Also, I'm afraid \"copy to\" and \"rename to\" could be confusing if both\n> appeared, since there's only one \"to\" path. If there is both a copy\n> and a rename involved relative to different parents, should these be\n> coalesced into a \"copy/rename to\" line?\n\nThere are three possible labels (i.e. 'in-place modification',\n'rename from elsewhere' and 'copy from elsewhere'), and you can say\n\"this commit created file F by renaming from X (or by copying X)\"\nonly when you know path F did not exist _immediately before_ this\ncommit. The distinction between rename and copy is whether the path\nX remains in the resulting commit (i.e. if there is no X, the commit\ncreated path F by moving X; if there is X, the commit copied the\ncontents of X into a new path F).\n\nSo telling renames and copies apart is probably straight-forward (if\nyou have sufficient information---I am not sure if you do in this\ncodepath offhand); as long as you know what pathname each preimage\n(i.e. parent of the perge) tree had and if that pathname is missing\nin the postimage (luckily there is only one---the merge result), it\nwas renamed, and otherwise it was copied.\n\nBut telling in-place modification and other two might be\ntrickier. In one parent path F may be missing but in the other\nparent path F may exist, and the result of the merge is made by\nmerging the contents of path X in the first parent and the contents\nof path F in the second parent. From the view of the transition\nbetween the first parent to the merge result, we moved the path X to\npath F and made some modifications (i.e. renamed). From the view of\nthe transition from the other branch, we kept the contents in path F\nduring the transition and there is no renames or copies involved.\n\nActually what I had in mind when I mentioned the extended headers\nthe first time in this discussion was that we would have \"rename\nfrom\", \"copy from\", etc. separately for each parent, as the contents\nmay have come from different paths in these parents. And that was\nwhere my earlier \"... might only become waste of the screen real\nestate\" comes from.\n\nSo, again, do not spend too much effort to emit these textual info\nthat can be easily seen with the N+1 plus/minus header lines.\n\nThanks.\n\n"},{"id":"368745","messageId":"CABPp-BHyLFNcdh+=hf=R5xhUoNAQHPophxgc5e2HRqgeTU4e8Q@mail.gmail.com","threadId":"50310","inReplyTo":"xmqqimxvny6s.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-07T22:10:26Z","receivedAt":"2019-02-07T22:10:42Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Feb 7, 2019 at 12:25 PM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > I think \"copy from\" and \"rename from\" should be relatively\n> > straightforward. However, in a combined diff, we could have both a\n> > modified status, a renamed status, and a copied status, meaning that\n> > we'll need an array of both similarity and dissimilarity indexes...and\n> > trying to present that to the user in a way that makes sense seems\n> > like a lost cause to me. Does anyone else know how to represent that?\n> > I'm inclined to just leave it out.\n> >\n> > Also, I'm afraid \"copy to\" and \"rename to\" could be confusing if both\n> > appeared, since there's only one \"to\" path. If there is both a copy\n> > and a rename involved relative to different parents, should these be\n> > coalesced into a \"copy/rename to\" line?\n>\n> There are three possible labels (i.e. 'in-place modification',\n> 'rename from elsewhere' and 'copy from elsewhere'), and you can say\n> \"this commit created file F by renaming from X (or by copying X)\"\n> only when you know path F did not exist _immediately before_ this\n> commit. The distinction between rename and copy is whether the path\n> X remains in the resulting commit (i.e. if there is no X, the commit\n> created path F by moving X; if there is X, the commit copied the\n> contents of X into a new path F).\n>\n> So telling renames and copies apart is probably straight-forward (if\n> you have sufficient information---I am not sure if you do in this\n> codepath offhand); as long as you know what pathname each preimage\n> (i.e. parent of the perge) tree had and if that pathname is missing\n> in the postimage (luckily there is only one---the merge result), it\n> was renamed, and otherwise it was copied.\n\nWe have change status, M, C, A, R, D, etc. So, R vs. C tells us\nrenamed or copied. We also have the original filename.\n\n> But telling in-place modification and other two might be\n> trickier. In one parent path F may be missing but in the other\n> parent path F may exist, and the result of the merge is made by\n> merging the contents of path X in the first parent and the contents\n> of path F in the second parent. From the view of the transition\n> between the first parent to the merge result, we moved the path X to\n> path F and made some modifications (i.e. renamed). From the view of\n> the transition from the other branch, we kept the contents in path F\n> during the transition and there is no renames or copies involved.\n>\n> Actually what I had in mind when I mentioned the extended headers\n> the first time in this discussion was that we would have \"rename\n> from\", \"copy from\", etc. separately for each parent, as the contents\n> may have come from different paths in these parents. And that was\n> where my earlier \"... might only become waste of the screen real\n> estate\" comes from.\n\nI think I'm with you on everything you said here, but perhaps not\nsince I can't see an answer to my question. Maybe an example will\nhelp:\n\nLet's say we have an octopus merge. Parent 1 had file F. Parent 2\nhad file X. Parent 3 had file Y. The octopus has two files: F' and\nX, with F' being very similar to F, X, and Y.\n\nThere's no \"modified from\" header; it's not needed (unless we want to\nadd a new kind of noise header?)\nWe could emit a \"copied from X\" header, due to parent 2.\nWe could emit a \"renamed from Y\" header, due to parent 3.\n\nNow, the question: In addition to the two \"from\" headers, how many\n\"to\" headers do we emit? In particular, do we emit both a \"copied to\nF\" and a \"renamed to F\" header, or just a combined \"renamed/copied to\nF\" header? I'm inclined to go with the latter, to avoid giving the\nidea that there are multiple targets, but maybe folks expect there to\nbe one \"rename to\" and \"copy to\" for each \"rename from\" or \"copy from\"\nthat appeared.\n\n> So, again, do not spend too much effort to emit these textual info\n> that can be easily seen with the N+1 plus/minus header lines.\n"},{"id":"368747","messageId":"xmqqtvhfmdxf.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"20190204200754.16413-1-newren@gmail.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-07T22:28:12Z","receivedAt":"2019-02-07T22:28:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\n> index 231105cff4..69cb3b0349 100644\n> --- a/Documentation/diff-generate-patch.txt\n> +++ b/Documentation/diff-generate-patch.txt\n> @@ -144,6 +144,19 @@ Similar to two-line header for traditional 'unified' diff\n> format, `/dev/null` is used to signal created or deleted\n> files.\n> \n> +However, if the --combined-all-paths option is provided, instead of a\n> +two-line from-file/to-file you get a N+1 line from-file/to-file header,\n> +where N is the number of parents in the merge commit\n> +\n> + --- a/file\n> + --- a/file\n> + --- a/file\n> + +++ b/file\n> ++\n> +This extended format can be useful if rename or copy detection is\n> +active, to allow you to see the original name of the file in different\n> +parents.\n> +\n> 4. Chunk header format is modified to prevent people from\n> accidentally feeding it to `patch -p1`. Combined diff format\n> was created for review of merge commit changes, and was not\n\nYou need to replace the blank line before the new paragraph that\nbegins with \"However\" with a line with a single \"+\" on it, to tell\nthe formatter that the new text is still part of the third item in\nthe list.\n"},{"id":"368753","messageId":"xmqqlg2rmazz.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"CABPp-BHyLFNcdh+=hf=R5xhUoNAQHPophxgc5e2HRqgeTU4e8Q@mail.gmail.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-07T23:31:28Z","receivedAt":"2019-02-07T23:31:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> Now, the question: In addition to the two \"from\" headers, how many\n> \"to\" headers do we emit? In particular, do we emit both a \"copied to\n> F\" and a \"renamed to F\" header, or just a combined \"renamed/copied to\n> F\" header?\n\nThere is only a single path that can be on the \"to\", as there is\nonly one final result, but _how_ the contents got to that path would\nbe different, so to be technically truly correct, you would end up\nshowing N \"to\" lines for a N-way merge, each of which gives the same\npath in the postimage, but some may say renamed, another may say\ncopioed and some others may need to say in-place edited.\n\nThat would increase the number of necessary lines from N (from) + 1\n(to) to N*2 (N for each of from and to), which makes it even less\neconomical.\n\nAnd showing a single \"renamed/copied\" feels more like a cop-out to\navoid being techincally incorrect, than giving a useful piece of\ninformation to the users.\n\nI am inclined to say that we should do _without_ any \"to\" line. And\nif we can do without any \"to\", perhaps we do not need \"from\", either.\n\n"},{"id":"368756","messageId":"CABPp-BEA9zUpv0-ZH4kjMg+KaQF=jT4D9sozvLC0gv8viN2MYw@mail.gmail.com","threadId":"50310","inReplyTo":"xmqqlg2rmazz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-07T23:48:24Z","receivedAt":"2019-02-07T23:48:41Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Feb 7, 2019 at 3:31 PM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > Now, the question: In addition to the two \"from\" headers, how many\n> > \"to\" headers do we emit? In particular, do we emit both a \"copied to\n> > F\" and a \"renamed to F\" header, or just a combined \"renamed/copied to\n> > F\" header?\n>\n> There is only a single path that can be on the \"to\", as there is\n> only one final result, but _how_ the contents got to that path would\n> be different, so to be technically truly correct, you would end up\n> showing N \"to\" lines for a N-way merge, each of which gives the same\n> path in the postimage, but some may say renamed, another may say\n> copioed and some others may need to say in-place edited.\n>\n> That would increase the number of necessary lines from N (from) + 1\n> (to) to N*2 (N for each of from and to), which makes it even less\n> economical.\n>\n> And showing a single \"renamed/copied\" feels more like a cop-out to\n> avoid being techincally incorrect, than giving a useful piece of\n> information to the users.\n>\n> I am inclined to say that we should do _without_ any \"to\" line. And\n\nWorks for me.\n\n> if we can do without any \"to\", perhaps we do not need \"from\", either.\n\nI would be okay with that, but looking through a few of them, I think\nthe \"from\" lines do help a little, just because lengthy filenames are\nnot uncommon and it's easy to miss the fact that there is a rename,\noften in some directory somewhere in the middle. A couple examples:\n\n\ndiff --combined packages/search/ete/src/test/resources/test-suite.yml\nindex 4ae66932ef9,1cc55276946..1cc55276946\nrename from packages/search/src/geb/resources/test-suite.yml\n--- a/packages/search/ete/src/test/resources/test-suite.yml\n--- a/packages/search/src/geb/resources/test-suite.yml\n+++ b/packages/search/ete/src/test/resources/test-suite.yml\n@@@ -1,15 -1,5 +1,5 @@@\n\n\ndiff --combined packages/search/ete/var/conf/app/public/searchApp.yml\nindex 38cc5482e5b,487178ac0a9..487178ac0a9\nrename from packages/search/ete/var/conf/app/public/searchApp\nrename from packages/search/var/conf/app/public/searchApp.yml\n--- a/packages/search/ete/var/conf/app/public/searchApp\n--- a/packages/search/var/conf/app/public/searchApp.yml\n+++ b/packages/search/ete/var/conf/app/public/searchApp.yml\n\n\nIf there's no renames or copies, then I don't add anything to the\ncombined diff. With renames or copies, people can get the \"to\" name\nfrom looking at the \"+++ b/\" line.\n\nBut not a real big deal, I could just omit this if you prefer.\n"},{"id":"368757","messageId":"CABPp-BHN+RF+jF8ATpRY715f-YxBoaN42r1J0HxM0BwZFgc7Tw@mail.gmail.com","threadId":"50310","inReplyTo":"xmqqtvhfmdxf.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4] log,diff-tree: add --combined-all-names option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-07T23:48:51Z","receivedAt":"2019-02-07T23:49:06Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Feb 7, 2019 at 2:28 PM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\n> > index 231105cff4..69cb3b0349 100644\n> > --- a/Documentation/diff-generate-patch.txt\n> > +++ b/Documentation/diff-generate-patch.txt\n> > @@ -144,6 +144,19 @@ Similar to two-line header for traditional 'unified' diff\n> > format, `/dev/null` is used to signal created or deleted\n> > files.\n> >\n> > +However, if the --combined-all-paths option is provided, instead of a\n> > +two-line from-file/to-file you get a N+1 line from-file/to-file header,\n> > +where N is the number of parents in the merge commit\n> > +\n> > + --- a/file\n> > + --- a/file\n> > + --- a/file\n> > + +++ b/file\n> > ++\n> > +This extended format can be useful if rename or copy detection is\n> > +active, to allow you to see the original name of the file in different\n> > +parents.\n> > +\n> > 4. Chunk header format is modified to prevent people from\n> > accidentally feeding it to `patch -p1`. Combined diff format\n> > was created for review of merge commit changes, and was not\n>\n> You need to replace the blank line before the new paragraph that\n> begins with \"However\" with a line with a single \"+\" on it, to tell\n> the formatter that the new text is still part of the third item in\n> the list.\n\nSorry about that; including that in my fixes now.\n"},{"id":"368763","messageId":"20190208011247.21021-3-newren@gmail.com","threadId":"50310","inReplyTo":"20190208011247.21021-1-newren@gmail.com","subject":"[PATCH v5 2/2] squash! log,diff-tree: add --combined-all-paths option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-08T01:12:47Z","receivedAt":"2019-02-08T01:13:06Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This also adds \"rename from \" and \"copy from \"\nextended headers when renames or copies are involved.\n\nSigned-off-by: Elijah Newren \n---\n Documentation/diff-generate-patch.txt | 7 +++----\n combine-diff.c | 15 +++++++++++++++\n 2 files changed, 18 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex f10ca410ad..f0c2a17aef 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -128,12 +128,11 @@ or like this (when `--cc` option is used):\n mode ,..\n new file mode \n deleted file mode ,\n+ copy from \n+ rename from \n +\n The `mode ,..` line appears only if at least one of\n-the is different from the rest. Extended headers with\n-information about detected contents movement (renames and\n-copying detection) are designed to work with diff of two\n- and are not used by combined diff format.\n+the is different from the rest.\n \n 3. It is followed by two-line from-file/to-file header\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex 54cb892ae5..04a139ea03 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -981,6 +981,21 @@ static void show_combined_header(struct combine_diff_path *elem,\n \t\tprintf(\"%s\\n\", c_reset);\n \t}\n \n+\tfor (i = 0; i < num_parent; i++) {\n+\t\tswitch (elem->parent[i].status) {\n+\t\tcase DIFF_STATUS_COPIED:\n+\t\t\tdump_quoted_path(\"copy from \", \"\",\n+\t\t\t\t\t elem->parent[i].path.buf,\n+\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\t\tbreak;\n+\t\tcase DIFF_STATUS_RENAMED:\n+\t\t\tdump_quoted_path(\"rename from \", \"\",\n+\t\t\t\t\t elem->parent[i].path.buf,\n+\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n \tif (!show_file_header)\n \t\treturn;\n \n-- \n2.20.1.311.gb8408a6075\n\n"},{"id":"368764","messageId":"20190208011247.21021-2-newren@gmail.com","threadId":"50310","inReplyTo":"20190208011247.21021-1-newren@gmail.com","subject":"[PATCH v5 1/2] log,diff-tree: add --combined-all-paths option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-08T01:12:46Z","receivedAt":"2019-02-08T01:13:07Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The combined diff format for merges will only list one filename, even if\nrename or copy detection is active. For example, with raw format one\nmight see:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n\nThis doesn't let us know what the original name of bar.sh was in the\nfirst parent, and doesn't let us know what either of the original names\nof phooey.c were in either of the parents. In contrast, for non-merge\ncommits, raw format does provide original filenames (and a rename score\nto boot). In order to also provide original filenames for merge\ncommits, add a --combined-all-paths option (which must be used with\neither -c or --cc, and is likely only useful with rename or copy\ndetection active) so that we can print tab-separated filenames when\nrenames are involved. This transforms the above output to:\n\n ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\tdesc.c\tdesc.c\n ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\tbar.sh\n ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n\nFurther, in patch format, this changes the from/to headers so that\ninstead of just having one \"from\" header, we get one for each parent.\nFor example, instead of having\n\n --- a/phooey.c\n +++ b/phooey.c\n\nwe would see\n\n --- a/fooey.c\n --- a/fuey.c\n +++ b/phooey.c\n\nSigned-off-by: Elijah Newren \n---\n Documentation/diff-format.txt | 20 +++++-\n Documentation/diff-generate-patch.txt | 13 ++++\n Documentation/git-diff-tree.txt | 11 +++-\n Documentation/rev-list-options.txt | 7 +++\n builtin/diff-tree.c | 6 +-\n combine-diff.c | 76 +++++++++++++++++++----\n diff.h | 1 +\n revision.c | 6 ++\n revision.h | 1 +\n t/t4038-diff-combined.sh | 88 +++++++++++++++++++++++++++\n 10 files changed, 212 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt\nindex cdcc17f0ad..715cbb7604 100644\n--- a/Documentation/diff-format.txt\n+++ b/Documentation/diff-format.txt\n@@ -95,12 +95,26 @@ from the format described above in the following way:\n . there are more \"src\" modes and \"src\" sha1\n . status is concatenated status characters for each parent\n . no optional \"score\" number\n-. single path, only for \"dst\"\n+. tab-separated pathname(s) of the file\n \n-Example:\n+For `-c` and `--cc`, only the destination or final path is shown even\n+if the file was renamed on any side of history. With\n+`--combined-all-paths`, the name of the path in each parent is shown\n+followed by the name of the path in the merge commit.\n+\n+Examples for `-c` and `-cc` without `--combined-all-paths`:\n+------------------------------------------------\n+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n+------------------------------------------------\n+\n+Examples when `--combined-all-paths` added to either `-c` or `--cc`:\n \n ------------------------------------------------\n-::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\tdesc.c\tdesc.c\n+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tfoo.sh\tbar.sh\tbar.sh\n+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tfooey.c\tfuey.c\tphooey.c\n ------------------------------------------------\n \n Note that 'combined diff' lists only files which were modified from\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 231105cff4..f10ca410ad 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -143,6 +143,19 @@ copying detection) are designed to work with diff of two\n Similar to two-line header for traditional 'unified' diff\n format, `/dev/null` is used to signal created or deleted\n files.\n++\n+However, if the --combined-all-paths option is provided, instead of a\n+two-line from-file/to-file you get a N+1 line from-file/to-file header,\n+where N is the number of parents in the merge commit\n+\n+ --- a/file\n+ --- a/file\n+ --- a/file\n+ +++ b/file\n++\n+This extended format can be useful if rename or copy detection is\n+active, to allow you to see the original name of the file in different\n+parents.\n \n 4. Chunk header format is modified to prevent people from\n accidentally feeding it to `patch -p1`. Combined diff format\ndiff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt\nindex 2319b2b192..28b93ecd54 100644\n--- a/Documentation/git-diff-tree.txt\n+++ b/Documentation/git-diff-tree.txt\n@@ -10,8 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git diff-tree' [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty]\n-\t [-t] [-r] [-c | --cc] [--root] []\n-\t [] [...]\n+\t [-t] [-r] [-c | --cc] [--combined-all-paths] [--root]\n+\t [] [] [...]\n \n DESCRIPTION\n -----------\n@@ -108,6 +108,13 @@ include::pretty-options.txt[]\n \titself and the commit log message is not shown, just like in any other\n \t\"empty diff\" case.\n \n+--combined-all-paths::\n+\tThis flag causes combined diffs (used for merge commits) to\n+\tlist the name of the file from all parents. It thus only has\n+\teffect when -c or --cc are specified, and is likely only\n+\tuseful if filename changes are detected (i.e. when either\n+\trename or copy detection have been requested).\n+\n --always::\n \tShow the commit itself and the commit log message even\n \tif the diff itself is empty.\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 98b538bc77..642ce943c1 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -948,6 +948,13 @@ options may be given. See linkgit:git-diff-files[1] for more options.\n \tthe parents have only two variants and the merge result picks\n \tone of them without modification.\n \n+--combined-all-paths::\n+\tThis flag causes combined diffs (used for merge commits) to\n+\tlist the name of the file from all parents. It thus only has\n+\teffect when -c or --cc are specified, and is likely only\n+\tuseful if filename changes are detected (i.e. when either\n+\trename or copy detection have been requested).\n+\n -m::\n \tThis flag makes the merge commits show the full diff like\n \tregular commits; for each merge parent, a separate log entry\ndiff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\nindex ef996126d7..ebe48f60c1 100644\n--- a/builtin/diff-tree.c\n+++ b/builtin/diff-tree.c\n@@ -82,9 +82,13 @@ static int diff_tree_stdin(char *line)\n }\n \n static const char diff_tree_usage[] =\n-\"git diff-tree [--stdin] [-m] [-c] [--cc] [-s] [-v] [--pretty] [-t] [-r] [--root] \"\n+\"git diff-tree [--stdin] [-m] [-c | --cc] [-s] [-v] [--pretty] [-t] [-r] [--root] \"\n \"[] [] [...]\\n\"\n \" -r diff recursively\\n\"\n+\" -c show combined diff for merge commits\\n\"\n+\" --cc show combined diff for merge commits removing uninteresting hunks\\n\"\n+\" --combined-all-paths\\n\"\n+\" show name of file in all parents for combined diffs\\n\"\n \" --root include the initial commit as diff against /dev/null\\n\"\n COMMON_DIFF_OPTIONS_HELP;\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex a143c00634..54cb892ae5 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -23,11 +23,20 @@ static int compare_paths(const struct combine_diff_path *one,\n \t\t\t\t two->path, strlen(two->path), two->mode);\n }\n \n-static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)\n+static int filename_changed(char status)\n+{\n+\treturn status == 'R' || status == 'C';\n+}\n+\n+static struct combine_diff_path *intersect_paths(\n+\tstruct combine_diff_path *curr,\n+\tint n,\n+\tint num_parent,\n+\tint combined_all_paths)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct combine_diff_path *p, **tail = &curr;\n-\tint i, cmp;\n+\tint i, j, cmp;\n \n \tif (!n) {\n \t\tfor (i = 0; i < q->nr; i++) {\n@@ -50,6 +59,13 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\t\tp->parent[n].status = q->queue[i]->status;\n+\n+\t\t\tif (combined_all_paths &&\n+\t\t\t filename_changed(p->parent[n].status)) {\n+\t\t\t\tstrbuf_init(&p->parent[n].path, 0);\n+\t\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t\t q->queue[i]->one->path);\n+\t\t\t}\n \t\t\t*tail = p;\n \t\t\ttail = &p->next;\n \t\t}\n@@ -68,6 +84,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\tif (cmp < 0) {\n \t\t\t/* p->path not in q->queue[]; drop it */\n \t\t\t*tail = p->next;\n+\t\t\tfor (j = 0; j < num_parent; j++)\n+\t\t\t\tif (combined_all_paths &&\n+\t\t\t\t filename_changed(p->parent[j].status))\n+\t\t\t\t\tstrbuf_release(&p->parent[j].path);\n \t\t\tfree(p);\n \t\t\tcontinue;\n \t\t}\n@@ -81,6 +101,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\tp->parent[n].status = q->queue[i]->status;\n+\t\tif (combined_all_paths &&\n+\t\t filename_changed(p->parent[n].status))\n+\t\t\tstrbuf_addstr(&p->parent[n].path,\n+\t\t\t\t q->queue[i]->one->path);\n \n \t\ttail = &p->next;\n \t\ti++;\n@@ -960,12 +984,25 @@ static void show_combined_header(struct combine_diff_path *elem,\n \tif (!show_file_header)\n \t\treturn;\n \n-\tif (added)\n-\t\tdump_quoted_path(\"--- \", \"\", \"/dev/null\",\n-\t\t\t\t line_prefix, c_meta, c_reset);\n-\telse\n-\t\tdump_quoted_path(\"--- \", a_prefix, elem->path,\n-\t\t\t\t line_prefix, c_meta, c_reset);\n+\tif (rev->combined_all_paths) {\n+\t\tfor (i = 0; i < num_parent; i++) {\n+\t\t\tchar *path = filename_changed(elem->parent[i].status)\n+\t\t\t\t? elem->parent[i].path.buf : elem->path;\n+\t\t\tif (elem->parent[i].status == DIFF_STATUS_ADDED)\n+\t\t\t\tdump_quoted_path(\"--- \", \"\", \"/dev/null\",\n+\t\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\t\telse\n+\t\t\t\tdump_quoted_path(\"--- \", a_prefix, path,\n+\t\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\t}\n+\t} else {\n+\t\tif (added)\n+\t\t\tdump_quoted_path(\"--- \", \"\", \"/dev/null\",\n+\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t\telse\n+\t\t\tdump_quoted_path(\"--- \", a_prefix, elem->path,\n+\t\t\t\t\t line_prefix, c_meta, c_reset);\n+\t}\n \tif (deleted)\n \t\tdump_quoted_path(\"+++ \", \"\", \"/dev/null\",\n \t\t\t\t line_prefix, c_meta, c_reset);\n@@ -1227,6 +1264,15 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re\n \t\tputchar(inter_name_termination);\n \t}\n \n+\tfor (i = 0; i < num_parent; i++)\n+\t\tif (rev->combined_all_paths) {\n+\t\t\tif (filename_changed(p->parent[i].status))\n+\t\t\t\twrite_name_quoted(p->parent[i].path.buf, stdout,\n+\t\t\t\t\t\t inter_name_termination);\n+\t\t\telse\n+\t\t\t\twrite_name_quoted(p->path, stdout,\n+\t\t\t\t\t\t inter_name_termination);\n+\t\t}\n \twrite_name_quoted(p->path, stdout, line_termination);\n }\n \n@@ -1324,7 +1370,9 @@ static const char *path_path(void *obj)\n \n /* find set of paths that every parent touches */\n static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n-\tconst struct oid_array *parents, struct diff_options *opt)\n+\tconst struct oid_array *parents,\n+\tstruct diff_options *opt,\n+\tint combined_all_paths)\n {\n \tstruct combine_diff_path *paths = NULL;\n \tint i, num_parent = parents->nr;\n@@ -1350,7 +1398,8 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n \t\t\topt->output_format = DIFF_FORMAT_NO_OUTPUT;\n \t\tdiff_tree_oid(&parents->oid[i], oid, \"\", opt);\n \t\tdiffcore_std(opt);\n-\t\tpaths = intersect_paths(paths, i, num_parent);\n+\t\tpaths = intersect_paths(paths, i, num_parent,\n+\t\t\t\t\tcombined_all_paths);\n \n \t\t/* if showing diff, show it in requested order */\n \t\tif (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&\n@@ -1460,7 +1509,8 @@ void diff_tree_combined(const struct object_id *oid,\n \t\t * diff(sha1,parent_i) for all i to do the job, specifically\n \t\t * for parent0.\n \t\t */\n-\t\tpaths = find_paths_generic(oid, parents, &diffopts);\n+\t\tpaths = find_paths_generic(oid, parents, &diffopts,\n+\t\t\t\t\t rev->combined_all_paths);\n \t}\n \telse {\n \t\tint stat_opt;\n@@ -1535,6 +1585,10 @@ void diff_tree_combined(const struct object_id *oid,\n \twhile (paths) {\n \t\tstruct combine_diff_path *tmp = paths;\n \t\tpaths = paths->next;\n+\t\tfor (i = 0; i < num_parent; i++)\n+\t\t\tif (rev->combined_all_paths &&\n+\t\t\t filename_changed(tmp->parent[i].status))\n+\t\t\t\tstrbuf_release(&tmp->parent[i].path);\n \t\tfree(tmp);\n \t}\n \ndiff --git a/diff.h b/diff.h\nindex b512d0477a..90ea0256a5 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -294,6 +294,7 @@ struct combine_diff_path {\n \t\tchar status;\n \t\tunsigned int mode;\n \t\tstruct object_id oid;\n+\t\tstruct strbuf path;\n \t} parent[FLEX_ARRAY];\n };\n #define combine_diff_path_size(n, l) \\\ndiff --git a/revision.c b/revision.c\nindex 13cfb59b38..3be3f7da83 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1995,6 +1995,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 0;\n \t\trevs->combine_merges = 1;\n+\t} else if (!strcmp(arg, \"--combined-all-paths\")) {\n+\t\trevs->diff = 1;\n+\t\trevs->combined_all_paths = 1;\n \t} else if (!strcmp(arg, \"--cc\")) {\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 1;\n@@ -2491,6 +2494,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t}\n \tif (revs->combine_merges)\n \t\trevs->ignore_merges = 0;\n+\tif (revs->combined_all_paths && !revs->combine_merges)\n+\t\tdie(\"--combined-all-paths makes no sense without -c or --cc\");\n+\n \trevs->diffopt.abbrev = revs->abbrev;\n \n \tif (revs->line_level_traverse) {\ndiff --git a/revision.h b/revision.h\nindex 52e5a88ff5..253c12c29a 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -171,6 +171,7 @@ struct rev_info {\n \t\t\tverbose_header:1,\n \t\t\tignore_merges:1,\n \t\t\tcombine_merges:1,\n+\t\t\tcombined_all_paths:1,\n \t\t\tdense_combined_merges:1,\n \t\t\talways_show_header:1;\n \ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex e2824d3437..07b49f6d6d 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -435,4 +435,92 @@ test_expect_success 'combine diff gets tree sorting right' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup for --combined-all-paths' '\n+\tgit branch side1c &&\n+\tgit branch side2c &&\n+\tgit checkout side1c &&\n+\ttest_seq 1 10 >filename-side1c &&\n+\tgit add filename-side1c &&\n+\tgit commit -m with &&\n+\tgit checkout side2c &&\n+\ttest_seq 1 9 >filename-side2c &&\n+\techo ten >>filename-side2c &&\n+\tgit add filename-side2c &&\n+\tgit commit -m iam &&\n+\tgit checkout -b mergery side1c &&\n+\tgit merge --no-commit side2c &&\n+\tgit rm filename-side1c &&\n+\techo eleven >>filename-side2c &&\n+\tgit mv filename-side2c filename-merged &&\n+\tgit add filename-merged &&\n+\tgit commit\n+'\n+\n+test_expect_success '--combined-all-paths and --raw' '\n+\tcat <<-\\EOF >expect &&\n+\t::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\tfilename-side1c\tfilename-side2c\tfilename-merged\n+\tEOF\n+\tgit diff-tree -c -M --raw --combined-all-paths HEAD >actual.tmp &&\n+\tsed 1d actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--combined-all-paths and --cc' '\n+\tcat <<-\\EOF >expect &&\n+\t--- a/filename-side1c\n+\t--- a/filename-side2c\n+\t+++ b/filename-merged\n+\tEOF\n+\tgit diff-tree --cc -M --combined-all-paths HEAD >actual.tmp &&\n+\tgrep ^[-+][-+][-+] actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success FUNNYNAMES 'setup for --combined-all-paths with funny names' '\n+\tgit branch side1d &&\n+\tgit branch side2d &&\n+\tgit checkout side1d &&\n+\ttest_seq 1 10 >$(printf \"file\\twith\\ttabs\") &&\n+\tgit add file* &&\n+\tgit commit -m with &&\n+\tgit checkout side2d &&\n+\ttest_seq 1 9 >$(printf \"i\\tam\\ttabbed\") &&\n+\techo ten >>$(printf \"i\\tam\\ttabbed\") &&\n+\tgit add *tabbed &&\n+\tgit commit -m iam &&\n+\tgit checkout -b funny-names-mergery side1d &&\n+\tgit merge --no-commit side2d &&\n+\tgit rm *tabs &&\n+\techo eleven >>$(printf \"i\\tam\\ttabbed\") &&\n+\tgit mv \"$(printf \"i\\tam\\ttabbed\")\" \"$(printf \"fickle\\tnaming\")\" &&\n+\tgit add fickle* &&\n+\tgit commit\n+'\n+\n+test_expect_success FUNNYNAMES '--combined-all-paths and --raw and funny names' '\n+\tcat <<-\\EOF >expect &&\n+\t::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\t\"file\\twith\\ttabs\"\t\"i\\tam\\ttabbed\"\t\"fickle\\tnaming\"\n+\tEOF\n+\tgit diff-tree -c -M --raw --combined-all-paths HEAD >actual.tmp &&\n+\tsed 1d actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success FUNNYNAMES '--combined-all-paths and --raw -and -z and funny names' '\n+\tprintf \"aaf8087c3cbd4db8e185a2d074cf27c53cfb75d7\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n+\tgit diff-tree -c -M --raw --combined-all-paths -z HEAD >actual &&\n+\ttest_cmp -a expect actual\n+'\n+\n+test_expect_success FUNNYNAMES '--combined-all-paths and --cc and funny names' '\n+\tcat <<-\\EOF >expect &&\n+\t--- \"a/file\\twith\\ttabs\"\n+\t--- \"a/i\\tam\\ttabbed\"\n+\t+++ \"b/fickle\\tnaming\"\n+\tEOF\n+\tgit diff-tree --cc -M --combined-all-paths HEAD >actual.tmp &&\n+\tgrep ^[-+][-+][-+] actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.20.1.311.gb8408a6075\n\n"},{"id":"368765","messageId":"20190208011247.21021-1-newren@gmail.com","threadId":"50310","inReplyTo":"20190204200754.16413-1-newren@gmail.com","subject":"[PATCH v5 0/2] add --combined-all-paths option to log and diff-tree","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-08T01:12:45Z","receivedAt":"2019-02-08T01:13:08Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Changes since v4:\n * Added a second patch that can be squashed in which will add\n 'rename from' and 'copy from' extended headers. I like it, but\n Junio sounded pessimistic about it. See\n https://public-inbox.org/git/xmqqlg2rmazz.fsf@gitster-ct.c.googlers.com/\n * Micro fixes:\n * Renamed --combined-all-names to --combined-all-paths\n * Fixed formatting error (missed '+') in diff-generate-patch.txt\n * Marked tests which used tabs in filenames with FUNNYNAMES prereq\n * Added tests that didn't depend on FUNNYNAMES\n\nElijah Newren (2):\n log,diff-tree: add --combined-all-paths option\n squash! log,diff-tree: add --combined-all-paths option\n\n Documentation/diff-format.txt | 20 +++++-\n Documentation/diff-generate-patch.txt | 20 ++++--\n Documentation/git-diff-tree.txt | 11 +++-\n Documentation/rev-list-options.txt | 7 +++\n builtin/diff-tree.c | 6 +-\n combine-diff.c | 91 +++++++++++++++++++++++----\n diff.h | 1 +\n revision.c | 6 ++\n revision.h | 1 +\n t/t4038-diff-combined.sh | 88 ++++++++++++++++++++++++++\n 10 files changed, 230 insertions(+), 21 deletions(-)\n\nRange-diff:\n1: 26c64cee8a ! 1: 2205640429 log,diff-tree: add --combined-all-names option\n @@ -1,6 +1,6 @@\n Author: Elijah Newren \n \n - log,diff-tree: add --combined-all-names option\n + log,diff-tree: add --combined-all-paths option\n \n The combined diff format for merges will only list one filename, even if\n rename or copy detection is active. For example, with raw format one\n @@ -15,7 +15,7 @@\n of phooey.c were in either of the parents. In contrast, for non-merge\n commits, raw format does provide original filenames (and a rename score\n to boot). In order to also provide original filenames for merge\n - commits, add a --combined-all-names option (which must be used with\n + commits, add a --combined-all-paths option (which must be used with\n either -c or --cc, and is likely only useful with rename or copy\n detection active) so that we can print tab-separated filenames when\n renames are involved. This transforms the above output to:\n @@ -38,8 +38,6 @@\n +++ b/phooey.c\n \n Signed-off-by: Elijah Newren \n - Signed-off-by: Junio C Hamano \n - Message-Id: <20190204200754.16413-1-newren@gmail.com>\n \n diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt\n --- a/Documentation/diff-format.txt\n @@ -54,17 +52,17 @@\n -Example:\n +For `-c` and `--cc`, only the destination or final path is shown even\n +if the file was renamed on any side of history. With\n -+`--combined-all-names`, the name of the path in each parent is shown\n ++`--combined-all-paths`, the name of the path in each parent is shown\n +followed by the name of the path in the merge commit.\n +\n -+Examples for `-c` and `-cc` without `--combined-all-names`:\n ++Examples for `-c` and `-cc` without `--combined-all-paths`:\n +------------------------------------------------\n +::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdesc.c\n +::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM\tbar.sh\n +::100644 100644 100644 e07d6c5 9042e82 ee91881 RR\tphooey.c\n +------------------------------------------------\n +\n -+Examples when `--combined-all-names` added to either `-c` or `--cc`:\n ++Examples when `--combined-all-paths` added to either `-c` or `--cc`:\n \n ------------------------------------------------\n -::100644 100644 100644 fabadb8 cc95eb0 4866510 MM\tdescribe.c\n @@ -79,9 +77,10 @@\n --- a/Documentation/diff-generate-patch.txt\n +++ b/Documentation/diff-generate-patch.txt\n @@\n + Similar to two-line header for traditional 'unified' diff\n format, `/dev/null` is used to signal created or deleted\n files.\n - \n +++\n +However, if the --combined-all-paths option is provided, instead of a\n +two-line from-file/to-file you get a N+1 line from-file/to-file header,\n +where N is the number of parents in the merge commit\n @@ -94,10 +93,9 @@\n +This extended format can be useful if rename or copy detection is\n +active, to allow you to see the original name of the file in different\n +parents.\n -+\n + \n 4. Chunk header format is modified to prevent people from\n accidentally feeding it to `patch -p1`. Combined diff format\n - was created for review of merge commit changes, and was not\n \n diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt\n --- a/Documentation/git-diff-tree.txt\n @@ -108,7 +106,7 @@\n 'git diff-tree' [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty]\n -\t [-t] [-r] [-c | --cc] [--root] []\n -\t [] [...]\n -+\t [-t] [-r] [-c | --cc] [--combined-all-names] [--root]\n ++\t [-t] [-r] [-c | --cc] [--combined-all-paths] [--root]\n +\t [] [] [...]\n \n DESCRIPTION\n @@ -117,7 +115,7 @@\n \titself and the commit log message is not shown, just like in any other\n \t\"empty diff\" case.\n \n -+--combined-all-names::\n ++--combined-all-paths::\n +\tThis flag causes combined diffs (used for merge commits) to\n +\tlist the name of the file from all parents. It thus only has\n +\teffect when -c or --cc are specified, and is likely only\n @@ -135,7 +133,7 @@\n \tthe parents have only two variants and the merge result picks\n \tone of them without modification.\n \n -+--combined-all-names::\n ++--combined-all-paths::\n +\tThis flag causes combined diffs (used for merge commits) to\n +\tlist the name of the file from all parents. It thus only has\n +\teffect when -c or --cc are specified, and is likely only\n @@ -159,7 +157,7 @@\n \" -r diff recursively\\n\"\n +\" -c show combined diff for merge commits\\n\"\n +\" --cc show combined diff for merge commits removing uninteresting hunks\\n\"\n -+\" --combined-all-names\\n\"\n ++\" --combined-all-paths\\n\"\n +\" show name of file in all parents for combined diffs\\n\"\n \" --root include the initial commit as diff against /dev/null\\n\"\n COMMON_DIFF_OPTIONS_HELP;\n @@ -182,7 +180,7 @@\n +\tstruct combine_diff_path *curr,\n +\tint n,\n +\tint num_parent,\n -+\tint combined_all_names)\n ++\tint combined_all_paths)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct combine_diff_path *p, **tail = &curr;\n @@ -196,7 +194,7 @@\n \t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\t\tp->parent[n].status = q->queue[i]->status;\n +\n -+\t\t\tif (combined_all_names &&\n ++\t\t\tif (combined_all_paths &&\n +\t\t\t filename_changed(p->parent[n].status)) {\n +\t\t\t\tstrbuf_init(&p->parent[n].path, 0);\n +\t\t\t\tstrbuf_addstr(&p->parent[n].path,\n @@ -210,7 +208,7 @@\n \t\t\t/* p->path not in q->queue[]; drop it */\n \t\t\t*tail = p->next;\n +\t\t\tfor (j = 0; j < num_parent; j++)\n -+\t\t\t\tif (combined_all_names &&\n ++\t\t\t\tif (combined_all_paths &&\n +\t\t\t\t filename_changed(p->parent[j].status))\n +\t\t\t\t\tstrbuf_release(&p->parent[j].path);\n \t\t\tfree(p);\n @@ -220,7 +218,7 @@\n \t\toidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);\n \t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\tp->parent[n].status = q->queue[i]->status;\n -+\t\tif (combined_all_names &&\n ++\t\tif (combined_all_paths &&\n +\t\t filename_changed(p->parent[n].status))\n +\t\t\tstrbuf_addstr(&p->parent[n].path,\n +\t\t\t\t q->queue[i]->one->path);\n @@ -237,7 +235,7 @@\n -\telse\n -\t\tdump_quoted_path(\"--- \", a_prefix, elem->path,\n -\t\t\t\t line_prefix, c_meta, c_reset);\n -+\tif (rev->combined_all_names) {\n ++\tif (rev->combined_all_paths) {\n +\t\tfor (i = 0; i < num_parent; i++) {\n +\t\t\tchar *path = filename_changed(elem->parent[i].status)\n +\t\t\t\t? elem->parent[i].path.buf : elem->path;\n @@ -264,7 +262,7 @@\n \t}\n \n +\tfor (i = 0; i < num_parent; i++)\n -+\t\tif (rev->combined_all_names) {\n ++\t\tif (rev->combined_all_paths) {\n +\t\t\tif (filename_changed(p->parent[i].status))\n +\t\t\t\twrite_name_quoted(p->parent[i].path.buf, stdout,\n +\t\t\t\t\t\t inter_name_termination);\n @@ -282,7 +280,7 @@\n -\tconst struct oid_array *parents, struct diff_options *opt)\n +\tconst struct oid_array *parents,\n +\tstruct diff_options *opt,\n -+\tint combined_all_names)\n ++\tint combined_all_paths)\n {\n \tstruct combine_diff_path *paths = NULL;\n \tint i, num_parent = parents->nr;\n @@ -292,7 +290,7 @@\n \t\tdiffcore_std(opt);\n -\t\tpaths = intersect_paths(paths, i, num_parent);\n +\t\tpaths = intersect_paths(paths, i, num_parent,\n -+\t\t\t\t\tcombined_all_names);\n ++\t\t\t\t\tcombined_all_paths);\n \n \t\t/* if showing diff, show it in requested order */\n \t\tif (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&\n @@ -302,7 +300,7 @@\n \t\t */\n -\t\tpaths = find_paths_generic(oid, parents, &diffopts);\n +\t\tpaths = find_paths_generic(oid, parents, &diffopts,\n -+\t\t\t\t\t rev->combined_all_names);\n ++\t\t\t\t\t rev->combined_all_paths);\n \t}\n \telse {\n \t\tint stat_opt;\n @@ -311,7 +309,7 @@\n \t\tstruct combine_diff_path *tmp = paths;\n \t\tpaths = paths->next;\n +\t\tfor (i = 0; i < num_parent; i++)\n -+\t\t\tif (rev->combined_all_names &&\n ++\t\t\tif (rev->combined_all_paths &&\n +\t\t\t filename_changed(tmp->parent[i].status))\n +\t\t\t\tstrbuf_release(&tmp->parent[i].path);\n \t\tfree(tmp);\n @@ -337,9 +335,9 @@\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 0;\n \t\trevs->combine_merges = 1;\n -+\t} else if (!strcmp(arg, \"--combined-all-names\")) {\n ++\t} else if (!strcmp(arg, \"--combined-all-paths\")) {\n +\t\trevs->diff = 1;\n -+\t\trevs->combined_all_names = 1;\n ++\t\trevs->combined_all_paths = 1;\n \t} else if (!strcmp(arg, \"--cc\")) {\n \t\trevs->diff = 1;\n \t\trevs->dense_combined_merges = 1;\n @@ -347,8 +345,8 @@\n \t}\n \tif (revs->combine_merges)\n \t\trevs->ignore_merges = 0;\n -+\tif (revs->combined_all_names && !revs->combine_merges)\n -+\t\tdie(\"--combined-all-names makes no sense without -c or --cc\");\n ++\tif (revs->combined_all_paths && !revs->combine_merges)\n ++\t\tdie(\"--combined-all-paths makes no sense without -c or --cc\");\n +\n \trevs->diffopt.abbrev = revs->abbrev;\n \n @@ -361,7 +359,7 @@\n \t\t\tverbose_header:1,\n \t\t\tignore_merges:1,\n \t\t\tcombine_merges:1,\n -+\t\t\tcombined_all_names:1,\n ++\t\t\tcombined_all_paths:1,\n \t\t\tdense_combined_merges:1,\n \t\t\talways_show_header:1;\n \n @@ -373,20 +371,61 @@\n \ttest_cmp expect actual\n '\n \n -+test_expect_success 'setup for --combined-with-paths' '\n ++test_expect_success 'setup for --combined-all-paths' '\n +\tgit branch side1c &&\n +\tgit branch side2c &&\n +\tgit checkout side1c &&\n ++\ttest_seq 1 10 >filename-side1c &&\n ++\tgit add filename-side1c &&\n ++\tgit commit -m with &&\n ++\tgit checkout side2c &&\n ++\ttest_seq 1 9 >filename-side2c &&\n ++\techo ten >>filename-side2c &&\n ++\tgit add filename-side2c &&\n ++\tgit commit -m iam &&\n ++\tgit checkout -b mergery side1c &&\n ++\tgit merge --no-commit side2c &&\n ++\tgit rm filename-side1c &&\n ++\techo eleven >>filename-side2c &&\n ++\tgit mv filename-side2c filename-merged &&\n ++\tgit add filename-merged &&\n ++\tgit commit\n ++'\n ++\n ++test_expect_success '--combined-all-paths and --raw' '\n ++\tcat <<-\\EOF >expect &&\n ++\t::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\tfilename-side1c\tfilename-side2c\tfilename-merged\n ++\tEOF\n ++\tgit diff-tree -c -M --raw --combined-all-paths HEAD >actual.tmp &&\n ++\tsed 1d actual &&\n ++\ttest_cmp expect actual\n ++'\n ++\n ++test_expect_success '--combined-all-paths and --cc' '\n ++\tcat <<-\\EOF >expect &&\n ++\t--- a/filename-side1c\n ++\t--- a/filename-side2c\n ++\t+++ b/filename-merged\n ++\tEOF\n ++\tgit diff-tree --cc -M --combined-all-paths HEAD >actual.tmp &&\n ++\tgrep ^[-+][-+][-+] actual &&\n ++\ttest_cmp expect actual\n ++'\n ++\n ++test_expect_success FUNNYNAMES 'setup for --combined-all-paths with funny names' '\n ++\tgit branch side1d &&\n ++\tgit branch side2d &&\n ++\tgit checkout side1d &&\n +\ttest_seq 1 10 >$(printf \"file\\twith\\ttabs\") &&\n +\tgit add file* &&\n +\tgit commit -m with &&\n -+\tgit checkout side2c &&\n ++\tgit checkout side2d &&\n +\ttest_seq 1 9 >$(printf \"i\\tam\\ttabbed\") &&\n +\techo ten >>$(printf \"i\\tam\\ttabbed\") &&\n +\tgit add *tabbed &&\n +\tgit commit -m iam &&\n -+\tgit checkout -b mergery side1c &&\n -+\tgit merge --no-commit side2c &&\n ++\tgit checkout -b funny-names-mergery side1d &&\n ++\tgit merge --no-commit side2d &&\n +\tgit rm *tabs &&\n +\techo eleven >>$(printf \"i\\tam\\ttabbed\") &&\n +\tgit mv \"$(printf \"i\\tam\\ttabbed\")\" \"$(printf \"fickle\\tnaming\")\" &&\n @@ -394,28 +433,28 @@\n +\tgit commit\n +'\n +\n -+test_expect_success '--combined-all-names and --raw' '\n ++test_expect_success FUNNYNAMES '--combined-all-paths and --raw and funny names' '\n +\tcat <<-\\EOF >expect &&\n +\t::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\t\"file\\twith\\ttabs\"\t\"i\\tam\\ttabbed\"\t\"fickle\\tnaming\"\n +\tEOF\n -+\tgit diff-tree -c -M --raw --combined-all-names HEAD >actual.tmp &&\n ++\tgit diff-tree -c -M --raw --combined-all-paths HEAD >actual.tmp &&\n +\tsed 1d actual &&\n +\ttest_cmp expect actual\n +'\n +\n -+test_expect_success '--combined-all-names and --raw -and -z' '\n -+\tprintf \"0f9645804ebb04cc3eef91f799eb7fb54d70cefb\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n -+\tgit diff-tree -c -M --raw --combined-all-names -z HEAD >actual &&\n ++test_expect_success FUNNYNAMES '--combined-all-paths and --raw -and -z and funny names' '\n ++\tprintf \"aaf8087c3cbd4db8e185a2d074cf27c53cfb75d7\\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\\0file\\twith\\ttabs\\0i\\tam\\ttabbed\\0fickle\\tnaming\\0\" >expect &&\n ++\tgit diff-tree -c -M --raw --combined-all-paths -z HEAD >actual &&\n +\ttest_cmp -a expect actual\n +'\n +\n -+test_expect_success '--combined-all-names and --cc' '\n ++test_expect_success FUNNYNAMES '--combined-all-paths and --cc and funny names' '\n +\tcat <<-\\EOF >expect &&\n +\t--- \"a/file\\twith\\ttabs\"\n +\t--- \"a/i\\tam\\ttabbed\"\n +\t+++ \"b/fickle\\tnaming\"\n +\tEOF\n -+\tgit diff-tree --cc -M --combined-all-names HEAD >actual.tmp &&\n ++\tgit diff-tree --cc -M --combined-all-paths HEAD >actual.tmp &&\n +\tgrep ^[-+][-+][-+] actual &&\n +\ttest_cmp expect actual\n +'\n2: d93e6c5fee < -: ---------- SQUASH??? fix mark-up\n-: ---------- > 2: b8408a6075 squash! log,diff-tree: add --combined-all-paths option\n-- \n2.20.1.311.gb8408a6075\n\n"},{"id":"368774","messageId":"xmqq8syrlyj5.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"20190208011247.21021-2-newren@gmail.com","subject":"Re: [PATCH v5 1/2] log,diff-tree: add --combined-all-paths option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-08T04:00:46Z","receivedAt":"2019-02-08T04:00:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> +For `-c` and `--cc`, only the destination or final path is shown even\n> +if the file was renamed on any side of history. With\n> +`--combined-all-paths`, the name of the path in each parent is shown\n> +followed by the name of the path in the merge commit.\n> +\n> +Examples for `-c` and `-cc` without `--combined-all-paths`:\n\ns/-cc/--cc/\n\n"},{"id":"368775","messageId":"xmqq4l9ench8.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"20190208011247.21021-3-newren@gmail.com","subject":"Re: [PATCH v5 2/2] squash! log,diff-tree: add --combined-all-paths option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-08T04:14:11Z","receivedAt":"2019-02-08T04:14:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> +\tfor (i = 0; i < num_parent; i++) {\n> +\t\tswitch (elem->parent[i].status) {\n> +\t\tcase DIFF_STATUS_COPIED:\n> +\t\t\tdump_quoted_path(\"copy from \", \"\",\n> +\t\t\t\t\t elem->parent[i].path.buf,\n> +\t\t\t\t\t line_prefix, c_meta, c_reset);\n> +\t\t\tbreak;\n> +\t\tcase DIFF_STATUS_RENAMED:\n> +\t\t\tdump_quoted_path(\"rename from \", \"\",\n> +\t\t\t\t\t elem->parent[i].path.buf,\n> +\t\t\t\t\t line_prefix, c_meta, c_reset);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n\nThe explanation for this addition was that it is hard to tell from\nwhich side a rename happened in the three-dash lines alone:\n\n --- a/packages/search/ete/src/test/resources/test-suite.yml\n --- a/packages/search/src/geb/resources/test-suite.yml\n +++ b/packages/search/ete/src/test/resources/test-suite.yml\n\nand your hope was that adding:\n\n rename from packages/search/src/geb/resources/test-suite.yml\n\nwould help especially when the path is overly long.\n\nBut I am not sure if that single \"rename from\" is all that helpful.\nYou cannot tell relative to which parent the rename happened without\ngoing back to the three-dash lines. A loop that iterates over all\nparents but shows only a line for a parent that actually had copy or\nrename loses \"the line is talking about the change from this parent\"\nwhich is a fairly important piece of information, doesn't it?\n\nIf we attempt to clarify it by adding some more information on the\nline, e.g.\n\n rename relative to parent #1 from packages/search/src/geb/...\n\nthe line gets even longer, and going back to look at\n\n --- a/packages/search/src/geb/resources/test-suite.yml\n\nmay turn out to be an easier way to learn that information.\nSo,... I dunno.\n"},{"id":"368779","messageId":"CABPp-BHHjvC7tv27dBaUNuqXYdmhpbQXLadXTMFWR8GgKH_ngg@mail.gmail.com","threadId":"50310","inReplyTo":"xmqq4l9ench8.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5 2/2] squash! log,diff-tree: add --combined-all-paths option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-08T06:48:22Z","receivedAt":"2019-02-08T06:48:37Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Feb 7, 2019 at 8:14 PM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > + for (i = 0; i < num_parent; i++) {\n> > + switch (elem->parent[i].status) {\n> > + case DIFF_STATUS_COPIED:\n> > + dump_quoted_path(\"copy from \", \"\",\n> > + elem->parent[i].path.buf,\n> > + line_prefix, c_meta, c_reset);\n> > + break;\n> > + case DIFF_STATUS_RENAMED:\n> > + dump_quoted_path(\"rename from \", \"\",\n> > + elem->parent[i].path.buf,\n> > + line_prefix, c_meta, c_reset);\n> > + break;\n> > + }\n> > + }\n>\n> The explanation for this addition was that it is hard to tell from\n> which side a rename happened in the three-dash lines alone:\n>\n> --- a/packages/search/ete/src/test/resources/test-suite.yml\n> --- a/packages/search/src/geb/resources/test-suite.yml\n> +++ b/packages/search/ete/src/test/resources/test-suite.yml\n>\n> and your hope was that adding:\n>\n> rename from packages/search/src/geb/resources/test-suite.yml\n>\n> would help especially when the path is overly long.\n>\n> But I am not sure if that single \"rename from\" is all that helpful.\n> You cannot tell relative to which parent the rename happened without\n> going back to the three-dash lines. A loop that iterates over all\n> parents but shows only a line for a parent that actually had copy or\n> rename loses \"the line is talking about the change from this parent\"\n> which is a fairly important piece of information, doesn't it?\n>\n> If we attempt to clarify it by adding some more information on the\n> line, e.g.\n>\n> rename relative to parent #1 from packages/search/src/geb/...\n>\n> the line gets even longer, and going back to look at\n>\n> --- a/packages/search/src/geb/resources/test-suite.yml\n>\n> may turn out to be an easier way to learn that information.\n> So,... I dunno.\n\nYeah, fair enough. I'll drop the patch.\n"},{"id":"368780","messageId":"CABPp-BECeDCKpcM6DExxbPrn41k_VVqVik=z8WWU53eoFNM0jA@mail.gmail.com","threadId":"50310","inReplyTo":"xmqq8syrlyj5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5 1/2] log,diff-tree: add --combined-all-paths option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-08T06:52:03Z","receivedAt":"2019-02-08T06:52:16Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Feb 7, 2019 at 8:00 PM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > +For `-c` and `--cc`, only the destination or final path is shown even\n> > +if the file was renamed on any side of history. With\n> > +`--combined-all-paths`, the name of the path in each parent is shown\n> > +followed by the name of the path in the merge commit.\n> > +\n> > +Examples for `-c` and `-cc` without `--combined-all-paths`:\n>\n> s/-cc/--cc/\n\nThanks for catching that. I was going to go fixup and resubmit, and\ndrop patch 2 while at it, but it looks like you've already squashed\nthis fix in to commit d76ce4f73 that you've pushed out to pu. Do you\nwant to just drop patch two from that series and call it good?\n"},{"id":"368847","messageId":"xmqqmun6kw3t.fsf@gitster-ct.c.googlers.com","threadId":"50310","inReplyTo":"CABPp-BECeDCKpcM6DExxbPrn41k_VVqVik=z8WWU53eoFNM0jA@mail.gmail.com","subject":"Re: [PATCH v5 1/2] log,diff-tree: add --combined-all-paths option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-08T17:50:46Z","receivedAt":"2019-02-08T17:50:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> On Thu, Feb 7, 2019 at 8:00 PM Junio C Hamano wrote:\n>>\n>> Elijah Newren writes:\n>>\n>> > +For `-c` and `--cc`, only the destination or final path is shown even\n>> > +if the file was renamed on any side of history. With\n>> > +`--combined-all-paths`, the name of the path in each parent is shown\n>> > +followed by the name of the path in the merge commit.\n>> > +\n>> > +Examples for `-c` and `-cc` without `--combined-all-paths`:\n>>\n>> s/-cc/--cc/\n>\n> Thanks for catching that. I was going to go fixup and resubmit, and\n> drop patch 2 while at it, but it looks like you've already squashed\n> this fix in to commit d76ce4f73 that you've pushed out to pu. Do you\n> want to just drop patch two from that series and call it good?\n\nSure.\nThanks.\n"}]}