{"thread":{"id":"53789","subject":"[PATCH v2 0/2] Add support for %(contents:size) in ref-filter","startedAt":"2020-07-02T14:09:03Z","lastAt":"2020-07-08T04:43:50Z","messageCount":10,"participants":["Christian Couder","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"400967","messageId":"20200702140845.24945-1-chriscool@tuxfamily.org","threadId":"53789","inReplyTo":null,"subject":"[PATCH v2 0/2] Add support for %(contents:size) in ref-filter","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-02T14:08:43Z","receivedAt":"2020-07-02T14:09:03Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This is version 2 of a very small patch series to teach ref-filter\nabout %(contents:size).\n\nVersion 1 consisted on a single patch which is available here:\n\nhttps://lore.kernel.org/git/20200701132308.16691-1-chriscool@tuxfamily.org/\n\nAs suggested by Peff, I added a preparatory patch (1/2) to clean up\nthe documentation about the %(contents:XXXX) format specifiers.\n\nThe other difference with V1 is that there are more tests in patch\n2/2. These new tests required a small helper function to be\nintroduced.\n\nChristian Couder (2):\n  Documentation: clarify %(contents:XXXX) doc\n  ref-filter: add support for %(contents:size)\n\n Documentation/git-for-each-ref.txt | 27 +++++++++++++++++++++------\n ref-filter.c                       |  7 ++++++-\n t/t6300-for-each-ref.sh            | 17 +++++++++++++++++\n 3 files changed, 44 insertions(+), 7 deletions(-)\n\n-- \n2.27.0.221.ga08a83db2b.dirty\n\n"},{"id":"400968","messageId":"20200702140845.24945-2-chriscool@tuxfamily.org","threadId":"53789","inReplyTo":"20200702140845.24945-1-chriscool@tuxfamily.org","subject":"[PATCH v2 1/2] Documentation: clarify %(contents:XXXX) doc","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-02T14:08:44Z","receivedAt":"2020-07-02T14:09:04Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Let's avoid a big dense paragraph by using an unordered\nlist for the %(contents:XXXX) format specifiers.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Documentation/git-for-each-ref.txt | 24 ++++++++++++++++++------\n 1 file changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 6dcd39f6f6..2db9779d54 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -232,12 +232,24 @@ Fields that have name-email-date tuple as its value (`author`,\n `committer`, and `tagger`) can be suffixed with `name`, `email`,\n and `date` to extract the named component.\n \n-The complete message in a commit and tag object is `contents`.\n-Its first line is `contents:subject`, where subject is the concatenation\n-of all lines of the commit message up to the first blank line.  The next\n-line is `contents:body`, where body is all of the lines after the first\n-blank line.  The optional GPG signature is `contents:signature`.  The\n-first `N` lines of the message is obtained using `contents:lines=N`.\n+The complete message of a commit or tag object is `contents`. This\n+field can also be used in the following ways:\n+\n+contents:subject::\n+\tThe \"subject\" of the commit or tag message.  It's actually the\n+\tconcatenation of all lines of the commit message up to the\n+\tfirst blank line.\n+\n+contents:body::\n+\tThe \"body\" of the commit or tag message.  It's made of the\n+\tlines after the first blank line.\n+\n+contents:signature::\n+\tThe optional GPG signature.\n+\n+contents:lines=N::\n+\tThe first `N` lines of the message.\n+\n Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n are obtained as `trailers` (or by using the historical alias\n `contents:trailers`).  Non-trailer lines from the trailer block can be omitted\n-- \n2.27.0.221.ga08a83db2b.dirty\n\n"},{"id":"400969","messageId":"20200702140845.24945-3-chriscool@tuxfamily.org","threadId":"53789","inReplyTo":"20200702140845.24945-1-chriscool@tuxfamily.org","subject":"[PATCH v2 2/2] ref-filter: add support for %(contents:size)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-02T14:08:45Z","receivedAt":"2020-07-02T14:09:07Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"It's useful and efficient to be able to get the size of the\ncontents directly without having to pipe through `wc -c`.\n\nAlso the result of the following:\n\n`git for-each-ref --format='%(contents)' | wc -c`\n\nis off by one as `git for-each-ref` appends a newline character\nafter the contents, which can be seen by comparing its ouput\nwith the output from `git cat-file`.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Documentation/git-for-each-ref.txt |  3 +++\n ref-filter.c                       |  7 ++++++-\n t/t6300-for-each-ref.sh            | 17 +++++++++++++++++\n 3 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 2db9779d54..833641eacd 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -235,6 +235,9 @@ and `date` to extract the named component.\n The complete message of a commit or tag object is `contents`. This\n field can also be used in the following ways:\n \n+contents:size::\n+\tThe size in bytes of the complete message.\n+\n contents:subject::\n \tThe \"subject\" of the commit or tag message.  It's actually the\n \tconcatenation of all lines of the commit message up to the\ndiff --git a/ref-filter.c b/ref-filter.c\nindex bf7b70299b..036a95d0d2 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -127,7 +127,8 @@ static struct used_atom {\n \t\t\tunsigned int nobracket : 1, push : 1, push_remote : 1;\n \t\t} remote_ref;\n \t\tstruct {\n-\t\t\tenum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB, C_TRAILERS } option;\n+\t\t\tenum { C_BARE, C_BODY, C_BODY_DEP, C_LENGTH,\n+\t\t\t       C_LINES, C_SIG, C_SUB, C_TRAILERS } option;\n \t\t\tstruct process_trailer_options trailer_opts;\n \t\t\tunsigned int nlines;\n \t\t} contents;\n@@ -338,6 +339,8 @@ static int contents_atom_parser(const struct ref_format *format, struct used_ato\n \t\tatom->u.contents.option = C_BARE;\n \telse if (!strcmp(arg, \"body\"))\n \t\tatom->u.contents.option = C_BODY;\n+\telse if (!strcmp(arg, \"size\"))\n+\t\tatom->u.contents.option = C_LENGTH;\n \telse if (!strcmp(arg, \"signature\"))\n \t\tatom->u.contents.option = C_SIG;\n \telse if (!strcmp(arg, \"subject\"))\n@@ -1253,6 +1256,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = copy_subject(subpos, sublen);\n \t\telse if (atom->u.contents.option == C_BODY_DEP)\n \t\t\tv->s = xmemdupz(bodypos, bodylen);\n+\t\telse if (atom->u.contents.option == C_LENGTH)\n+\t\t\tv->s = xstrfmt(\"%ld\", strlen(subpos));\n \t\telse if (atom->u.contents.option == C_BODY)\n \t\t\tv->s = xmemdupz(bodypos, nonsiglen);\n \t\telse if (atom->u.contents.option == C_SIG)\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex da59fadc5d..ceee8d9372 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -125,6 +125,7 @@ test_atom head contents:body ''\n test_atom head contents:signature ''\n test_atom head contents 'Initial\n '\n+test_atom head contents:size '8'\n test_atom head HEAD '*'\n \n test_atom tag refname refs/tags/testtag\n@@ -170,6 +171,7 @@ test_atom tag contents:body ''\n test_atom tag contents:signature ''\n test_atom tag contents 'Tagging at 1151968727\n '\n+test_atom tag contents:size '22'\n test_atom tag HEAD ' '\n \n test_expect_success 'Check invalid atoms names are errors' '\n@@ -580,6 +582,7 @@ test_atom refs/tags/subject-body contents 'the subject line\n first body line\n second body line\n '\n+test_atom refs/tags/subject-body contents:size '51'\n \n test_expect_success 'create tag with multiline subject' '\n \tcat >msg <<-\\EOF &&\n@@ -606,6 +609,7 @@ second subject line\n first body line\n second body line\n '\n+test_atom refs/tags/multiline contents:size '73'\n \n test_expect_success GPG 'create signed tags' '\n \tgit tag -s -m \"\" signed-empty &&\n@@ -622,6 +626,16 @@ sig='-----BEGIN PGP SIGNATURE-----\n -----END PGP SIGNATURE-----\n '\n \n+# We cannot use test_atom to check contents:size of signed tags due to sanitize_pgp\n+test_tag_contents_size_pgp () {\n+\tref=\"$1\"\n+\ttest_expect_success $PREREQ \"basic atom: $ref contents:size\" \"\n+\t\tgit cat-file tag $ref | tail -n +6 | wc -c >expected &&\n+\t\tgit for-each-ref --format='%(contents:size)' $ref >actual &&\n+\t\ttest_cmp expected actual\n+\t\"\n+}\n+\n PREREQ=GPG\n test_atom refs/tags/signed-empty subject ''\n test_atom refs/tags/signed-empty contents:subject ''\n@@ -629,6 +643,7 @@ test_atom refs/tags/signed-empty body \"$sig\"\n test_atom refs/tags/signed-empty contents:body ''\n test_atom refs/tags/signed-empty contents:signature \"$sig\"\n test_atom refs/tags/signed-empty contents \"$sig\"\n+test_tag_contents_size_pgp refs/tags/signed-empty\n \n test_atom refs/tags/signed-short subject 'subject line'\n test_atom refs/tags/signed-short contents:subject 'subject line'\n@@ -637,6 +652,7 @@ test_atom refs/tags/signed-short contents:body ''\n test_atom refs/tags/signed-short contents:signature \"$sig\"\n test_atom refs/tags/signed-short contents \"subject line\n $sig\"\n+test_tag_contents_size_pgp refs/tags/signed-short\n \n test_atom refs/tags/signed-long subject 'subject line'\n test_atom refs/tags/signed-long contents:subject 'subject line'\n@@ -649,6 +665,7 @@ test_atom refs/tags/signed-long contents \"subject line\n \n body contents\n $sig\"\n+test_tag_contents_size_pgp refs/tags/signed-long\n \n test_expect_success 'set up multiple-sort tags' '\n \tfor when in 100000 200000\n-- \n2.27.0.221.ga08a83db2b.dirty\n\n"},{"id":"401054","messageId":"xmqqa70c9vkb.fsf@gitster.c.googlers.com","threadId":"53789","inReplyTo":"20200702140845.24945-2-chriscool@tuxfamily.org","subject":"Re: [PATCH v2 1/2] Documentation: clarify %(contents:XXXX) doc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-06T21:34:44Z","receivedAt":"2020-07-06T21:34:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> Let's avoid a big dense paragraph by using an unordered\n> list for the %(contents:XXXX) format specifiers.\n>\n> Suggested-by: Jeff King <peff@peff.net>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  Documentation/git-for-each-ref.txt | 24 ++++++++++++++++++------\n>  1 file changed, 18 insertions(+), 6 deletions(-)\n\nNice.\nWill queue.  Thanks.\n\n>\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index 6dcd39f6f6..2db9779d54 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -232,12 +232,24 @@ Fields that have name-email-date tuple as its value (`author`,\n>  `committer`, and `tagger`) can be suffixed with `name`, `email`,\n>  and `date` to extract the named component.\n>  \n> -The complete message in a commit and tag object is `contents`.\n> -Its first line is `contents:subject`, where subject is the concatenation\n> -of all lines of the commit message up to the first blank line.  The next\n> -line is `contents:body`, where body is all of the lines after the first\n> -blank line.  The optional GPG signature is `contents:signature`.  The\n> -first `N` lines of the message is obtained using `contents:lines=N`.\n> +The complete message of a commit or tag object is `contents`. This\n> +field can also be used in the following ways:\n> +\n> +contents:subject::\n> +\tThe \"subject\" of the commit or tag message.  It's actually the\n> +\tconcatenation of all lines of the commit message up to the\n> +\tfirst blank line.\n> +\n> +contents:body::\n> +\tThe \"body\" of the commit or tag message.  It's made of the\n> +\tlines after the first blank line.\n> +\n> +contents:signature::\n> +\tThe optional GPG signature.\n> +\n> +contents:lines=N::\n> +\tThe first `N` lines of the message.\n> +\n>  Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n>  are obtained as `trailers` (or by using the historical alias\n>  `contents:trailers`).  Non-trailer lines from the trailer block can be omitted\n"},{"id":"401055","messageId":"xmqq5zb09v4e.fsf@gitster.c.googlers.com","threadId":"53789","inReplyTo":"20200702140845.24945-3-chriscool@tuxfamily.org","subject":"Re: [PATCH v2 2/2] ref-filter: add support for %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-06T21:44:17Z","receivedAt":"2020-07-06T21:44:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> It's useful and efficient to be able to get the size of the\n> contents directly without having to pipe through `wc -c`.\n>\n> Also the result of the following:\n>\n> `git for-each-ref --format='%(contents)' | wc -c`\n>\n> is off by one as `git for-each-ref` appends a newline character\n> after the contents, which can be seen by comparing its ouput\n> with the output from `git cat-file`.\n\nSo that's off by number of refs that are shown?\n\n> +contents:size::\n> +\tThe size in bytes of the complete message.\n> +\n\nComplete as opposed to what?  \n\nWhat happens when the object referred to by the ref is not a commit\nor a tag?\n\nI am fine if it just is silently ignored (which is consistent with\nalready existing behaviour of other requests that do not make sense\nfor the given type) if the thing is a blob or a tree, but we'd need\nto cover the case with a test or two.  It seems you only expect this\nwith a tag object and do not have any test that checks for other\ntypes of objects?\n\nThanks.\n\n> +# We cannot use test_atom to check contents:size of signed tags due to sanitize_pgp\n> +test_tag_contents_size_pgp () {\n> +\tref=\"$1\"\n> +\ttest_expect_success $PREREQ \"basic atom: $ref contents:size\" \"\n> +\t\tgit cat-file tag $ref | tail -n +6 | wc -c >expected &&\n> +\t\tgit for-each-ref --format='%(contents:size)' $ref >actual &&\n> +\t\ttest_cmp expected actual\n> +\t\"\n> +}\n> +\n>  PREREQ=GPG\n>  test_atom refs/tags/signed-empty subject ''\n>  test_atom refs/tags/signed-empty contents:subject ''\n> @@ -629,6 +643,7 @@ test_atom refs/tags/signed-empty body \"$sig\"\n>  test_atom refs/tags/signed-empty contents:body ''\n>  test_atom refs/tags/signed-empty contents:signature \"$sig\"\n>  test_atom refs/tags/signed-empty contents \"$sig\"\n> +test_tag_contents_size_pgp refs/tags/signed-empty\n>  \n>  test_atom refs/tags/signed-short subject 'subject line'\n>  test_atom refs/tags/signed-short contents:subject 'subject line'\n> @@ -637,6 +652,7 @@ test_atom refs/tags/signed-short contents:body ''\n>  test_atom refs/tags/signed-short contents:signature \"$sig\"\n>  test_atom refs/tags/signed-short contents \"subject line\n>  $sig\"\n> +test_tag_contents_size_pgp refs/tags/signed-short\n>  \n>  test_atom refs/tags/signed-long subject 'subject line'\n>  test_atom refs/tags/signed-long contents:subject 'subject line'\n> @@ -649,6 +665,7 @@ test_atom refs/tags/signed-long contents \"subject line\n>  \n>  body contents\n>  $sig\"\n> +test_tag_contents_size_pgp refs/tags/signed-long\n>  \n>  test_expect_success 'set up multiple-sort tags' '\n>  \tfor when in 100000 200000\n"},{"id":"401067","messageId":"20200707050247.GA105016@coredump.intra.peff.net","threadId":"53789","inReplyTo":"20200702140845.24945-1-chriscool@tuxfamily.org","subject":"Re: [PATCH v2 0/2] Add support for %(contents:size) in ref-filter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-07T05:02:47Z","receivedAt":"2020-07-07T05:02:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 02, 2020 at 04:08:43PM +0200, Christian Couder wrote:\n\n> This is version 2 of a very small patch series to teach ref-filter\n> about %(contents:size).\n> \n> Version 1 consisted on a single patch which is available here:\n> \n> https://lore.kernel.org/git/20200701132308.16691-1-chriscool@tuxfamily.org/\n> \n> As suggested by Peff, I added a preparatory patch (1/2) to clean up\n> the documentation about the %(contents:XXXX) format specifiers.\n\nThanks, I think it looks much nicer.\n\n> The other difference with V1 is that there are more tests in patch\n> 2/2. These new tests required a small helper function to be\n> introduced.\n\nI'm still not sure why %(objectsize) isn't sufficient here. Is there\nsome use case that's served by %(contents:size) that it wouldn't work\nfor? Or are we just trying to make it more discoverable when you're\nlooking at the contents already?\n\n-Peff\n"},{"id":"401077","messageId":"CAP8UFD0gUK4TGUA7JZ_V7hFe4LSDAP7DfPyz4gqy1RjLXJPF2w@mail.gmail.com","threadId":"53789","inReplyTo":"20200707050247.GA105016@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/2] Add support for %(contents:size) in ref-filter","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-07T06:19:06Z","receivedAt":"2020-07-07T06:19:21Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Jul 7, 2020 at 7:02 AM Jeff King <peff@peff.net> wrote:\n>\n> On Thu, Jul 02, 2020 at 04:08:43PM +0200, Christian Couder wrote:\n\n> > The other difference with V1 is that there are more tests in patch\n> > 2/2. These new tests required a small helper function to be\n> > introduced.\n>\n> I'm still not sure why %(objectsize) isn't sufficient here. Is there\n> some use case that's served by %(contents:size) that it wouldn't work\n> for? Or are we just trying to make it more discoverable when you're\n> looking at the contents already?\n\n%(objectsize) is the size of the whole commit or tag object, while\n%(contents:size) is the size of the complete message (the whole commit\nmessage or tag message, including trailers and signatures).\n\n$ git for-each-ref --format='refname: %(refname)%0a%09objectsize:\n%(objectsize)%0a%09contents:size: %(contents:size)'\nrefs/heads/{master,next,seen} refs/tags/v2.2{5,6,7}.0\nrefname: refs/heads/master\n        objectsize: 285\n        contents:size: 69\nrefname: refs/heads/next\n        objectsize: 281\n        contents:size: 17\nrefname: refs/heads/seen\n        objectsize: 589\n        contents:size: 325\nrefname: refs/tags/v2.25.0\n        objectsize: 974\n        contents:size: 842\nrefname: refs/tags/v2.26.0\n        objectsize: 974\n        contents:size: 842\nrefname: refs/tags/v2.27.0\n        objectsize: 974\n        contents:size: 842\n\nWhen sending only the complete message, not the whole object, through\na protocol, it might be interesting to easily get the length of the\ncomplete message. For example one could use \"Content-Length:\n%(contents:size)%0a\" when sending a complete message over HTTP.\n"},{"id":"401085","messageId":"CAP8UFD1JiBOhpXeADObFBgoGm292dxQ933TENrZNPnmSv+SbUg@mail.gmail.com","threadId":"53789","inReplyTo":"xmqq5zb09v4e.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 2/2] ref-filter: add support for %(contents:size)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-07T08:40:10Z","receivedAt":"2020-07-07T08:40:24Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jul 6, 2020 at 11:44 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n> > It's useful and efficient to be able to get the size of the\n> > contents directly without having to pipe through `wc -c`.\n> >\n> > Also the result of the following:\n> >\n> > `git for-each-ref --format='%(contents)' | wc -c`\n> >\n> > is off by one as `git for-each-ref` appends a newline character\n> > after the contents, which can be seen by comparing its ouput\n> > with the output from `git cat-file`.\n>\n> So that's off by number of refs that are shown?\n\nYeah, true. I should have added a ref as I mean something like the\nfollowing is off by one:\n\n`git for-each-ref --format='%(contents)' refs/heads/my-branch | wc -c`\n\nI have changed it to the above in my current version.\n\n> > +contents:size::\n> > +     The size in bytes of the complete message.\n> > +\n>\n> Complete as opposed to what?\n\nIn the existing documentation there is already \"The complete message\nof a commit or tag object is `contents`. \"\n\nSo yeah I could add another preparatory patch to change the above to\nsomething like:\n\n\"The complete message (subject, body, trailers and signature) of a\ncommit or tag object is `contents`. \"\n\n> What happens when the object referred to by the ref is not a commit\n> or a tag?\n\nRight now %(contents) shows nothing (a blank line actually) when the\nref points to something other than a commit or a tag, and\n%(contents:size) does the same:\n\n```\n$ git update-ref refs/mytrees/first HEAD^{tree}\n$ git for-each-ref --format='%(contents)' refs/mytrees/first\n\n$ git for-each-ref --format='%(contents:size)' refs/mytrees/first\n\n```\n\nI am not sure if it's worth updating the existing documentation or\njust the commit message of this patch. For now I have done the latter\nin my current version.\n\n> I am fine if it just is silently ignored (which is consistent with\n> already existing behaviour of other requests that do not make sense\n> for the given type) if the thing is a blob or a tree, but we'd need\n> to cover the case with a test or two.  It seems you only expect this\n> with a tag object and do not have any test that checks for other\n> types of objects?\n\nMy patch already adds 1 test with a commit:\n\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -125,6 +125,7 @@ test_atom head contents:body ''\n test_atom head contents:signature ''\n test_atom head contents 'Initial\n '\n+test_atom head contents:size '8'\n test_atom head HEAD '*'\n\nThere is only one test with a commit, because that's already the case\nfor %(contents) too.\n\nI am ok with adding another preparatory patch to the series that would\nadd a few more test cases with commits, trees and blobs though.\n"},{"id":"401126","messageId":"xmqqr1tn5oq2.fsf@gitster.c.googlers.com","threadId":"53789","inReplyTo":"CAP8UFD1JiBOhpXeADObFBgoGm292dxQ933TENrZNPnmSv+SbUg@mail.gmail.com","subject":"Re: [PATCH v2 2/2] ref-filter: add support for %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-07T15:28:21Z","receivedAt":"2020-07-07T15:28:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n>> I am fine if it just is silently ignored (which is consistent with\n>> already existing behaviour of other requests that do not make sense\n>> for the given type) if the thing is a blob or a tree, but we'd need\n>> to cover the case with a test or two.  It seems you only expect this\n>> with a tag object and do not have any test that checks for other\n>> types of objects?\n>\n> My patch already adds 1 test with a commit:\n\nBut that is not an interesting case, no?  Unless I missed that there\nwere new tests to see what happens with a blob and a tree, that is.\n\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -125,6 +125,7 @@ test_atom head contents:body ''\n>  test_atom head contents:signature ''\n>  test_atom head contents 'Initial\n>  '\n> +test_atom head contents:size '8'\n>  test_atom head HEAD '*'\n>\n> There is only one test with a commit, because that's already the case\n> for %(contents) too.\n>\n> I am ok with adding another preparatory patch to the series that would\n> add a few more test cases with commits, trees and blobs though.\n\nI am OK either way, because I know fairly well how these formatting atom\nsystem works (I had heavy involvement in the initial one) and I know\nit would be hard to make it do nonsensical things when given an\nobject of an unexpected type.\n\nBut I was hoping some among us experienced contributors would lead\nbeginners by example of making sure we test both positive (\"see, my\nshiny new toy does work\") and negative (\"and it won't do nonsensical\nthings when given an input it is not designed to work with\") cases.\n\nThanks.\n"},{"id":"401176","messageId":"20200708044347.GB2303891@coredump.intra.peff.net","threadId":"53789","inReplyTo":"CAP8UFD0gUK4TGUA7JZ_V7hFe4LSDAP7DfPyz4gqy1RjLXJPF2w@mail.gmail.com","subject":"Re: [PATCH v2 0/2] Add support for %(contents:size) in ref-filter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-08T04:43:47Z","receivedAt":"2020-07-08T04:43:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 07, 2020 at 08:19:06AM +0200, Christian Couder wrote:\n\n> > I'm still not sure why %(objectsize) isn't sufficient here. Is there\n> > some use case that's served by %(contents:size) that it wouldn't work\n> > for? Or are we just trying to make it more discoverable when you're\n> > looking at the contents already?\n> \n> %(objectsize) is the size of the whole commit or tag object, while\n> %(contents:size) is the size of the complete message (the whole commit\n> message or tag message, including trailers and signatures).\n\nAh, right, that makes sense.\n\nI'd probably use \"git log --no-walk --format=%B\" or similar for this,\nbut there is nothing wrong with using for-each-ref (and it is better if\nyou really do care about properties of the refs themselves, and not just\nthe commit they point to).\n\nI do think in the long run it might be nice to have a generic\nplaceholder for \"expand this thing and give me the number of bytes\", so\nwe could do:\n\n  %(sizeof:%(contents))\n\nor even:\n\n  %(sizeof:%(authorname) <%(authoremail)>)\n\nbut that is definitely outside the scope. If we end up eventually with\na generic mechanism and have to support contents:size forever for\ncompatibility, I don't think it is that big a problem.\n\n-Peff\n"}]}