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

[Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>]

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