Re: [PATCH] git-pickaxe: blame rewritten.

[Junio C Hamano <junkio@cox.net>]

Junio C Hamano <junkio@cox.net> writes:

>> (I'd actually also like to have a range-modifier, so that I could do
>>
>> 	git annotate --since=2.weeks.ago v2.6.18.. <path>
>>
>> that didn't go back beyond a certain point,...
>
> I am not sure about revision bottom (v.2.6.18..) offhand, but
> the age limit (--since=2.weeks) should be trivial.
>
> Inside pass_blame() while we iterate over the parents of the
> suspect we are looking at, you can skip the parent if it is
> older than the age limit, or an ancestor of revision bottom,
> like this:
>
> --- l/builtin-pickaxe.c
> +++ k/builtin-pickaxe.c
> @@ -450,6 +450,12 @@ static void pass_blame(struct scoreboard
>  	     parent = parent->next, parent_ix++) {
>  		if (parse_commit(parent->item))
>  			continue;
> +
> +		if (parent is older than age limit)
> +			continue;
> +		if (parent is an ancestor of revision bottom)
> +			continue;
> +
>  		porigin = find_origin(sb, parent->item, origin->path);
>  		if (!porigin)
>  			porigin = find_rename(sb, parent->item, origin);
>

This is not quite right.  The above code tries to avoid passing
blames to the boundary revisions, so "v2.6.18.." makes every old
line to be blamed on one revision after v2.6.18, which might be
technically correct but not what we want.

Instead, we should pass blame, and then prevent boundary
revisions to pass blame further down.  The code in "pu" today
has seen slight restructure of this part and this change should
be easier to implement there, something along the lines of...

--- l/builtin-pickaxe.c
+++ k/builtin-pickaxe.c
@@ -450,8 +450,7 @@ static int pass_blame_to_parent(struct s
 }
 
 #define MAXPARENT 16
-static void pass_blame(struct scoreboard *sb, struct origin *origin,
-		       struct rev_info *revs)
+static void pass_blame(struct scoreboard *sb, struct origin *origin)
 {
 	int i, parent_ix;
 	struct commit *commit = origin->commit;
@@ -469,10 +468,6 @@ static void pass_blame(struct scoreboard
 
 		if (parse_commit(p))
 			continue;
-		if (p->object.flags & UNINTERESTING)
-			continue;
-		if (revs->max_age != -1 && p->date  < revs->max_age)
-			continue;
 
 		porigin = find_origin(sb, parent->item, origin->path);
 		if (!porigin)
@@ -516,6 +511,7 @@ static void assign_blame(struct scoreboa
 	while (1) {
 		int i;
 		struct origin *suspect = NULL;
+		struct commit *commit;
 
 		/* find one suspect to break down */
 		for (i = 0; !suspect && i < sb->num_entries; i++) {
@@ -525,7 +521,10 @@ static void assign_blame(struct scoreboa
 		if (!suspect)
 			return; /* all done */
 
-		pass_blame(sb, suspect, revs);
+		commit = suspect->commit;
+		if (!(commit->object.flags & UNINTERESTING) &&
+		    !(revs->max_age != -1 && commit->date  < revs->max_age))
+			pass_blame(sb, suspect, revs);
 
 		/* Take responsibility for the remaining entries */
 		for (i = 0; i < sb->num_entries; i++)