range-diff: add section header instead of diff header

Currently range-diff keeps the diff header of the inner diff
intact (apart from stripping lines starting with index).  This diff
header is somewhat useful, especially when files get different
names in different ranges.

However there is no real need to keep the whole diff header for that.
The main reason we currently do that is probably because it is easy to
do.

Introduce a new range diff hunk header, that's enclosed by "##",
similar to how line numbers in diff hunks are enclosed by "@@", and
give human readable information of what exactly happened to the file,
including the file name.

This improves the readability of the range-diff by giving more concise
information to the users.  For example if a file was renamed in one
iteration, but not in another, the diff of the headers would be quite
noisy.  However the diff of a single line is concise and should be
easier to understand.

Additionally, this allows us to add these range diff section headers to
the outer diffs hunk headers using a custom userdiff pattern, which
should help making the range-diff more readable.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/range-diff.c b/range-diff.c
index f4a90b33b84bee5681d45f9300c87f5f5841bbe6..5f64380fe4d0597ac271ab2ab38da26ee0841291 100644 (file)
--- a/range-diff.c
+++ b/range-diff.c
@@ -10,6 +10,7 @@
 #include "commit.h"
 #include "pretty.h"
 #include "userdiff.h"
+#include "apply.h"
 
 struct patch_util {
        /* For the search for an exact match */
@@ -101,12 +102,35 @@ static int read_patches(const char *range, struct string_list *list)
                }
 
                if (starts_with(line, "diff --git")) {
+                       struct patch patch = { 0 };
+                       struct strbuf root = STRBUF_INIT;
+                       int linenr = 0;
+
                        in_header = 0;
                        strbuf_addch(&buf, '\n');
                        if (!util->diff_offset)
                                util->diff_offset = buf.len;
-                       strbuf_addch(&buf, ' ');
-                       strbuf_addstr(&buf, line);
+                       line[len - 1] = '\n';
+                       len = parse_git_diff_header(&root, &linenr, 1, line,
+                                                   len, size, &patch);
+                       if (len < 0)
+                               die(_("could not parse git header '%.*s'"), (int)len, line);
+                       strbuf_addstr(&buf, " ## ");
+                       if (patch.is_new > 0)
+                               strbuf_addf(&buf, "%s (new)", patch.new_name);
+                       else if (patch.is_delete > 0)
+                               strbuf_addf(&buf, "%s (deleted)", patch.old_name);
+                       else if (patch.is_rename)
+                               strbuf_addf(&buf, "%s => %s", patch.old_name, patch.new_name);
+                       else
+                               strbuf_addstr(&buf, patch.new_name);
+
+                       if (patch.new_mode && patch.old_mode &&
+                           patch.old_mode != patch.new_mode)
+                               strbuf_addf(&buf, " (mode change %06o => %06o)",
+                                           patch.old_mode, patch.new_mode);
+
+                       strbuf_addstr(&buf, " ##");
                } else if (in_header) {
                        if (starts_with(line, "Author: ")) {
                                strbuf_addstr(&buf, line);
@@ -122,17 +146,13 @@ static int read_patches(const char *range, struct string_list *list)
                } else if (skip_prefix(line, "@@ ", &p)) {
                        p = strstr(p, "@@");
                        strbuf_addstr(&buf, p ? p : "@@");
-               } else if (!line[0] || starts_with(line, "index "))
+               } else if (!line[0])
                        /*
                         * A completely blank (not ' \n', which is context)
                         * line is not valid in a diff.  We skip it
                         * silently, because this neatly handles the blank
                         * separator line between commits in git-log
                         * output.
-                        *
-                        * We also want to ignore the diff's `index` lines
-                        * because they contain exact blob hashes in which
-                        * we are not interested.
                         */
                        continue;
                else if (line[0] == '>') {

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 9f89af71789f515dadbd16c7aa4578dbc4781dcf..c2777560570bb73818d4b237a0a6d3eac377e90b 100755 (executable)
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -181,6 +181,85 @@ test_expect_success 'changed commit with sm config' '
        test_cmp expected actual
 '
 
+test_expect_success 'renamed file' '
+       git range-diff --no-color --submodule=log topic...renamed-file >actual &&
+       sed s/Z/\ /g >expected <<-EOF &&
+       1:  4de457d = 1:  f258d75 s/5/A/
+       2:  fccce22 ! 2:  017b62d s/4/A/
+           @@
+           ZAuthor: Thomas Rast <trast@inf.ethz.ch>
+           Z
+           -    s/4/A/
+           +    s/4/A/ + rename file
+           Z
+           - ## file ##
+           + ## file => renamed-file ##
+           Z@@
+           Z 1
+           Z 2
+       3:  147e64e ! 3:  3ce7af6 s/11/B/
+           @@
+           Z
+           Z    s/11/B/
+           Z
+           - ## file ##
+           + ## renamed-file ##
+           Z@@ A
+           Z 8
+           Z 9
+       4:  a63e992 ! 4:  1e6226b s/12/B/
+           @@
+           Z
+           Z    s/12/B/
+           Z
+           - ## file ##
+           + ## renamed-file ##
+           Z@@ A
+           Z 9
+           Z 10
+       EOF
+       test_cmp expected actual
+'
+
+test_expect_success 'file added and later removed' '
+       git range-diff --no-color --submodule=log topic...added-removed >actual &&
+       sed s/Z/\ /g >expected <<-EOF &&
+       1:  4de457d = 1:  096b1ba s/5/A/
+       2:  fccce22 ! 2:  d92e698 s/4/A/
+           @@
+           ZAuthor: Thomas Rast <trast@inf.ethz.ch>
+           Z
+           -    s/4/A/
+           +    s/4/A/ + new-file
+           Z
+           Z ## file ##
+           Z@@
+           @@
+           Z A
+           Z 6
+           Z 7
+           +
+           + ## new-file (new) ##
+       3:  147e64e ! 3:  9a1db4d s/11/B/
+           @@
+           ZAuthor: Thomas Rast <trast@inf.ethz.ch>
+           Z
+           -    s/11/B/
+           +    s/11/B/ + remove file
+           Z
+           Z ## file ##
+           Z@@ A
+           @@
+           Z 12
+           Z 13
+           Z 14
+           +
+           + ## new-file (deleted) ##
+       4:  a63e992 = 4:  fea3b5c s/12/B/
+       EOF
+       test_cmp expected actual
+'
+
 test_expect_success 'no commits on one side' '
        git commit --amend -m "new message" &&
        git range-diff master HEAD@{1} HEAD
@@ -197,9 +276,9 @@ test_expect_success 'changed message' '
            Z
            +    Also a silly comment here!
            +
-           Z diff --git a/file b/file
-           Z --- a/file
-           Z +++ b/file
+           Z ## file ##
+           Z@@
+           Z 1
        3:  147e64e = 3:  b9cb956 s/11/B/
        4:  a63e992 = 4:  8add5f1 s/12/B/
        EOF
@@ -216,9 +295,9 @@ test_expect_success 'dual-coloring' '
        :     <RESET>
        :    <REVERSE><GREEN>+<RESET><BOLD>    Also a silly comment here!<RESET>
        :    <REVERSE><GREEN>+<RESET>
-       :      diff --git a/file b/file<RESET>
-       :      --- a/file<RESET>
-       :      +++ b/file<RESET>
+       :      ## file ##<RESET>
+       :    <CYAN> @@<RESET>
+       :      1<RESET>
        :<RED>3:  0559556 <RESET><YELLOW>!<RESET><GREEN> 3:  b9cb956<RESET><YELLOW> s/11/B/<RESET>
        :    <REVERSE><CYAN>@@<RESET>
        :      9<RESET>

diff --git a/t/t3206/history.export b/t/t3206/history.export
index b8ffff0940d6f15da7b90400106a162af921c069..7bb38149622737ac65a05b5b8105360640e0aa6e 100644 (file)
--- a/t/t3206/history.export
+++ b/t/t3206/history.export
@@ -22,8 +22,8 @@ data 51
 19
 20
 
-reset refs/heads/removed
-commit refs/heads/removed
+reset refs/heads/renamed-file
+commit refs/heads/renamed-file
 mark :2
 author Thomas Rast <trast@inf.ethz.ch> 1374424921 +0200
 committer Thomas Rast <trast@inf.ethz.ch> 1374484724 +0200
@@ -599,6 +599,82 @@ s/12/B/
 from :46
 M 100644 :28 file
 
-reset refs/heads/removed
-from :47
+commit refs/heads/added-removed
+mark :48
+author Thomas Rast <trast@inf.ethz.ch> 1374485014 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1556574151 +0100
+data 7
+s/5/A/
+from :2
+M 100644 :3 file
+
+blob
+mark :49
+data 0
+
+commit refs/heads/added-removed
+mark :50
+author Thomas Rast <trast@inf.ethz.ch> 1374485024 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1556574177 +0100
+data 18
+s/4/A/ + new-file
+from :48
+M 100644 :5 file
+M 100644 :49 new-file
+
+commit refs/heads/added-removed
+mark :51
+author Thomas Rast <trast@inf.ethz.ch> 1374485036 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1556574177 +0100
+data 22
+s/11/B/ + remove file
+from :50
+M 100644 :7 file
+D new-file
+
+commit refs/heads/added-removed
+mark :52
+author Thomas Rast <trast@inf.ethz.ch> 1374485044 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1556574177 +0100
+data 8
+s/12/B/
+from :51
+M 100644 :9 file
+
+commit refs/heads/renamed-file
+mark :53
+author Thomas Rast <trast@inf.ethz.ch> 1374485014 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1556574309 +0100
+data 7
+s/5/A/
+from :2
+M 100644 :3 file
+
+commit refs/heads/renamed-file
+mark :54
+author Thomas Rast <trast@inf.ethz.ch> 1374485024 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1556574312 +0100
+data 21
+s/4/A/ + rename file
+from :53
+D file
+M 100644 :5 renamed-file
+
+commit refs/heads/renamed-file
+mark :55
+author Thomas Rast <trast@inf.ethz.ch> 1374485036 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1556574319 +0100
+data 8
+s/11/B/
+from :54
+M 100644 :7 renamed-file
+
+commit refs/heads/renamed-file
+mark :56
+author Thomas Rast <trast@inf.ethz.ch> 1374485044 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1556574319 +0100
+data 8
+s/12/B/
+from :55
+M 100644 :9 renamed-file