Jeff King <peff@xxxxxxxx> writes:
> I'm not too familiar with the code, but this _seems_ to work for me:
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 21321be..2e03d47 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1375,6 +1375,10 @@ static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit
> static int num_scapegoats(struct rev_info *revs, struct commit *commit)
> {
> struct commit_list *l = first_scapegoat(revs, commit);
> + if (!l)
> + return 0;
> + if (revs->first_parent_only)
> + return 1;
> return commit_list_count(l);
> }
>
> I suspect it doesn't work at all with `--reverse`. I also have the
> nagging feeling that this could be handled inside revision.c with
> parent-rewriting, but I don't really know.
I am not sure how well --children, which is what we use from
revision.c, works with --first-parent flag passed to the revision
walking machinery. If it already does the right thing, then the
revs.children decoration that collects all children of any given
commit should automatically give its "sole child" (in the world
limited to the first-parent chain) from first_scapegoat().
And if it does not, perhaps that is what we would want to fix,
i.e. making sure rev-list --first-parent --children does what
people would expect.
> But "git blame --first-parent <file>" seems to behave sanely in git.git
> with this.
It is intresting to see that the above is the only thing necessary,
as a naive way to try all parents would be to do:
for (sg = first_scapegoat(...); sg; sg = sg->next)
assign blame to sg;
take the remainder ourselves;
in which case, a better place to patch would be first_scapegoat(),
not this function, so that we will always see zero or one element
parent list returned from the function.
But in reality, the code instead does this:
num_sg = num_scapegoats(...);
for (i = 0, sg = first_scapegoat(...);
i < num_sg && sg;
i++, sg = sg->next)
assign blame to sg;
take the remainder ourselves;
so you do not have to touch first_scapegoat() at all.
I do not offhand know if this was a sign of foresight in the
original, or just an overly redundant programming ;-).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html