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

Linus Torvalds [off-list ref] writes:

Btw, could we please get rid of this horrible command line syntax.

Pretty much _every_ other git command takes the form

	git cmd [<rev>] [--] <path>

but for some reason annotate and blame (and now pickaxe) do it the wrong 
way around, and do

	git cmd [--] <path> [<rev>]

which is just irritating to somebody who has grown very used to being able 
to specify revisions first.

Side note: I do not intend to keep it named pickaxe -- only
while it is in "pu".

I think we could support all of them to retain backward
compatibility:

	git blame [-options] <path> [<rev>]		(*1*)
	git blame [-options] -- <path> [<rev>]		(*2*)
	git blame [-options] [<rev>] [--] <path>	(*3*)

(*1*) Only for path that does not start with a '-'; we should
      tighten the input to make sure <path> lstat()'s fine
      (which we currently do with pickaxe), <path> cannot be
      interpreted as a valud rev, and when <rev> is given, <rev>
      does not lstat() Ok, to avoid ambiguity.  Other cases we
      should require the newer format with explicit -- (*3*).

(*2*) Backward compatible canoncal format.  The above comment
      for validation does not apply, as (*3*) never has more
      than one path for 'annotate/blame/pickaxe'

(*3*) The canonical format for ohter git commands.  Without
      an explicit --, <rev> should not lstat() Ok, and <path>
      should, to avoid ambiguity.

(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);

I think we can get away by checking if the parent _is_ the
revision bottom (or one of the bottoms, if you say "--not
v2.6.18 v2.6.17.13") instead of doing "is it an ancestor" check,
in practice.  But that is not correct when a merge is involved.