for-each-ref: %(object) and %(type) unimplemented
Try the following on a repository with tag objects in it:
git for-each-ref --format='%(object)'
git for-each-ref --format='%(type)'
Each command prints only newlines. The %(type) and %(object) options are not rejected with an error message (like "%(doesnotexist)"), but they don't seem to be implemented. "%(tag)" works though.
Anyone care to implement the missing options? Or should they rather be removed from the documentation?
-- Lea
Quoted 11 lines · Lea Wiemann
Try the following on a repository with tag objects in it:
git for-each-ref --format='%(object)'
git for-each-ref --format='%(type)'Each command prints only newlines. The %(type) and %(object) options are not rejected with an error message (like "%(doesnotexist)"), but they don't seem to be implemented. "%(tag)" works though.
Anyone care to implement the missing options? Or should they rather be removed from the documentation?
Looks like they were part of the original set of atoms, but they just never ended up implemented. Clearly nobody has actually cared in the intervening time, but it is easy enough to add them. See below.
Since you seem to be testing for-each-ref, maybe it would make sense to put together a test script that exercises each of the atoms?
-- >8 --
for-each-ref: implement missing tag values
The "type" and "object" fields for tags were accepted as valid atoms, but never implemented. Consequently, they simply returned the empty string, even for valid tags.
Noticed by Lea Wiemann.
--- builtin-for-each-ref.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
builtin-for-each-ref.c +7 -0
@@ -234,6 +234,13 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob name++;if (!strcmp (name,"tag" )) v->s = tag->tag; +else if (!strcmp (name,"type" ) && tag->tagged) + v->s =typename (tag->tagged->type); +else if (!strcmp (name,"object" ) && tag->tagged) { +char *s =xmalloc (41 ); +strcpy (s,sha1_to_hex (tag->tagged->sha1)); + v->s = s; + } } }
Signature · 1 line
1.5.6.129.g8dea5.dirty
Quoted 2 lines · Jeff King
Since you seem to be testing for-each-ref, maybe it would make sense to put together a test script that exercises each of the atoms?
Hmm. Actually, there is a test in t6300 that runs with each atom name. Unfortunately, it doesn't bother actually checking the output for sanity, so the fact that these atoms returned the empty string was missed.
-Peff
2008/6/25 Jeff King <[email protected]>:
Quoted 9 lines · Jeff King
Quoted 3 lines
Since you seem to be testing for-each-ref, maybe it would make sense to put together a test script that exercises each of the atoms?
Hmm. Actually, there is a test in t6300 that runs with each atom name. Unfortunately, it doesn't bother actually checking the output for sanity, so the fact that these atoms returned the empty string was missed.
The documentation also says "tree" and "parent" should work (which they also don't).
Signature · 1 line
Mikael Magnusson
Quoted 6 lines · Mikael Magnusson
2008/6/25 Jeff King <[email protected]>:
Quoted 5 lines · Jeff King
Quoted 3 lines
Since you seem to be testing for-each-ref, maybe it would make sense to put together a test script that exercises each of the atoms?
Quoted 7 lines
Quoted 4 lines
Hmm. Actually, there is a test in t6300 that runs with each atom name. Unfortunately, it doesn't bother actually checking the output for sanity, so the fact that these atoms returned the empty string was missed.
The documentation also says "tree" and "parent" should work (which they also don't).
The doc does not say any such thing. A tag object does not have 'parent' nor 'tree' header fields.
Neither a commit have 'object' nor 'type' header fields and you cannot ask for them when showing a commit object.
2008/6/25 Junio C Hamano <[email protected]>:
Quoted 13 lines
Quoted 11 lines · Mikael Magnusson
2008/6/25 Jeff King <[email protected]>:
Quoted 9 lines · Jeff King
Quoted 3 lines
Since you seem to be testing for-each-ref, maybe it would make sense to put together a test script that exercises each of the atoms?
Hmm. Actually, there is a test in t6300 that runs with each atom name. Unfortunately, it doesn't bother actually checking the output for sanity, so the fact that these atoms returned the empty string was missed.
Quoted 8 lines
Quoted 2 lines
The documentation also says "tree" and "parent" should work (which they also don't).
The doc does not say any such thing. A tag object does not have 'parent' nor 'tree' header fields.
Neither a commit have 'object' nor 'type' header fields and you cannot ask for them when showing a commit object.
Ah, sorry, I read this paragraph:
In addition to the above, for commit and tag objects,
the header field
names (tree, parent, object, type, and tag) can be used
to specify the
value in the header field.
But not this one (which perhaps should be right after the
above instead of 3 paragraphs down):
In any case, a field name that refers to a field
inapplicable to the
object referred by the ref does not cause an error. It
returns an empty
string instead.
Signature · 1 line
Mikael Magnusson
Quoted 4 lines · Jeff King
Hmm. Actually, there is a test in t6300 that runs with each atom name. Unfortunately, it doesn't bother actually checking the output for sanity, so the fact that these atoms returned the empty string was missed.
And here is a two patch series cleaning up the test script and fixing the bug.
-Peff
PATCH 1/2 improve for-each-ref test script
Previously, we did a sanity check by doing for-each-ref using each possible format atom. However, we never checked the actual output produced by that atom. This patch checks the output, and in doing so reveals breakage on a few atoms.
While we're at it, also clean up a few '!' into test_must_fail.
--- t/t6300-for-each-ref.sh | 87 +++++++++++++++++++++++++++++++++++++--------- 1 files changed, 70 insertions(+), 17 deletions(-)
t/t6300-for-each-ref.sh +70 -17
@@ -26,25 +26,78 @@ test_expect_success 'Create sample commit with known timestamp' ' git tag -a -m "Tagging at $datestamp" testtag ' -test_expect_success 'Check atom names are valid' ' - bad= - for token in \ - refname objecttype objectsize objectname tree parent \ - numparent object type author authorname authoremail \ - authordate committer committername committeremail \ - committerdate tag tagger taggername taggeremail \ - taggerdate creator creatordate subject body contents - do - git for-each-ref --format="$token=%($token)" refs/heads || { - bad=$token - break - } - done - test -z "$bad" +test_atom() { + case "$1" in + head) ref=refs/heads/master ;; + tag) ref=refs/tags/testtag ;; + esac + printf '%s\n' "$3" >expected + test_expect_${4:-success} "basic atom: $1 $2" " + git for-each-ref --format='%($2)' $ref >actual && + test_cmp expected actual + " +} + +test_atom head refname refs/heads/master +test_atom head objecttype commit +test_atom head objectsize 171 +test_atom head objectname 67a36f10722846e891fbada1ba48ed035de75581 +test_atom head tree 0e51c00fcb93dffc755546f27593d511e1bdb46f +test_atom head parent '' +test_atom head numparent 0 +test_atom head object '' +test_atom head type '' +test_atom head author 'A U Thor <[email protected]> 1151939924 +0200' +test_atom head authorname 'A U Thor' +test_atom head authoremail '<[email protected]>' +test_atom head authordate 'Mon Jul 3 17:18:44 2006 +0200' +test_atom head committer 'C O Mitter <[email protected]> 1151939923 +0200' +test_atom head committername 'C O Mitter' +test_atom head committeremail '<[email protected]>' +test_atom head committerdate 'Mon Jul 3 17:18:43 2006 +0200' +test_atom head tag '' +test_atom head tagger '' +test_atom head taggername '' +test_atom head taggeremail '' +test_atom head taggerdate '' +test_atom head creator 'C O Mitter <[email protected]> 1151939923 +0200' +test_atom head creatordate 'Mon Jul 3 17:18:43 2006 +0200' +test_atom head subject 'Initial' +test_atom head body '' +test_atom head contents 'Initial +' + +test_atom tag refname refs/tags/testtag +test_atom tag objecttype tag +test_atom tag objectsize 154 +test_atom tag objectname 98b46b1d36e5b07909de1b3886224e3e81e87322 +test_atom tag tree '' +test_atom tag parent '' +test_atom tag numparent '' +test_atom tag object '67a36f10722846e891fbada1ba48ed035de75581' failure +test_atom tag type 'commit' failure +test_atom tag author '' +test_atom tag authorname '' +test_atom tag authoremail '' +test_atom tag authordate '' +test_atom tag committer '' +test_atom tag committername '' +test_atom tag committeremail '' +test_atom tag committerdate '' +test_atom tag tag 'testtag' +test_atom tag tagger 'C O Mitter <[email protected]> 1151939925 +0200' +test_atom tag taggername 'C O Mitter' +test_atom tag taggeremail '<[email protected]>' +test_atom tag taggerdate 'Mon Jul 3 17:18:45 2006 +0200' +test_atom tag creator 'C O Mitter <[email protected]> 1151939925 +0200' +test_atom tag creatordate 'Mon Jul 3 17:18:45 2006 +0200' +test_atom tag subject 'Tagging at 1151939927' +test_atom tag body '' +test_atom tag contents 'Tagging at 1151939927 ' test_expect_success 'Check invalid atoms names are errors' ' - ! git-for-each-ref --format="%(INVALID)" refs/heads + test_must_fail git-for-each-ref --format="%(INVALID)" refs/heads ' test_expect_success 'Check format specifiers are ignored in naming date atoms' '
@@ -64,7 +117,7 @@ test_expect_success 'Check valid format specifiers for date fields' ' ' test_expect_success 'Check invalid format specifiers are errors' ' - ! git-for-each-ref --format="%(authordate:INVALID)" refs/heads + test_must_fail git-for-each-ref --format="%(authordate:INVALID)" refs/heads ' cat >expected <<\EOF
Signature · 1 line
1.5.6
PATCH 2/2 for-each-ref: implement missing tag values
In reply to Jeff KingThe "type" and "object" fields for tags were accepted as valid atoms, but never implemented. Consequently, they simply returned the empty string, even for valid tags.
--- builtin-for-each-ref.c | 7 +++++++ t/t6300-for-each-ref.sh | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-)
builtin-for-each-ref.c +7 -0
@@ -234,6 +234,13 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob name++;if (!strcmp (name,"tag" )) v->s = tag->tag; +else if (!strcmp (name,"type" ) && tag->tagged) + v->s =typename (tag->tagged->type); +else if (!strcmp (name,"object" ) && tag->tagged) { +char *s =xmalloc (41 ); +strcpy (s,sha1_to_hex (tag->tagged->sha1)); + v->s = s; + } } }
t/t6300-for-each-ref.sh +2 -2
@@ -74,8 +74,8 @@ test_atom tag objectname 98b46b1d36e5b07909de1b3886224e3e81e87322 test_atom tag tree '' test_atom tag parent '' test_atom tag numparent '' -test_atom tag object '67a36f10722846e891fbada1ba48ed035de75581' failure -test_atom tag type 'commit' failure +test_atom tag object '67a36f10722846e891fbada1ba48ed035de75581' +test_atom tag type 'commit' test_atom tag author '' test_atom tag authorname '' test_atom tag authoremail ''
Signature · 1 line
1.5.6
Quoted 7 lines · Jeff King
Quoted 4 lines
Hmm. Actually, there is a test in t6300 that runs with each atom name. Unfortunately, it doesn't bother actually checking the output for sanity, so the fact that these atoms returned the empty string was missed.
And here is a two patch series cleaning up the test script and fixing the bug.
Hmm, looks like I just missed you applying the original fix to maint. Ignore 2/2, then, as you already have it. I think it is still worth doing the test script cleanup, though. Here it is, rebased on the current master (the only difference is we don't have to mark the breakage as expect_failure).
-- >8 --
improve for-each-ref test script
Previously, we did a sanity check by doing for-each-ref using each possible format atom. However, we never checked the actual output produced by that atom, which recently let an obvious bug go undetected for some time.
While we're at it, also clean up a few '!' intotest_must_fail.
--- t/t6300-for-each-ref.sh | 87 +++++++++++++++++++++++++++++++++++++--------- 1 files changed, 70 insertions(+), 17 deletions(-)
t/t6300-for-each-ref.sh +70 -17
@@ -26,25 +26,78 @@ test_expect_success 'Create sample commit with known timestamp' ' git tag -a -m "Tagging at $datestamp" testtag ' -test_expect_success 'Check atom names are valid' ' - bad= - for token in \ - refname objecttype objectsize objectname tree parent \ - numparent object type author authorname authoremail \ - authordate committer committername committeremail \ - committerdate tag tagger taggername taggeremail \ - taggerdate creator creatordate subject body contents - do - git for-each-ref --format="$token=%($token)" refs/heads || { - bad=$token - break - } - done - test -z "$bad" +test_atom() { + case "$1" in + head) ref=refs/heads/master ;; + tag) ref=refs/tags/testtag ;; + esac + printf '%s\n' "$3" >expected + test_expect_${4:-success} "basic atom: $1 $2" " + git for-each-ref --format='%($2)' $ref >actual && + test_cmp expected actual + " +} + +test_atom head refname refs/heads/master +test_atom head objecttype commit +test_atom head objectsize 171 +test_atom head objectname 67a36f10722846e891fbada1ba48ed035de75581 +test_atom head tree 0e51c00fcb93dffc755546f27593d511e1bdb46f +test_atom head parent '' +test_atom head numparent 0 +test_atom head object '' +test_atom head type '' +test_atom head author 'A U Thor <[email protected]> 1151939924 +0200' +test_atom head authorname 'A U Thor' +test_atom head authoremail '<[email protected]>' +test_atom head authordate 'Mon Jul 3 17:18:44 2006 +0200' +test_atom head committer 'C O Mitter <[email protected]> 1151939923 +0200' +test_atom head committername 'C O Mitter' +test_atom head committeremail '<[email protected]>' +test_atom head committerdate 'Mon Jul 3 17:18:43 2006 +0200' +test_atom head tag '' +test_atom head tagger '' +test_atom head taggername '' +test_atom head taggeremail '' +test_atom head taggerdate '' +test_atom head creator 'C O Mitter <[email protected]> 1151939923 +0200' +test_atom head creatordate 'Mon Jul 3 17:18:43 2006 +0200' +test_atom head subject 'Initial' +test_atom head body '' +test_atom head contents 'Initial +' + +test_atom tag refname refs/tags/testtag +test_atom tag objecttype tag +test_atom tag objectsize 154 +test_atom tag objectname 98b46b1d36e5b07909de1b3886224e3e81e87322 +test_atom tag tree '' +test_atom tag parent '' +test_atom tag numparent '' +test_atom tag object '67a36f10722846e891fbada1ba48ed035de75581' +test_atom tag type 'commit' +test_atom tag author '' +test_atom tag authorname '' +test_atom tag authoremail '' +test_atom tag authordate '' +test_atom tag committer '' +test_atom tag committername '' +test_atom tag committeremail '' +test_atom tag committerdate '' +test_atom tag tag 'testtag' +test_atom tag tagger 'C O Mitter <[email protected]> 1151939925 +0200' +test_atom tag taggername 'C O Mitter' +test_atom tag taggeremail '<[email protected]>' +test_atom tag taggerdate 'Mon Jul 3 17:18:45 2006 +0200' +test_atom tag creator 'C O Mitter <[email protected]> 1151939925 +0200' +test_atom tag creatordate 'Mon Jul 3 17:18:45 2006 +0200' +test_atom tag subject 'Tagging at 1151939927' +test_atom tag body '' +test_atom tag contents 'Tagging at 1151939927 ' test_expect_success 'Check invalid atoms names are errors' ' - ! git-for-each-ref --format="%(INVALID)" refs/heads + test_must_fail git-for-each-ref --format="%(INVALID)" refs/heads ' test_expect_success 'Check format specifiers are ignored in naming date atoms' '
@@ -64,7 +117,7 @@ test_expect_success 'Check valid format specifiers for date fields' ' ' test_expect_success 'Check invalid format specifiers are errors' ' - ! git-for-each-ref --format="%(authordate:INVALID)" refs/heads + test_must_fail git-for-each-ref --format="%(authordate:INVALID)" refs/heads ' cat >expected <<\EOF
Signature · 1 line
1.5.6.129.g8dea5.dirty
Quoted 3 lines · Jeff King
Hmm, looks like I just missed you applying the original fix to maint. Ignore 2/2, then, as you already have it. I think it is still worth doing the test script cleanup, though.
Absolutely. And thanks.