blame: allow blame --reverse --first-parent when it makes sense

Allow combining --reverse and --first-parent if initial commit of
specified range is at the first-parent chain starting from the final
commit. Disable the prepare_revision_walk()'s builtin children
collection, instead picking only the ones which are along the first
parent chain.

Signed-off-by: Max Kirillov <max@max630.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/blame.c b/builtin/blame.c
index 7b075583cf504bc121a9de327f5600ec4b63ac96..861a5372b9194b71d3f315c72965fb171d4e3392 100644 (file)
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2502,6 +2502,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
        long dashdash_pos, lno;
        char *final_commit_name = NULL;
        enum object_type type;
        long dashdash_pos, lno;
        char *final_commit_name = NULL;
        enum object_type type;

+       struct commit *final_commit = NULL;

 
        static struct string_list range_list;
        static int output_option = 0, opt = 0;
 
        static struct string_list range_list;
        static int output_option = 0, opt = 0;


@@ -2689,11 +2690,11 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
        }
        else if (contents_from)
                die("--contents and --children do not blend well.");
        }
        else if (contents_from)
                die("--contents and --children do not blend well.");

-       else if (revs.first_parent_only)
-               die("combining --first-parent and --reverse is not supported");

        else {
                final_commit_name = prepare_initial(&sb);
                sb.commits.compare = compare_commits_by_reverse_commit_date;
        else {
                final_commit_name = prepare_initial(&sb);
                sb.commits.compare = compare_commits_by_reverse_commit_date;

+               if (revs.first_parent_only)
+                       revs.children.name = NULL;

        }
 
        if (!sb.final) {
        }
 
        if (!sb.final) {


@@ -2710,6 +2711,14 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
        else if (contents_from)
                die("Cannot use --contents with final commit object name");
 
        else if (contents_from)
                die("Cannot use --contents with final commit object name");
 

+       if (reverse && revs.first_parent_only) {
+               struct object_array_entry *entry = find_single_final(sb.revs);
+               if (!entry)
+                       die("--reverse and --first-parent together require specified latest commit");
+               else
+                       final_commit = (struct commit*) entry->item;
+       }
+

        /*
         * If we have bottom, this will mark the ancestors of the
         * bottom commits we would reach while traversing as
        /*
         * If we have bottom, this will mark the ancestors of the
         * bottom commits we would reach while traversing as


@@ -2718,6 +2727,25 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
        if (prepare_revision_walk(&revs))
                die(_("revision walk setup failed"));
 
        if (prepare_revision_walk(&revs))
                die(_("revision walk setup failed"));
 

+       if (reverse && revs.first_parent_only) {
+               struct commit *c = final_commit;
+
+               sb.revs->children.name = "children";
+               while (c->parents &&
+                      hashcmp(c->object.sha1, sb.final->object.sha1)) {
+                       struct commit_list *l = xcalloc(1, sizeof(*l));
+
+                       l->item = c;
+                       if (add_decoration(&sb.revs->children,
+                                          &c->parents->item->object, l))
+                               die("BUG: not unique item in first-parent chain");
+                       c = c->parents->item;
+               }
+
+               if (hashcmp(c->object.sha1, sb.final->object.sha1))
+                       die("--reverse --first-parent together require range along first-parent chain");
+       }
+

        if (is_null_sha1(sb.final->object.sha1)) {
                o = sb.final->util;
                sb.final_buf = xmemdupz(o->file.ptr, o->file.size);
        if (is_null_sha1(sb.final->object.sha1)) {
                o = sb.final->util;
                sb.final_buf = xmemdupz(o->file.ptr, o->file.size);

diff --git a/t/t8009-blame-vs-topicbranches.sh b/t/t8009-blame-vs-topicbranches.sh
index 175ad376bdc42a14e81538b1292c200b3bb0e3b9..72596e38b25f65fc52da2a94cf0f453636e8f66c 100755 (executable)
--- a/t/t8009-blame-vs-topicbranches.sh
+++ b/t/t8009-blame-vs-topicbranches.sh
@@ -26,7 +26,7 @@ test_expect_success setup '
        test_merge A3 C1
        '
 
        test_merge A3 C1
        '
 

-test_expect_failure 'blame --reverse --first-parent finds A1' '
+test_expect_success 'blame --reverse --first-parent finds A1' '

        git blame --porcelain --reverse --first-parent A0..A3 -- file.t >actual_full &&
        head -n 1 <actual_full | sed -e "s/ .*//" >actual &&
        git rev-parse A1 >expect &&
        git blame --porcelain --reverse --first-parent A0..A3 -- file.t >actual_full &&
        head -n 1 <actual_full | sed -e "s/ .*//" >actual &&
        git rev-parse A1 >expect &&