log,diff-tree: add --combined-all-paths option
authorElijah Newren <newren@gmail.com>
Fri, 8 Feb 2019 01:12:46 +0000 (17:12 -0800)
committerJunio C Hamano <gitster@pobox.com>
Fri, 8 Feb 2019 04:15:25 +0000 (20:15 -0800)
The combined diff format for merges will only list one filename, even if
rename or copy detection is active. For example, with raw format one
might see:

::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c
::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM bar.sh
::100644 100644 100644 e07d6c5 9042e82 ee91881 RR phooey.c

This doesn't let us know what the original name of bar.sh was in the
first parent, and doesn't let us know what either of the original names
of phooey.c were in either of the parents. In contrast, for non-merge
commits, raw format does provide original filenames (and a rename score
to boot). In order to also provide original filenames for merge
commits, add a --combined-all-paths option (which must be used with
either -c or --cc, and is likely only useful with rename or copy
detection active) so that we can print tab-separated filenames when
renames are involved. This transforms the above output to:

::100644 100644 100644 fabadb8 cc95eb0 4866510 MM desc.c desc.c desc.c
::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM foo.sh bar.sh bar.sh
::100644 100644 100644 e07d6c5 9042e82 ee91881 RR fooey.c fuey.c phooey.c

Further, in patch format, this changes the from/to headers so that
instead of just having one "from" header, we get one for each parent.
For example, instead of having

--- a/phooey.c
+++ b/phooey.c

we would see

--- a/fooey.c
--- a/fuey.c
+++ b/phooey.c

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation/diff-format.txt
Documentation/diff-generate-patch.txt
Documentation/git-diff-tree.txt
Documentation/rev-list-options.txt
builtin/diff-tree.c
combine-diff.c
diff.h
revision.c
revision.h
t/t4038-diff-combined.sh
index cdcc17f0ad574b435bac76c0876a3b344e2839e0..4d846d73463c520046e8fa22d5da375733fa3fe9 100644 (file)
@@ -95,12 +95,26 @@ from the format described above in the following way:
 . there are more "src" modes and "src" sha1
 . status is concatenated status characters for each parent
 . no optional "score" number
-. single path, only for "dst"
+. tab-separated pathname(s) of the file
 
-Example:
+For `-c` and `--cc`, only the destination or final path is shown even
+if the file was renamed on any side of history.  With
+`--combined-all-paths`, the name of the path in each parent is shown
+followed by the name of the path in the merge commit.
+
+Examples for `-c` and `--cc` without `--combined-all-paths`:
+------------------------------------------------
+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM      desc.c
+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM      bar.sh
+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR      phooey.c
+------------------------------------------------
+
+Examples when `--combined-all-paths` added to either `-c` or `--cc`:
 
 ------------------------------------------------
-::100644 100644 100644 fabadb8 cc95eb0 4866510 MM      describe.c
+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM      desc.c  desc.c  desc.c
+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM      foo.sh  bar.sh  bar.sh
+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR      fooey.c fuey.c  phooey.c
 ------------------------------------------------
 
 Note that 'combined diff' lists only files which were modified from
index 231105cff48d9c109b2001623c67d51690c595ba..f10ca410ad8a937e1acc62864c6b33b1db965ac4 100644 (file)
@@ -143,6 +143,19 @@ copying detection) are designed to work with diff of two
 Similar to two-line header for traditional 'unified' diff
 format, `/dev/null` is used to signal created or deleted
 files.
++
+However, if the --combined-all-paths option is provided, instead of a
+two-line from-file/to-file you get a N+1 line from-file/to-file header,
+where N is the number of parents in the merge commit
+
+       --- a/file
+       --- a/file
+       --- a/file
+       +++ b/file
++
+This extended format can be useful if rename or copy detection is
+active, to allow you to see the original name of the file in different
+parents.
 
 4.   Chunk header format is modified to prevent people from
      accidentally feeding it to `patch -p1`. Combined diff format
index 2319b2b19209467bf5796abf8df425c6118dadac..28b93ecd54fd638ffb7b8ccb39f89b38d4e6f362 100644 (file)
@@ -10,8 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git diff-tree' [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty]
-             [-t] [-r] [-c | --cc] [--root] [<common diff options>]
-             <tree-ish> [<tree-ish>] [<path>...]
+             [-t] [-r] [-c | --cc] [--combined-all-paths] [--root]
+             [<common diff options>] <tree-ish> [<tree-ish>] [<path>...]
 
 DESCRIPTION
 -----------
@@ -108,6 +108,13 @@ include::pretty-options.txt[]
        itself and the commit log message is not shown, just like in any other
        "empty diff" case.
 
+--combined-all-paths::
+       This flag causes combined diffs (used for merge commits) to
+       list the name of the file from all parents.  It thus only has
+       effect when -c or --cc are specified, and is likely only
+       useful if filename changes are detected (i.e. when either
+       rename or copy detection have been requested).
+
 --always::
        Show the commit itself and the commit log message even
        if the diff itself is empty.
index 98b538bc779635fcf59afd1a8cb366e9e57ed7ae..642ce943c1796211568e18f09c105785909b192b 100644 (file)
@@ -948,6 +948,13 @@ options may be given. See linkgit:git-diff-files[1] for more options.
        the parents have only two variants and the merge result picks
        one of them without modification.
 
+--combined-all-paths::
+       This flag causes combined diffs (used for merge commits) to
+       list the name of the file from all parents.  It thus only has
+       effect when -c or --cc are specified, and is likely only
+       useful if filename changes are detected (i.e. when either
+       rename or copy detection have been requested).
+
 -m::
        This flag makes the merge commits show the full diff like
        regular commits; for each merge parent, a separate log entry
index ef996126d7b521ea1de2c837e4ed97fdcb6f1e71..ebe48f60c159bf61ae1dcf66448db861f0060519 100644 (file)
@@ -82,9 +82,13 @@ static int diff_tree_stdin(char *line)
 }
 
 static const char diff_tree_usage[] =
-"git diff-tree [--stdin] [-m] [-c] [--cc] [-s] [-v] [--pretty] [-t] [-r] [--root] "
+"git diff-tree [--stdin] [-m] [-c | --cc] [-s] [-v] [--pretty] [-t] [-r] [--root] "
 "[<common-diff-options>] <tree-ish> [<tree-ish>] [<path>...]\n"
 "  -r            diff recursively\n"
+"  -c            show combined diff for merge commits\n"
+"  --cc          show combined diff for merge commits removing uninteresting hunks\n"
+"  --combined-all-paths\n"
+"                show name of file in all parents for combined diffs\n"
 "  --root        include the initial commit as diff against /dev/null\n"
 COMMON_DIFF_OPTIONS_HELP;
 
index a143c006341170fad8c42604eca8f9bbf3226de8..54cb892ae5c3ef559d87e61497ae20879a7aa0c0 100644 (file)
@@ -23,11 +23,20 @@ static int compare_paths(const struct combine_diff_path *one,
                                 two->path, strlen(two->path), two->mode);
 }
 
-static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
+static int filename_changed(char status)
+{
+       return status == 'R' || status == 'C';
+}
+
+static struct combine_diff_path *intersect_paths(
+       struct combine_diff_path *curr,
+       int n,
+       int num_parent,
+       int combined_all_paths)
 {
        struct diff_queue_struct *q = &diff_queued_diff;
        struct combine_diff_path *p, **tail = &curr;
-       int i, cmp;
+       int i, j, cmp;
 
        if (!n) {
                for (i = 0; i < q->nr; i++) {
@@ -50,6 +59,13 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
                        oidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);
                        p->parent[n].mode = q->queue[i]->one->mode;
                        p->parent[n].status = q->queue[i]->status;
+
+                       if (combined_all_paths &&
+                           filename_changed(p->parent[n].status)) {
+                               strbuf_init(&p->parent[n].path, 0);
+                               strbuf_addstr(&p->parent[n].path,
+                                             q->queue[i]->one->path);
+                       }
                        *tail = p;
                        tail = &p->next;
                }
@@ -68,6 +84,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
                if (cmp < 0) {
                        /* p->path not in q->queue[]; drop it */
                        *tail = p->next;
+                       for (j = 0; j < num_parent; j++)
+                               if (combined_all_paths &&
+                                   filename_changed(p->parent[j].status))
+                                       strbuf_release(&p->parent[j].path);
                        free(p);
                        continue;
                }
@@ -81,6 +101,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
                oidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);
                p->parent[n].mode = q->queue[i]->one->mode;
                p->parent[n].status = q->queue[i]->status;
+               if (combined_all_paths &&
+                   filename_changed(p->parent[n].status))
+                       strbuf_addstr(&p->parent[n].path,
+                                     q->queue[i]->one->path);
 
                tail = &p->next;
                i++;
@@ -960,12 +984,25 @@ static void show_combined_header(struct combine_diff_path *elem,
        if (!show_file_header)
                return;
 
-       if (added)
-               dump_quoted_path("--- ", "", "/dev/null",
-                                line_prefix, c_meta, c_reset);
-       else
-               dump_quoted_path("--- ", a_prefix, elem->path,
-                                line_prefix, c_meta, c_reset);
+       if (rev->combined_all_paths) {
+               for (i = 0; i < num_parent; i++) {
+                       char *path = filename_changed(elem->parent[i].status)
+                               ? elem->parent[i].path.buf : elem->path;
+                       if (elem->parent[i].status == DIFF_STATUS_ADDED)
+                               dump_quoted_path("--- ", "", "/dev/null",
+                                                line_prefix, c_meta, c_reset);
+                       else
+                               dump_quoted_path("--- ", a_prefix, path,
+                                                line_prefix, c_meta, c_reset);
+               }
+       } else {
+               if (added)
+                       dump_quoted_path("--- ", "", "/dev/null",
+                                        line_prefix, c_meta, c_reset);
+               else
+                       dump_quoted_path("--- ", a_prefix, elem->path,
+                                        line_prefix, c_meta, c_reset);
+       }
        if (deleted)
                dump_quoted_path("+++ ", "", "/dev/null",
                                 line_prefix, c_meta, c_reset);
@@ -1227,6 +1264,15 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
                putchar(inter_name_termination);
        }
 
+       for (i = 0; i < num_parent; i++)
+               if (rev->combined_all_paths) {
+                       if (filename_changed(p->parent[i].status))
+                               write_name_quoted(p->parent[i].path.buf, stdout,
+                                                 inter_name_termination);
+                       else
+                               write_name_quoted(p->path, stdout,
+                                                 inter_name_termination);
+               }
        write_name_quoted(p->path, stdout, line_termination);
 }
 
@@ -1324,7 +1370,9 @@ static const char *path_path(void *obj)
 
 /* find set of paths that every parent touches */
 static struct combine_diff_path *find_paths_generic(const struct object_id *oid,
-       const struct oid_array *parents, struct diff_options *opt)
+       const struct oid_array *parents,
+       struct diff_options *opt,
+       int combined_all_paths)
 {
        struct combine_diff_path *paths = NULL;
        int i, num_parent = parents->nr;
@@ -1350,7 +1398,8 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,
                        opt->output_format = DIFF_FORMAT_NO_OUTPUT;
                diff_tree_oid(&parents->oid[i], oid, "", opt);
                diffcore_std(opt);
-               paths = intersect_paths(paths, i, num_parent);
+               paths = intersect_paths(paths, i, num_parent,
+                                       combined_all_paths);
 
                /* if showing diff, show it in requested order */
                if (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&
@@ -1460,7 +1509,8 @@ void diff_tree_combined(const struct object_id *oid,
                 * diff(sha1,parent_i) for all i to do the job, specifically
                 * for parent0.
                 */
-               paths = find_paths_generic(oid, parents, &diffopts);
+               paths = find_paths_generic(oid, parents, &diffopts,
+                                          rev->combined_all_paths);
        }
        else {
                int stat_opt;
@@ -1535,6 +1585,10 @@ void diff_tree_combined(const struct object_id *oid,
        while (paths) {
                struct combine_diff_path *tmp = paths;
                paths = paths->next;
+               for (i = 0; i < num_parent; i++)
+                       if (rev->combined_all_paths &&
+                           filename_changed(tmp->parent[i].status))
+                               strbuf_release(&tmp->parent[i].path);
                free(tmp);
        }
 
diff --git a/diff.h b/diff.h
index b512d0477ac3a4a0338094a4d3b21770ecb57dd8..90ea0256a584bb3c4feccb4ee907b5b7ca2a6636 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -294,6 +294,7 @@ struct combine_diff_path {
                char status;
                unsigned int mode;
                struct object_id oid;
+               struct strbuf path;
        } parent[FLEX_ARRAY];
 };
 #define combine_diff_path_size(n, l) \
index 119947ced0ba74cd20a007daf43f61386a393a6a..384fa69aed1d1b8d69f3906d1b9d6404e653b1d5 100644 (file)
@@ -1995,6 +1995,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
                revs->diff = 1;
                revs->dense_combined_merges = 0;
                revs->combine_merges = 1;
+       } else if (!strcmp(arg, "--combined-all-paths")) {
+               revs->diff = 1;
+               revs->combined_all_paths = 1;
        } else if (!strcmp(arg, "--cc")) {
                revs->diff = 1;
                revs->dense_combined_merges = 1;
@@ -2491,6 +2494,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
        }
        if (revs->combine_merges)
                revs->ignore_merges = 0;
+       if (revs->combined_all_paths && !revs->combine_merges)
+               die("--combined-all-paths makes no sense without -c or --cc");
+
        revs->diffopt.abbrev = revs->abbrev;
 
        if (revs->line_level_traverse) {
index 52e5a88ff5725862dced5c72fbc2aa6435b94e3b..253c12c29a5a1dba63ab0240636eb453ee212476 100644 (file)
@@ -171,6 +171,7 @@ struct rev_info {
                        verbose_header:1,
                        ignore_merges:1,
                        combine_merges:1,
+                       combined_all_paths:1,
                        dense_combined_merges:1,
                        always_show_header:1;
 
index e2824d343783438f9a3ae78884df7c8b8c63670d..07b49f6d6d9ef93476af2af10fe694d2f971b59d 100755 (executable)
@@ -435,4 +435,92 @@ test_expect_success 'combine diff gets tree sorting right' '
        test_cmp expect actual
 '
 
+test_expect_success 'setup for --combined-all-paths' '
+       git branch side1c &&
+       git branch side2c &&
+       git checkout side1c &&
+       test_seq 1 10 >filename-side1c &&
+       git add filename-side1c &&
+       git commit -m with &&
+       git checkout side2c &&
+       test_seq 1 9 >filename-side2c &&
+       echo ten >>filename-side2c &&
+       git add filename-side2c &&
+       git commit -m iam &&
+       git checkout -b mergery side1c &&
+       git merge --no-commit side2c &&
+       git rm filename-side1c &&
+       echo eleven >>filename-side2c &&
+       git mv filename-side2c filename-merged &&
+       git add filename-merged &&
+       git commit
+'
+
+test_expect_success '--combined-all-paths and --raw' '
+       cat <<-\EOF >expect &&
+       ::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR    filename-side1c filename-side2c filename-merged
+       EOF
+       git diff-tree -c -M --raw --combined-all-paths HEAD >actual.tmp &&
+       sed 1d <actual.tmp >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--combined-all-paths and --cc' '
+       cat <<-\EOF >expect &&
+       --- a/filename-side1c
+       --- a/filename-side2c
+       +++ b/filename-merged
+       EOF
+       git diff-tree --cc -M --combined-all-paths HEAD >actual.tmp &&
+       grep ^[-+][-+][-+] <actual.tmp >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success FUNNYNAMES 'setup for --combined-all-paths with funny names' '
+       git branch side1d &&
+       git branch side2d &&
+       git checkout side1d &&
+       test_seq 1 10 >$(printf "file\twith\ttabs") &&
+       git add file* &&
+       git commit -m with &&
+       git checkout side2d &&
+       test_seq 1 9 >$(printf "i\tam\ttabbed") &&
+       echo ten >>$(printf "i\tam\ttabbed") &&
+       git add *tabbed &&
+       git commit -m iam &&
+       git checkout -b funny-names-mergery side1d &&
+       git merge --no-commit side2d &&
+       git rm *tabs &&
+       echo eleven >>$(printf "i\tam\ttabbed") &&
+       git mv "$(printf "i\tam\ttabbed")" "$(printf "fickle\tnaming")" &&
+       git add fickle* &&
+       git commit
+'
+
+test_expect_success FUNNYNAMES '--combined-all-paths and --raw and funny names' '
+       cat <<-\EOF >expect &&
+       ::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR    "file\twith\ttabs"      "i\tam\ttabbed" "fickle\tnaming"
+       EOF
+       git diff-tree -c -M --raw --combined-all-paths HEAD >actual.tmp &&
+       sed 1d <actual.tmp >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success FUNNYNAMES '--combined-all-paths and --raw -and -z and funny names' '
+       printf "aaf8087c3cbd4db8e185a2d074cf27c53cfb75d7\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\0file\twith\ttabs\0i\tam\ttabbed\0fickle\tnaming\0" >expect &&
+       git diff-tree -c -M --raw --combined-all-paths -z HEAD >actual &&
+       test_cmp -a expect actual
+'
+
+test_expect_success FUNNYNAMES '--combined-all-paths and --cc and funny names' '
+       cat <<-\EOF >expect &&
+       --- "a/file\twith\ttabs"
+       --- "a/i\tam\ttabbed"
+       +++ "b/fickle\tnaming"
+       EOF
+       git diff-tree --cc -M --combined-all-paths HEAD >actual.tmp &&
+       grep ^[-+][-+][-+] <actual.tmp >actual &&
+       test_cmp expect actual
+'
+
 test_done