Re: [PATCH] git-rev-parse.txt: clarify meaning of rev~ and rev~0.

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Mar 14, 2008, 18:49 UTC
Message-ID: <alpine.LFD.1.00.0803141141240.3557@woody.linux-foundation.org>
In-Reply-To: <87wso5mcs7.fsf@osv.gnss.ru>

On Fri, 14 Mar 2008, Sergei Organov wrote:

>
> +                      ...  'rev{tilde}' is equivalent to 'rev{tilde}0'
> +  which in turn is equivalent to 'rev'.

I'd actually prefer to just fix that.

I think it would make more sense to have the same guarantees that rev^ has, namely to always return a commit. I would also suggest that not giving a number would have the same effect of defaulting to 1, not 0.

Right now it's a bit illogical, but at least it's an _undocumented_ illogical behaviour. If we document it, we should fix it and document the logical behaviour instead, no?

Here's a patch to make '^' and '~' act the same for the default count (ie both default to 1), and also have the same behaviour for a count of zero.

Before (no discernible pattern):

	[torvalds@woody git]$ git rev-parse v1.5.1 v1.5.1^0 v1.5.1~0 v1.5.1^ v1.5.1~
	45354a57ee7e3e42c7137db6c94fa968c6babe8d
	89815cab95268e8f0f58142b848ac4cd5e9cbdcb
	45354a57ee7e3e42c7137db6c94fa968c6babe8d
	045f5759c97746589a067461e50fad16f60711ac
	45354a57ee7e3e42c7137db6c94fa968c6babe8d

After (fairly logical):

	[torvalds@woody git]$ ./git rev-parse v1.5.1 v1.5.1^0 v1.5.1~0 v1.5.1^ v1.5.1~
	45354a57ee7e3e42c7137db6c94fa968c6babe8d
	89815cab95268e8f0f58142b848ac4cd5e9cbdcb
	89815cab95268e8f0f58142b848ac4cd5e9cbdcb
	045f5759c97746589a067461e50fad16f60711ac
	045f5759c97746589a067461e50fad16f60711ac

Hmm?

(That parent-finding loop is also now much tighter, not that it matters one whit ;)

		Linus

---
 sha1_name.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 8b6c76f..6afc0e8 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -407,18 +407,22 @@ static int get_nth_ancestor(const char *name, int len,
 			    unsigned char *result, int generation)
 {
 	unsigned char sha1[20];
-	int ret = get_sha1_1(name, len, sha1);
+	struct commit *commit;
+	int ret;
+
+	ret = get_sha1_1(name, len, sha1);
 	if (ret)
 		return ret;
+	commit = lookup_commit_reference(sha1);
+	if (!commit)
+		return -1;
 
 	while (generation--) {
-		struct commit *commit = lookup_commit_reference(sha1);
-
-		if (!commit || parse_commit(commit) || !commit->parents)
+		if (parse_commit(commit) || !commit->parents)
 			return -1;
-		hashcpy(sha1, commit->parents->item->object.sha1);
+		commit = commit->parents->item;
 	}
-	hashcpy(result, sha1);
+	hashcpy(result, commit->object.sha1);
 	return 0;
 }
 
@@ -564,11 +568,10 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1)
 		cp++;
 		while (cp < name + len)
 			num = num * 10 + *cp++ - '0';
-		if (has_suffix == '^') {
-			if (!num && len1 == len - 1)
-				num = 1;
+		if (!num && len1 == len - 1)
+			num = 1;
+		if (has_suffix == '^')
 			return get_parent(name, len1, sha1, num);
-		}
 		/* else if (has_suffix == '~') -- goes without saying */
 		return get_nth_ancestor(name, len1, sha1, num);
 	}