Re: [PATCH 2/2] peel_onion(): teach $foo^{object} peeler
While I was in the middle of suggesting documentation for this new
syntax, I discovered that you already added documentation to your repo
but didn't mention the new version on the mailing list (or maybe I
overlooked it). It would be helpful if you would submit your own
changes to the mailing list to make it harder for the rest of us to
overlook them--and easier to look over them :-)
The documentation looks fine to me.
Off topic: Your patch reminds me of something else that surprised me:
there is no $userstring^{tag}. I suppose it would be a bit ambiguous,
given that tags can point at tags, and it would also be less useful than
the other suffixes. But its absence irked the completionist in me :-)
Michael
On 04/01/2013 12:38 AM, Junio C Hamano wrote:
A string that names an object can be suffixed with ^{type} peeler to
say I have this object name; peel it until you get this type. If
you cannot do so, it is an error. v1.8.2^{commit} asks for a commit
that is pointed at an annotated tag v1.8.2; v1.8.2^{tree} unwraps it
further to the top-level tree object. A special suffix ^{} (i.e. no
type specified) means I do not care what it unwraps to; just peel
annotated tag until you get something that is not a tag.
When you have a random user-supplied string, you can turn it to a
bare 40-hex object name, and cause it to error out if such an object
does not exist, with:
git rev-parse --verify $userstring^{}
for most objects, but this does not yield the tag object name when
$userstring refers to an annotated tag.
Introduce a new suffix, ^{object}, that only makes sure the given
name refers to an existing object. Then
git rev-parse --verify $userstring^{object}
becomes a way to make sure $userstring refers to an existing object.
This is necessary because the plumbing rev-parse --verify is only
about make sure the argument is something we can feed to get_sha1()
and turn it into a raw 20-byte object name SHA-1 and is not about
make sure that 20-byte object name SHA-1 refers to an object that
exists in our object store. When the given $userstring is already
a 40-hex, by definition rev-parse --verify $userstring can turn it
into a raw 20-byte object name. With $userstring^{object}, we can
make sure that the 40-hex string names an object that exists in our
object store before --verify kicks in.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
sha1_name.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sha1_name.c b/sha1_name.c
index 45788df..85b6e75 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -594,7 +594,7 @@ struct object *peel_to_type(const char *name, int namelen,
while (1) {
if (!o || (!o-parsed !parse_object(o-sha1)))
return NULL;
- if (o-type == expected_type)
+ if (expected_type == OBJ_ANY || o-type == expected_type)
return o;
if (o-type == OBJ_TAG)
o = ((struct tag*) o)-tagged;
@@ -645,6 +645,8 @@ static int peel_onion(const char *name, int len, unsigned
char *sha1)
expected_type = OBJ_TREE;
else if (!strncmp(blob_type, sp, 4) sp[4] == '}')
expected_type = OBJ_BLOB;
+ else if (!prefixcmp(sp, object}))
+ expected_type = OBJ_ANY;
else if (sp[0] == '}')
expected_type = OBJ_NONE;
else if (sp[0] == '/')
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] peel_onion(): teach $foo^{object} peeler
Michael Haggerty mhag...@alum.mit.edu writes:
Off topic: Your patch reminds me of something else that surprised me:
there is no $userstring^{tag}. I suppose it would be a bit ambiguous,
given that tags can point at tags, and it would also be less useful than
the other suffixes. But its absence irked the completionist in me :-)
Yes, unfortunately, foo^{type} means start from foo, and until what
you are looking at it is of type, repeatedly peel to see if you can
get to an object of that type, or stop and report an error. If a
tag A points at another tag B, which in turn points at an object C,
you will never see B by applying usual peeling operator.
Also, v1.8.2^{tag} would be give the tag itself, while master^{tag}
would not report the commit master but would error out, which
would be useless. You are better off doing `git cat-file -t foo`
and seeing if it is a tag object at that point.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] peel_onion(): teach $foo^{object} peeler
On 04/02/2013 05:45 PM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
Off topic: Your patch reminds me of something else that surprised me:
there is no $userstring^{tag}. I suppose it would be a bit ambiguous,
given that tags can point at tags, and it would also be less useful than
the other suffixes. But its absence irked the completionist in me :-)
Yes, unfortunately, foo^{type} means start from foo, and until what
you are looking at it is of type, repeatedly peel to see if you can
get to an object of that type, or stop and report an error. If a
tag A points at another tag B, which in turn points at an object C,
you will never see B by applying usual peeling operator.
Also, v1.8.2^{tag} would be give the tag itself, while master^{tag}
would not report the commit master but would error out, which
would be useless. You are better off doing `git cat-file -t foo`
and seeing if it is a tag object at that point.
All correct, of course. But the user would never use master^{tag}
unless he wants a tag and nothing else, so erroring out would be exactly
the thing he wants in that case. This is no different than the
^{commit} part of master^{tree}^{commit}, which correctly errors out
because a commit cannot be inferred from a tree.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] peel_onion(): teach $foo^{object} peeler
Michael Haggerty mhag...@alum.mit.edu writes:
On 04/02/2013 05:45 PM, Junio C Hamano wrote:
Also, v1.8.2^{tag} would be give the tag itself, while master^{tag}
would not report the commit master but would error out, which
would be useless. You are better off doing `git cat-file -t foo`
and seeing if it is a tag object at that point.
All correct, of course. But the user would never use master^{tag}
unless he wants a tag and nothing else, so erroring out would be exactly
the thing he wants in that case. This is no different than the
^{commit} part of master^{tree}^{commit}, which correctly errors out
because a commit cannot be inferred from a tree.
Correct; I was only saying adding it is not something that solves a
problem that cannot be solved with the current system (i.e. no added
value from feature point-of-view). I would not object to a patch to
allow git rev-parse v1.8.2^{tag} for completeness.
We may want to rethink if we can lose the hardcoded lengths like 6,
3, 4, 4 from here, but I didn't bother ;-).
sha1_name.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 85b6e75..47f39a8 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -639,16 +639,18 @@ static int peel_onion(const char *name, int len, unsigned
char *sha1)
return -1;
sp++; /* beginning of type name, or closing brace for empty */
- if (!strncmp(commit_type, sp, 6) sp[6] == '}')
+ if (!strncmp(tag_type, sp, 3) sp[3] == '}')
+ expected_type = OBJ_TAG;
+ else if (!strncmp(commit_type, sp, 6) sp[6] == '}')
expected_type = OBJ_COMMIT;
else if (!strncmp(tree_type, sp, 4) sp[4] == '}')
expected_type = OBJ_TREE;
else if (!strncmp(blob_type, sp, 4) sp[4] == '}')
expected_type = OBJ_BLOB;
else if (!prefixcmp(sp, object}))
- expected_type = OBJ_ANY;
+ expected_type = OBJ_ANY; /* ok as long as it exists */
else if (sp[0] == '}')
- expected_type = OBJ_NONE;
+ expected_type = OBJ_NONE; /* unwrap until we get a non-tag */
else if (sp[0] == '/')
expected_type = OBJ_COMMIT;
else
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
