Re: [PATCH 2/2] blame: allow blame --reverse --first-parent when it makes sense
On Wed, Oct 21, 2015 at 12:08 AM, Max Kirillov <max@max630.net> wrote:
> Do not die immediately when the two flags are specified. Instead
> check that the specified range is along first-parent chain. Explioit
Show 63 quoted lines
> how prepare_revision_walk() handles first_parent_only flag: the commits
> outside of first-parent chain are either unknown (and do not have any
> children recorded) or appear as non-first parent of a commit along the
> first-parent chain.
>
> Since the check seems fragile, add test which makes sure that blame dies
> in both cases.
>
> Signed-off-by: Max Kirillov <max@max630.net>
> ---
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 295ce92..27de544 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2692,8 +2692,6 @@ parse_done:
> }
> 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;
> @@ -2721,6 +2719,15 @@ parse_done:
> if (prepare_revision_walk(&revs))
> die(_("revision walk setup failed"));
>
> + if (reverse && revs.first_parent_only) {
> + struct commit_list *final_children = lookup_decoration(&revs.children,
> + &sb.final->object);
> + if (!final_children ||
> + hashcmp(final_children->item->parents->item->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);
> diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh
> index 9f40613..042863b 100755
> --- a/t/t8009-blame-reverse.sh
> +++ b/t/t8009-blame-reverse.sh
> @@ -24,11 +24,16 @@ test_expect_failure 'blame --reverse finds B1, not C1' '
> test_cmp expect actual
> '
>
> -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 -1 <actual_full | sed -e "sX .*XX" >actual &&
> git rev-parse A1 >expect &&
> test_cmp expect actual
> '
>
> +test_expect_success 'blame --reverse --first-parse dies if no first parent chain' '
> + test_must_fail git blame --porcelain --reverse --first-parent B1..A3 -- file.t &&
> + test_must_fail git blame --porcelain --reverse --first-parent B2..A3 -- file.t
> + '
> +
> test_done
> --
> 2.3.4.2801.g3d0809bEric Sunshine <sunshine@sunshineco.com>