[PATCH] blame: improve diagnosis for "--reverse NEW"

[PATCH] blame: improve diagnosis for "--reverse NEW"

[Junio C Hamano]

"git blame --reverse OLD..NEW -- PATH" tells us to start from the
contents in PATH at OLD and observe how each line is changed while
the history developers up to NEW, and report for each line the
latest commit up to which the line survives in the original form.

If you say "git blame --reverse NEW -- PATH" by mistake, we complain
about the missing OLD, but we phrased it as "No commit to dig down
to?"  In this case, however, we are digging up from OLD, so say so.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-blame.txt | 2 +-
 builtin/blame.c             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index ba54175..16323eb 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
 	    [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
-	    [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>]
+	    [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>..<rev>]
 	    [--] <file>
 
 DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 21f42b0..281f372 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2473,7 +2473,7 @@ static char *prepare_initial(struct scoreboard *sb)
 		final_commit_name = revs->pending.objects[i].name;
 	}
 	if (!final_commit_name)
-		die("No commit to dig down to?");
+		die("No commit to dig up from?");
 	return xstrdup(final_commit_name);
 }
 
-- 
2.9.0-290-g8794d48

[PATCH v2] blame: improve diagnosis for "--reverse NEW"

[Junio C Hamano]

"git blame --reverse OLD..NEW -- PATH" tells us to start from the
contents in PATH at OLD and observe how each line is changed while
the history developers up to NEW, and report for each line the
latest commit up to which the line survives in the original form.

If you say "git blame --reverse NEW -- PATH" by mistake, we complain
about the missing OLD, but we phrased it as "No commit to dig down
to?"  In this case, however, we are digging up from OLD, so say so.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * There was another instance of "dig down to" in the same function.

 Documentation/git-blame.txt | 2 +-
 builtin/blame.c             | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index ba54175..16323eb 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
 	    [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
-	    [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>]
+	    [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>..<rev>]
 	    [--] <file>
 
 DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 21f42b0..a027b8a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2466,14 +2466,14 @@ static char *prepare_initial(struct scoreboard *sb)
 		if (obj->type != OBJ_COMMIT)
 			die("Non commit %s?", revs->pending.objects[i].name);
 		if (sb->final)
-			die("More than one commit to dig down to %s and %s?",
+			die("More than one commit to dig up from, %s and %s?",
 			    revs->pending.objects[i].name,
 			    final_commit_name);
 		sb->final = (struct commit *) obj;
 		final_commit_name = revs->pending.objects[i].name;
 	}
 	if (!final_commit_name)
-		die("No commit to dig down to?");
+		die("No commit to dig up from?");
 	return xstrdup(final_commit_name);
 }
 
-- 
2.9.0-290-g8794d48

[PATCH] blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."

[Junio C Hamano]

Instead of always requiring both ends of a range, we could DWIM
"OLD", which could be a misspelt "OLD..", to be a range that ends at
the current commit.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I am not convinced that this is a good change, though.  It is
   true that there is no other sensible interpretation of the user's
   intent when --reverse is given a single positive commit without
   any other revision, but at the same time, this feels a bit too
   much special casing that could hurt casual users when they are
   still forming their mental world model by learning from examples.

 Documentation/blame-options.txt |  5 +++--
 builtin/blame.c                 | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 02cb684..6c6c78f 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -28,12 +28,13 @@ include::line-range-format.txt[]
 -S <revs-file>::
 	Use revisions from revs-file instead of calling linkgit:git-rev-list[1].
 
---reverse::
+--reverse <rev>..<rev>::
 	Walk history forward instead of backward. Instead of showing
 	the revision in which a line appeared, this shows the last
 	revision in which a line has existed. This requires a range of
 	revision like START..END where the path to blame exists in
-	START.
+	START.  `git blame --reverse START` is taken as `git blame
+	--reverse START..HEAD` for convenience.
 
 -p::
 --porcelain::
diff --git a/builtin/blame.c b/builtin/blame.c
index a027b8a..574b47d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2447,6 +2447,41 @@ static char *prepare_final(struct scoreboard *sb)
 	return xstrdup_or_null(name);
 }
 
+static const char *dwim_reverse_initial(struct scoreboard *sb)
+{
+	/*
+	 * DWIM "git blame --reverse ONE -- PATH" as
+	 * "git blame --reverse ONE..HEAD -- PATH" but only do so
+	 * when it makes sense.
+	 */
+	struct object *obj;
+	struct commit *head_commit;
+	unsigned char head_sha1[20];
+
+	if (sb->revs->pending.nr != 1)
+		return NULL;
+
+	/* Is that sole rev a committish? */
+	obj = sb->revs->pending.objects[0].item;
+	obj = deref_tag(obj, NULL, 0);
+	if (obj->type != OBJ_COMMIT)
+		return NULL;
+
+	/* Do we have HEAD? */
+	if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
+		return NULL;
+	head_commit = lookup_commit_reference_gently(head_sha1, 1);
+	if (!head_commit)
+		return NULL;
+
+	/* Turn "ONE" into "ONE..HEAD" then */
+	obj->flags |= UNINTERESTING;
+	add_pending_object(sb->revs, &head_commit->object, "HEAD");
+
+	sb->final = (struct commit *)obj;
+	return sb->revs->pending.objects[0].name;
+}
+
 static char *prepare_initial(struct scoreboard *sb)
 {
 	int i;
@@ -2472,6 +2507,9 @@ static char *prepare_initial(struct scoreboard *sb)
 		sb->final = (struct commit *) obj;
 		final_commit_name = revs->pending.objects[i].name;
 	}
+
+	if (!final_commit_name)
+		final_commit_name = dwim_reverse_initial(sb);
 	if (!final_commit_name)
 		die("No commit to dig up from?");
 	return xstrdup(final_commit_name);