{"thread":{"id":"53811","subject":"[PATCH v3 0/4] Add support for %(contents:size) in ref-filter","startedAt":"2020-07-07T17:41:07Z","lastAt":"2020-07-31T20:41:00Z","messageCount":39,"participants":["Christian Couder","Junio C Hamano","Alban Gruin","Jeff King"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"401137","messageId":"20200707174049.21714-1-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":null,"subject":"[PATCH v3 0/4] Add support for %(contents:size) in ref-filter","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-07T17:40:45Z","receivedAt":"2020-07-07T17:41:07Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This is version 3 of a small patch series to teach ref-filter about\n%(contents:size).\n\nThis patch series is based on current master.\n\nPrevious versions and related discussions are here:\n\nV1: https://lore.kernel.org/git/20200701132308.16691-1-chriscool@tuxfamily.org/\nV2: https://lore.kernel.org/git/20200702140845.24945-1-chriscool@tuxfamily.org/\n\nThanks to Junio and Peff for their reviews of this series!\n\nThe changes compared to V2 are the following:\n\n - Added patch 2/4 that clarifies the meaning of \"complete message\"\n in the doc.\n\n - Added patch 3/4 that adds tests for refs pointing to a tree or a\n blob.\n\n - Improved commit message in patch 4/4 as suggested by Junio.\n\n - Added %(contents:size) tests in patch 4/4 for refs pointing to a\n tree or a blob.\n\nThe range diff is:\n\n1: c6e80b8bc1 = 1: b04b390f32 Documentation: clarify %(contents:XXXX) doc\n-: ---------- > 2: b62cab2630 Documentation: clarify 'complete message'\n-: ---------- > 3: b9584472a1 t6300: test refs pointing to tree and blob\n2: 9853b37091 ! 4: 23f941132e ref-filter: add support for %(contents:size)\n @@ Commit message\n \n Also the result of the following:\n \n - `git for-each-ref --format='%(contents)' | wc -c`\n + `git for-each-ref --format='%(contents)' refs/heads/my-branch | 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 + after the contents, which can be seen by comparing its output\n with the output from `git cat-file`.\n \n + As with %(contents), %(contents:size) is silently ignored, if a\n + ref points to something other than a commit or a tag:\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 +\n Signed-off-by: Christian Couder \n \n ## Documentation/git-for-each-ref.txt ##\n -@@ Documentation/git-for-each-ref.txt: 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 +@@ Documentation/git-for-each-ref.txt: The complete message (subject, body, trailers and signature) of a\n + commit or tag object is `contents`. This field can also be used in the\n + following ways:\n \n +contents:size::\n + The size in bytes of the complete message.\n @@ t/t6300-for-each-ref.sh: test_atom refs/tags/signed-long contents \"subject line\n $sig\"\n +test_tag_contents_size_pgp refs/tags/signed-long\n \n + test_expect_success 'set up refs pointing to tree and blob' '\n + git update-ref refs/mytrees/first refs/heads/master^{tree} &&\n +@@ t/t6300-for-each-ref.sh: test_atom refs/mytrees/first body \"\"\n + test_atom refs/mytrees/first contents:body \"\"\n + test_atom refs/mytrees/first contents:signature \"\"\n + test_atom refs/mytrees/first contents \"\"\n ++test_atom refs/mytrees/first contents:size \"\"\n + \n + test_atom refs/myblobs/first subject \"\"\n + test_atom refs/myblobs/first contents:subject \"\"\n +@@ t/t6300-for-each-ref.sh: test_atom refs/myblobs/first body \"\"\n + test_atom refs/myblobs/first contents:body \"\"\n + test_atom refs/myblobs/first contents:signature \"\"\n + test_atom refs/myblobs/first contents \"\"\n ++test_atom refs/myblobs/first contents:size \"\"\n + \n test_expect_success 'set up multiple-sort tags' '\n for when in 100000 200000\n\nChristian Couder (4):\n Documentation: clarify %(contents:XXXX) doc\n Documentation: clarify 'complete message'\n t6300: test refs pointing to tree and blob\n ref-filter: add support for %(contents:size)\n\n Documentation/git-for-each-ref.txt | 28 +++++++++++++++-----\n ref-filter.c | 7 ++++-\n t/t6300-for-each-ref.sh | 41 ++++++++++++++++++++++++++++++\n 3 files changed, 69 insertions(+), 7 deletions(-)\n\n-- \n2.27.0.460.g66f3a24dd5\n\n"},{"id":"401138","messageId":"20200707174049.21714-2-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200707174049.21714-1-chriscool@tuxfamily.org","subject":"[PATCH v3 1/4] Documentation: clarify %(contents:XXXX) doc","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-07T17:40:46Z","receivedAt":"2020-07-07T17:41:08Z","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 \nSigned-off-by: Christian Couder \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.460.g66f3a24dd5\n\n"},{"id":"401139","messageId":"20200707174049.21714-3-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200707174049.21714-1-chriscool@tuxfamily.org","subject":"[PATCH v3 2/4] Documentation: clarify 'complete message'","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-07T17:40:47Z","receivedAt":"2020-07-07T17:41:09Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"In Documentation/git-for-each-ref.txt let's clarify what\nwe mean by \"complete message\".\n\nSigned-off-by: Christian Couder \n---\n Documentation/git-for-each-ref.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 2db9779d54..788258c3ad 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -232,8 +232,9 @@ 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 of a commit or tag object is `contents`. This\n-field can also be used in the following ways:\n+The complete message (subject, body, trailers and signature) of a\n+commit or tag object is `contents`. This field can also be used in the\n+following ways:\n \n contents:subject::\n \tThe \"subject\" of the commit or tag message. It's actually the\n-- \n2.27.0.460.g66f3a24dd5\n\n"},{"id":"401140","messageId":"20200707174049.21714-4-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200707174049.21714-1-chriscool@tuxfamily.org","subject":"[PATCH v3 3/4] t6300: test refs pointing to tree and blob","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-07T17:40:48Z","receivedAt":"2020-07-07T17:41:11Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Adding tests for refs pointing to tree and blob shows that\nwe care about testing both positive (\"see, my shiny new toy\ndoes work\") and negative (\"and it won't do nonsensical\nthings when given an input it is not designed to work with\")\ncases.\n\nSigned-off-by: Christian Couder \n---\n t/t6300-for-each-ref.sh | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex da59fadc5d..371e45e5ad 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -650,6 +650,28 @@ test_atom refs/tags/signed-long contents \"subject line\n body contents\n $sig\"\n \n+test_expect_success 'set up refs pointing to tree and blob' '\n+\tgit update-ref refs/mytrees/first refs/heads/master^{tree} &&\n+\tgit ls-tree refs/mytrees/first one >one_info &&\n+\ttest $(cut -d\" \" -f2 one_info) = \"blob\" &&\n+\tblob_hash=$(cut \"-d\t\" -f1 one_info | cut -d\" \" -f3) &&\n+\tgit update-ref refs/myblobs/first \"$blob_hash\"\n+'\n+\n+test_atom refs/mytrees/first subject \"\"\n+test_atom refs/mytrees/first contents:subject \"\"\n+test_atom refs/mytrees/first body \"\"\n+test_atom refs/mytrees/first contents:body \"\"\n+test_atom refs/mytrees/first contents:signature \"\"\n+test_atom refs/mytrees/first contents \"\"\n+\n+test_atom refs/myblobs/first subject \"\"\n+test_atom refs/myblobs/first contents:subject \"\"\n+test_atom refs/myblobs/first body \"\"\n+test_atom refs/myblobs/first contents:body \"\"\n+test_atom refs/myblobs/first contents:signature \"\"\n+test_atom refs/myblobs/first contents \"\"\n+\n test_expect_success 'set up multiple-sort tags' '\n \tfor when in 100000 200000\n \tdo\n-- \n2.27.0.460.g66f3a24dd5\n\n"},{"id":"401141","messageId":"20200707174049.21714-5-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200707174049.21714-1-chriscool@tuxfamily.org","subject":"[PATCH v3 4/4] ref-filter: add support for %(contents:size)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-07T17:40:49Z","receivedAt":"2020-07-07T17:41:15Z","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)' refs/heads/my-branch | 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 output\nwith the output from `git cat-file`.\n\nAs with %(contents), %(contents:size) is silently ignored, if a\nref points to something other than a commit or a tag:\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\nSigned-off-by: Christian Couder \n---\n Documentation/git-for-each-ref.txt | 3 +++\n ref-filter.c | 7 ++++++-\n t/t6300-for-each-ref.sh | 19 +++++++++++++++++++\n 3 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 788258c3ad..049bc93e6a 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -236,6 +236,9 @@ The complete message (subject, body, trailers and signature) of a\n commit or tag object is `contents`. This field can also be used in the\n 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 8447cb09be..8ec28f0535 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 371e45e5ad..b580e27a32 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 refs pointing to tree and blob' '\n \tgit update-ref refs/mytrees/first refs/heads/master^{tree} &&\n@@ -664,6 +681,7 @@ test_atom refs/mytrees/first body \"\"\n test_atom refs/mytrees/first contents:body \"\"\n test_atom refs/mytrees/first contents:signature \"\"\n test_atom refs/mytrees/first contents \"\"\n+test_atom refs/mytrees/first contents:size \"\"\n \n test_atom refs/myblobs/first subject \"\"\n test_atom refs/myblobs/first contents:subject \"\"\n@@ -671,6 +689,7 @@ test_atom refs/myblobs/first body \"\"\n test_atom refs/myblobs/first contents:body \"\"\n test_atom refs/myblobs/first contents:signature \"\"\n test_atom refs/myblobs/first contents \"\"\n+test_atom refs/myblobs/first contents:size \"\"\n \n test_expect_success 'set up multiple-sort tags' '\n \tfor when in 100000 200000\n-- \n2.27.0.460.g66f3a24dd5\n\n"},{"id":"401146","messageId":"xmqqwo3f3zg3.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200707174049.21714-3-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 2/4] Documentation: clarify 'complete message'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-07T19:19:40Z","receivedAt":"2020-07-07T19:19:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> In Documentation/git-for-each-ref.txt let's clarify what\n> we mean by \"complete message\".\n>\n> Signed-off-by: Christian Couder \n> ---\n> Documentation/git-for-each-ref.txt | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index 2db9779d54..788258c3ad 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -232,8 +232,9 @@ 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 of a commit or tag object is `contents`. This\n> -field can also be used in the following ways:\n> +The complete message (subject, body, trailers and signature) of a\n> +commit or tag object is `contents`. This field can also be used in the\n> +following ways:\n\nHmph, I regret asking what is \"complete\", i.e. as opposed to what.\n\nThe above makes it even unclear if things like \"signature on commit\"\nis part of the complete message. I _think_ you meant the part after\nstripping the object header, so \"signature in a signed tag is part\nof 'complete message', while signature in a signed commit is not\",\nwhich feels somewhat strange.\n\nBut then, it may be easier to understand if we said\n\n The message in a commit or a tag object is `contents`, from\n which `contents:` can be used to extract various parts out\n of.\n\nwithout introducing \"complete\".\n\nIn any case I think patches 1 & 2 are definite improvement.\n\nThanks.\n"},{"id":"401148","messageId":"xmqqsge33z42.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200707174049.21714-2-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 1/4] Documentation: clarify %(contents:XXXX) doc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-07T19:26:53Z","receivedAt":"2020-07-07T19:27:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\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\nLet's avoid confusing readers by saying \"A is X. It's actually Y\".\n\n The first paragraph of the message, which typically is a single\n line, is taken as the \"subject\" of the commit or the tag\n message.\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 The remainder of the commit or the tag message that follows the\n \"subject\".\n\n> +contents:signature::\n> +\tThe optional GPG signature.\n\nI _think_ this only applies to signed tag objects and not signed\ncommit objects, but this text does not help to decide if I am\nright.\n\n> +contents:lines=N::\n> +\tThe first `N` lines of the message.\n\nGood.\n\n"},{"id":"401149","messageId":"xmqqo8or3yu6.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200707174049.21714-4-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 3/4] t6300: test refs pointing to tree and blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-07T19:32:49Z","receivedAt":"2020-07-07T19:32:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Adding tests for refs pointing to tree and blob shows that\n> we care about testing both positive (\"see, my shiny new toy\n> does work\") and negative (\"and it won't do nonsensical\n> things when given an input it is not designed to work with\")\n> cases.\n\nNice.\n\n>\n> Signed-off-by: Christian Couder \n> ---\n> t/t6300-for-each-ref.sh | 22 ++++++++++++++++++++++\n> 1 file changed, 22 insertions(+)\n>\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index da59fadc5d..371e45e5ad 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -650,6 +650,28 @@ test_atom refs/tags/signed-long contents \"subject line\n> body contents\n> $sig\"\n> \n> +test_expect_success 'set up refs pointing to tree and blob' '\n> +\tgit update-ref refs/mytrees/first refs/heads/master^{tree} &&\n> +\tgit ls-tree refs/mytrees/first one >one_info &&\n> +\ttest $(cut -d\" \" -f2 one_info) = \"blob\" &&\n> +\tblob_hash=$(cut \"-d\t\" -f1 one_info | cut -d\" \" -f3) &&\n> +\tgit update-ref refs/myblobs/first \"$blob_hash\"\n\nWouldn't it be sufficient to say\n\n\tgit update-ref refs/myblobs/first refs/heads/master:one\n\ninstead of the last 4 lines in this set-up?\n\n> +'\n> +\n> +test_atom refs/mytrees/first subject \"\"\n> +test_atom refs/mytrees/first contents:subject \"\"\n> +test_atom refs/mytrees/first body \"\"\n> +test_atom refs/mytrees/first contents:body \"\"\n> +test_atom refs/mytrees/first contents:signature \"\"\n> +test_atom refs/mytrees/first contents \"\"\n> +\n> +test_atom refs/myblobs/first subject \"\"\n> +test_atom refs/myblobs/first contents:subject \"\"\n> +test_atom refs/myblobs/first body \"\"\n> +test_atom refs/myblobs/first contents:body \"\"\n> +test_atom refs/myblobs/first contents:signature \"\"\n> +test_atom refs/myblobs/first contents \"\"\n\nAll makes sense. We require \"git for-each-ref\" that asks for these\natoms in the format to silently exit successfully when the object at\nthe tip of a ref is of these types, so all these test_atom should\nsucceed.\n\nNicely written.\n\n> test_expect_success 'set up multiple-sort tags' '\n> \tfor when in 100000 200000\n> \tdo\n"},{"id":"401150","messageId":"xmqqk0zf3y8s.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200707174049.21714-5-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 4/4] ref-filter: add support for %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-07T19:45:39Z","receivedAt":"2020-07-07T19:45:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder 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)' refs/heads/my-branch | 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 output\n> with the output from `git cat-file`.\n>\n> As with %(contents), %(contents:size) is silently ignored, if a\n> ref points to something other than a commit or a tag:\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>\n> Signed-off-by: Christian Couder \n> ---\n> Documentation/git-for-each-ref.txt | 3 +++\n> ref-filter.c | 7 ++++++-\n> t/t6300-for-each-ref.sh | 19 +++++++++++++++++++\n> 3 files changed, 28 insertions(+), 1 deletion(-)\n\nNice. The only questionable thing here is if we later regret for\nassuming that all sizes are always measured in bytes. If we later\nfind an application that wants an efficient access to \"| wc -l\"\n(instead of \"| wc -c\" that motivated this patch), we'd want to be\nable to say \"%(contents:lines)\" and at that point we may want to go\nback in time and call this \"%(contents:bytes)\" or something.\n\n> test_atom head contents 'Initial\n> '\n> +test_atom head contents:size '8'\n\nThese two are tied together (any change to the test script that\ncauses us to update the former also forces us to update the latter),\nbut I do not think of a way to unify the test without writing too\nmuch boilerplate code, so let's say this is good enough at least for\nnow (but I may change my opinion as I read along).\n\n> test_atom tag contents 'Tagging at 1151968727\n> '\n> +test_atom tag contents:size '22'\n\nLikewise.\n\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\nLikewise.\n\nOf course, we _could_ update the test_atom to do something magic\nonly when the 'contents' atom is being asked. We notice that $2 is\n'contents', do the usual test_expect_success for 'contents', and\nthen measure the byte length of $3 ourselves and test\n'contents:size'. That way, all the above test updates would become\nunnecessary (and the last two hunks of this patch can also go).\n\nThat approach may even allow you to hide the details of sanitize-pgp\nin the updated test_atom so that the actual tests may not have to get\nupdated even for signed tags.\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 refs pointing to tree and blob' '\n> \tgit update-ref refs/mytrees/first refs/heads/master^{tree} &&\n> @@ -664,6 +681,7 @@ test_atom refs/mytrees/first body \"\"\n> test_atom refs/mytrees/first contents:body \"\"\n> test_atom refs/mytrees/first contents:signature \"\"\n> test_atom refs/mytrees/first contents \"\"\n> +test_atom refs/mytrees/first contents:size \"\"\n> \n> test_atom refs/myblobs/first subject \"\"\n> test_atom refs/myblobs/first contents:subject \"\"\n> @@ -671,6 +689,7 @@ test_atom refs/myblobs/first body \"\"\n> test_atom refs/myblobs/first contents:body \"\"\n> test_atom refs/myblobs/first contents:signature \"\"\n> test_atom refs/myblobs/first contents \"\"\n> +test_atom refs/myblobs/first contents:size \"\"\n> \n> test_expect_success 'set up multiple-sort tags' '\n> \tfor when in 100000 200000\n"},{"id":"401163","messageId":"xmqqsge32cgm.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200707174049.21714-5-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 4/4] ref-filter: add support for %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-07T22:21:29Z","receivedAt":"2020-07-07T22:21:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> +\t\telse if (atom->u.contents.option == C_LENGTH)\n> +\t\t\tv->s = xstrfmt(\"%ld\", strlen(subpos));\n\nPlease squash something like this in. 32-bit builds are failing.\n\n ref-filter.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8ec28f0535..73d8bfa86d 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1257,7 +1257,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\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\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)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)\n"},{"id":"401203","messageId":"xmqq36611ubf.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200707174049.21714-5-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 4/4] ref-filter: add support for %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-08T23:05:40Z","receivedAt":"2020-07-08T23:05:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\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\nOf course, this will BREAK the tests on macOS and possibly others\nwith \"wc\" that emits leading whitespaces before the number.\n\nThe tip of 'seen' has been failing ever since this topic was merged\nthere e.g. https://travis-ci.org/github/git/git/jobs/705986794\n\nThanks.\n\n"},{"id":"401207","messageId":"xmqqtuyhzgro.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"xmqqk0zf3y8s.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 4/4] ref-filter: add support for %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-09T00:14:19Z","receivedAt":"2020-07-09T00:14:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Of course, we _could_ update the test_atom to do something magic\n> only when the 'contents' atom is being asked. We notice that $2 is\n> 'contents', do the usual test_expect_success for 'contents', and\n> then measure the byte length of $3 ourselves and test\n> 'contents:size'. That way, all the above test updates would become\n> unnecessary (and the last two hunks of this patch can also go).\n>\n> That approach may even allow you to hide the details of sanitize-pgp\n> in the updated test_atom so that the actual tests may not have to get\n> updated even for signed tags.\n\nAfter seeing the \"wc -c\" portability issues, I am now even more\ninclined to say that the above is the right direction. The\nportability worries can and should be encapsulated in a single\ntest_atom helper function, just as it can be used to hide the\ndifferences between signed tags, annotated tags and commits.\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"},{"id":"401232","messageId":"CAP8UFD2tUUgwjhkizihhqHc0LUYN_gS=wZCtXroLVtT3kMyqLw@mail.gmail.com","threadId":"53811","inReplyTo":"xmqqtuyhzgro.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 4/4] ref-filter: add support for %(contents:size)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-09T08:10:29Z","receivedAt":"2020-07-09T08:10:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Jul 9, 2020 at 2:14 AM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > Of course, we _could_ update the test_atom to do something magic\n> > only when the 'contents' atom is being asked. We notice that $2 is\n> > 'contents', do the usual test_expect_success for 'contents', and\n> > then measure the byte length of $3 ourselves and test\n> > 'contents:size'. That way, all the above test updates would become\n> > unnecessary (and the last two hunks of this patch can also go).\n> >\n> > That approach may even allow you to hide the details of sanitize-pgp\n> > in the updated test_atom so that the actual tests may not have to get\n> > updated even for signed tags.\n>\n> After seeing the \"wc -c\" portability issues, I am now even more\n> inclined to say that the above is the right direction. The\n> portability worries can and should be encapsulated in a single\n> test_atom helper function, just as it can be used to hide the\n> differences between signed tags, annotated tags and commits.\n\nYeah, I have been working on that and will send a new patch series soon.\nThe current test_atom() change looks like this:\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 371e45e5ad..e514d98574 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -52,6 +52,26 @@ test_atom() {\n sanitize_pgp actual.clean &&\n test_cmp expected actual.clean\n \"\n+ # Automatically test \"contents:size\" atom after testing \"contents\"\n+ if test \"$2\" = \"contents\"\n+ then\n+ case \"$1\" in\n+ refs/tags/signed-*)\n+ # We cannot use $3 as it expects sanitize_pgp to run\n+ git cat-file tag $ref | tail -n +6 | \\\n+ wc -c | sed -e 's/^ *//' >expected ;;\n+ refs/mytrees/*)\n+ echo >expected ;;\n+ refs/myblobs/*)\n+ echo >expected ;;\n+ *)\n+ printf '%s' \"$3\" | wc -c | sed -e 's/^ *//' >expected ;;\n+ esac\n+ test_expect_${4:-success} $PREREQ \"basic atom: $1 $2:size\" \"\n+ git for-each-ref --format='%($2:size)' $ref >actual &&\n+ test_cmp expected actual\n+ \"\n+ fi\n }\n\nI am wondering if it's worth adding a preparatory patch to introduce\nan helper function like the following in test-lib-functions.sh:\n\n+# test_byte_count outputs the number of bytes in files or stdin\n+#\n+# It is like wc -c but without portability issues, as on macOS and\n+# possibly other platforms leading whitespaces are emitted before the\n+# number.\n+\n+test_byte_count () {\n+ wc -c \"$@\" | sed -e 's/^ *//'\n+}\n\nNot sure about the name of this helper function as it works\ndifferently than test_line_count().\n"},{"id":"401244","messageId":"xmqq4kqgzto0.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"CAP8UFD2tUUgwjhkizihhqHc0LUYN_gS=wZCtXroLVtT3kMyqLw@mail.gmail.com","subject":"Re: [PATCH v3 4/4] ref-filter: add support for %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-09T13:47:59Z","receivedAt":"2020-07-09T13:48:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Yeah, I have been working on that and will send a new patch series soon.\n> The current test_atom() change looks like this:\n>\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 371e45e5ad..e514d98574 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -52,6 +52,26 @@ test_atom() {\n> sanitize_pgp actual.clean &&\n> test_cmp expected actual.clean\n> \"\n> + # Automatically test \"contents:size\" atom after testing \"contents\"\n> + if test \"$2\" = \"contents\"\n> + then\n> + case \"$1\" in\n> + refs/tags/signed-*)\n> + # We cannot use $3 as it expects sanitize_pgp to run\n> + git cat-file tag $ref | tail -n +6 | \\\n> + wc -c | sed -e 's/^ *//' >expected ;;\n> + refs/mytrees/*)\n> + echo >expected ;;\n> + refs/myblobs/*)\n> + echo >expected ;;\n> + *)\n> + printf '%s' \"$3\" | wc -c | sed -e 's/^ *//' >expected ;;\n> + esac\n> + test_expect_${4:-success} $PREREQ \"basic atom: $1 $2:size\" \"\n> + git for-each-ref --format='%($2:size)' $ref >actual &&\n> + test_cmp expected actual\n> + \"\n> + fi\n> }\n>\n> I am wondering if it's worth adding a preparatory patch to introduce\n> an helper function like the following in test-lib-functions.sh:\n>\n> +# test_byte_count outputs the number of bytes in files or stdin\n> +#\n> +# It is like wc -c but without portability issues, as on macOS and\n> +# possibly other platforms leading whitespaces are emitted before the\n> +# number.\n> +\n> +test_byte_count () {\n> + wc -c \"$@\" | sed -e 's/^ *//'\n> +}\n>\n> Not sure about the name of this helper function as it works\n> differently than test_line_count().\n\nYeah, if I were writing it, I'd call it \"sane_wc_c\" or something.\n\nBut more importantly, I think the invocation of \"|sed\" is overkill.\nIf I were writing it, I would go more like...\n\n\tif test $2 = contents\n\tthen\n\t\tcase \"$1\" in \n\t\t...)\n\t\t\texpect=$(git cat-file ... | wc -c)\n\t\t\t;;\n\t\trefs/mytrees/* | refs/myblobs/*)\n\t\t\texpect=0\n\t\t\t;;\n\t\t*)\n\t\t\texpect=$(printf ... | wc -c)\n\t\t\t;;\n\t\tesac\n\n\t\t# leave $expect unquoted to lose possible leading whitespaces\n\t echo $expect >expect\n\t\ttest_expect_success \"...\" '\n\t\t\t...\n\t\t\ttest_cmp expect actual\n\t\t'\n\tfi\n\n"},{"id":"401350","messageId":"CAP8UFD2vxYHvVV8nUBArCGNJTS9K1ynZ1LCBU1BBPBi9d5L77w@mail.gmail.com","threadId":"53811","inReplyTo":"xmqqsge33z42.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 1/4] Documentation: clarify %(contents:XXXX) doc","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-10T16:47:08Z","receivedAt":"2020-07-10T16:47:23Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Jul 7, 2020 at 9:26 PM Junio C Hamano wrote:\n>\n> Christian Couder writes:\n\n> > +contents:subject::\n> > + The \"subject\" of the commit or tag message. It's actually the\n> > + concatenation of all lines of the commit message up to the\n> > + first blank line.\n>\n> Let's avoid confusing readers by saying \"A is X. It's actually Y\".\n>\n> The first paragraph of the message, which typically is a single\n> line, is taken as the \"subject\" of the commit or the tag\n> message.\n\nOk.\n\n> > +contents:body::\n> > + The \"body\" of the commit or tag message. It's made of the\n> > + lines after the first blank line.\n>\n> The remainder of the commit or the tag message that follows the\n> \"subject\".\n\nOk.\n\n> > +contents:signature::\n> > + The optional GPG signature.\n>\n> I _think_ this only applies to signed tag objects and not signed\n> commit objects, but this text does not help to decide if I am\n> right.\n\nYou are right. It doesn't work for commits:\n\n---------------------------------------------------\n$ git cat-file commit refs/heads/signed_commit\ntree 9773e6a54521a5d99928685e5f62e937fc6a7593\nparent 1d1083b4c06fbb6055a2bd3d665a6d81468db5f5\nauthor Christian Couder 1594397089 +0200\ncommitter Christian Couder 1594397089 +0200\ngpgsig -----BEGIN PGP SIGNATURE-----\n\n iQGzBAABCgAdFiEElaRidyyI6IQbXM36ch8PKcZMVRkFAl8IkaEACgkQch8PKcZM\n[...]\n dkYKcRC3\n =fRMN\n -----END PGP SIGNATURE-----\n\nSigned commit\n$ git for-each-ref --format='%(contents:signature)' refs/heads/signed_commit\n\n---------------------------------------------------\n\nSo I changed the description to:\n\ncontents:signature::\n The optional GPG signature of the tag.\n"},{"id":"401351","messageId":"20200710164739.6616-1-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200707174049.21714-1-chriscool@tuxfamily.org","subject":"[PATCH v4 0/3] Add support for %(contents:size) in ref-filter","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-10T16:47:36Z","receivedAt":"2020-07-10T16:48:00Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This is version 4 of a small patch series to teach ref-filter about\n%(contents:size).\n\nThis patch series is based on master at 4a0fcf9f76 (The seventh batch,\n2020-07-06).\n\nPrevious versions and related discussions are there:\n\nV1: https://lore.kernel.org/git/20200701132308.16691-1-chriscool@tuxfamily.org/\nV2: https://lore.kernel.org/git/20200702140845.24945-1-chriscool@tuxfamily.org/\nV3: https://lore.kernel.org/git/20200707174049.21714-1-chriscool@tuxfamily.org/\n\nThanks to Junio and Peff for their reviews of this series!\n\nThe changes compared to V3 are the following:\n\n - Squashed patches 1/4 and 2/4 into 1/3 as they were both about\n %(contents) related doc improvements.\n\n - Improved patch 1/3 as suggested by Junio.\n\n - Simplified setup test in patch 2/3 about creating a ref pointing\n to a blob as suggested by Junio.\n\n - Modified test_atom() in patch 3/3 to automatically test\n %(contents:size) after testing %(contents) as suggested by Junio.\n\nThe range diff is:\n\n1: b04b390f32 ! 1: f750832fc7 Documentation: clarify %(contents:XXXX) doc\n @@ Commit message\n Let's avoid a big dense paragraph by using an unordered\n list for the %(contents:XXXX) format specifiers.\n \n + While at it let's also make the following improvements:\n +\n + - Let's not describe %(contents) using \"complete message\"\n + as it's not clear what an incomplete message is.\n +\n + - Let's improve how the \"subject\" and \"body\" are\n + described.\n +\n + - Let's state that \"signature\" is only available for\n + tag objects.\n +\n Suggested-by: Jeff King \n Signed-off-by: Christian Couder \n \n @@ Documentation/git-for-each-ref.txt: Fields that have name-email-date tuple as it\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 ++The message in a commit or a tag object is `contents`, from which\n ++`contents:` can be used to extract various parts out of:\n +\n +contents:subject::\n -+ The \"subject\" of the commit or tag message. It's actually the\n -+ concatenation of all lines of the commit message up to the\n -+ first blank line.\n ++ The first paragraph of the message, which typically is a\n ++ single line, is taken as the \"subject\" of the commit or the\n ++ tag message.\n +\n +contents:body::\n -+ The \"body\" of the commit or tag message. It's made of the\n -+ lines after the first blank line.\n ++ The remainder of the commit or the tag message that follows\n ++ the \"subject\".\n +\n +contents:signature::\n -+ The optional GPG signature.\n ++ The optional GPG signature of the tag.\n +\n +contents:lines=N::\n + The first `N` lines of the message.\n2: b62cab2630 < -: ---------- Documentation: clarify 'complete message'\n3: b9584472a1 ! 2: 51c72e09d2 t6300: test refs pointing to tree and blob\n @@ t/t6300-for-each-ref.sh: test_atom refs/tags/signed-long contents \"subject line\n \n +test_expect_success 'set up refs pointing to tree and blob' '\n + git update-ref refs/mytrees/first refs/heads/master^{tree} &&\n -+ git ls-tree refs/mytrees/first one >one_info &&\n -+ test $(cut -d\" \" -f2 one_info) = \"blob\" &&\n -+ blob_hash=$(cut \"-d \" -f1 one_info | cut -d\" \" -f3) &&\n -+ git update-ref refs/myblobs/first \"$blob_hash\"\n ++ git update-ref refs/myblobs/first refs/heads/master:one\n +'\n +\n +test_atom refs/mytrees/first subject \"\"\n4: 23f941132e < -: ---------- ref-filter: add support for %(contents:size)\n-: ---------- > 3: c2ed3e228b ref-filter: add support for %(contents:size)\n\nChristian Couder (3):\n Documentation: clarify %(contents:XXXX) doc\n t6300: test refs pointing to tree and blob\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 | 38 ++++++++++++++++++++++++++++++\n 3 files changed, 65 insertions(+), 7 deletions(-)\n\n-- \n2.27.0.347.gb620d8b0da\n\n"},{"id":"401352","messageId":"20200710164739.6616-2-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200710164739.6616-1-chriscool@tuxfamily.org","subject":"[PATCH v4 1/3] Documentation: clarify %(contents:XXXX) doc","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-10T16:47:37Z","receivedAt":"2020-07-10T16:48:02Z","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\nWhile at it let's also make the following improvements:\n\n - Let's not describe %(contents) using \"complete message\"\n as it's not clear what an incomplete message is.\n\n - Let's improve how the \"subject\" and \"body\" are\n described.\n\n - Let's state that \"signature\" is only available for\n tag objects.\n\nSuggested-by: Jeff King \nSigned-off-by: Christian Couder \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..b739412c30 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 message in a commit or a tag object is `contents`, from which\n+`contents:` can be used to extract various parts out of:\n+\n+contents:subject::\n+\tThe first paragraph of the message, which typically is a\n+\tsingle line, is taken as the \"subject\" of the commit or the\n+\ttag message.\n+\n+contents:body::\n+\tThe remainder of the commit or the tag message that follows\n+\tthe \"subject\".\n+\n+contents:signature::\n+\tThe optional GPG signature of the tag.\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.347.gb620d8b0da\n\n"},{"id":"401353","messageId":"20200710164739.6616-4-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200710164739.6616-1-chriscool@tuxfamily.org","subject":"[PATCH v4 3/3] ref-filter: add support for %(contents:size)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-10T16:47:39Z","receivedAt":"2020-07-10T16:48:03Z","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)' refs/heads/my-branch | 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 output\nwith the output from `git cat-file`.\n\nAs with %(contents), %(contents:size) is silently ignored, if a\nref points to something other than a commit or a tag:\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\nSigned-off-by: Christian Couder \n---\n Documentation/git-for-each-ref.txt | 3 +++\n ref-filter.c | 7 ++++++-\n t/t6300-for-each-ref.sh | 19 +++++++++++++++++++\n 3 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex b739412c30..2ea71c5f6c 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 message in a commit or a tag object is `contents`, from which\n `contents:` can be used to extract various parts out of:\n \n+contents:size::\n+\tThe size in bytes of the commit or tag message.\n+\n contents:subject::\n \tThe first paragraph of the message, which typically is a\n \tsingle line, is taken as the \"subject\" of the commit or the\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8447cb09be..73d8bfa86d 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(\"%\"PRIuMAX, (uintmax_t)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 e9f468d360..467871ac10 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -52,6 +52,25 @@ test_atom() {\n \t\tsanitize_pgp actual.clean &&\n \t\ttest_cmp expected actual.clean\n \t\"\n+\t# Automatically test \"contents:size\" atom after testing \"contents\"\n+\tif test \"$2\" = \"contents\"\n+\tthen\n+\t\tcase \"$1\" in\n+\t\trefs/tags/signed-*)\n+\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n+\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n+\t\trefs/mytrees/* | refs/myblobs/*)\n+\t\t\texpect='' ;;\n+\t\t*)\n+\t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n+\t\tesac\n+\t\t# Leave $expect unquoted to lose possible leading whitespaces\n+\t\techo $expect >expected\n+\t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 $2:size\" \"\n+\t\t\tgit for-each-ref --format='%($2:size)' $ref >actual &&\n+\t\t\ttest_cmp expected actual\n+\t\t\"\n+\tfi\n }\n \n hexlen=$(test_oid hexsz)\n-- \n2.27.0.347.gb620d8b0da\n\n"},{"id":"401354","messageId":"20200710164739.6616-3-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200710164739.6616-1-chriscool@tuxfamily.org","subject":"[PATCH v4 2/3] t6300: test refs pointing to tree and blob","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-10T16:47:38Z","receivedAt":"2020-07-10T16:48:05Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Adding tests for refs pointing to tree and blob shows that\nwe care about testing both positive (\"see, my shiny new toy\ndoes work\") and negative (\"and it won't do nonsensical\nthings when given an input it is not designed to work with\")\ncases.\n\nSigned-off-by: Christian Couder \n---\n t/t6300-for-each-ref.sh | 19 +++++++++++++++++++\n 1 file changed, 19 insertions(+)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex da59fadc5d..e9f468d360 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -650,6 +650,25 @@ test_atom refs/tags/signed-long contents \"subject line\n body contents\n $sig\"\n \n+test_expect_success 'set up refs pointing to tree and blob' '\n+\tgit update-ref refs/mytrees/first refs/heads/master^{tree} &&\n+\tgit update-ref refs/myblobs/first refs/heads/master:one\n+'\n+\n+test_atom refs/mytrees/first subject \"\"\n+test_atom refs/mytrees/first contents:subject \"\"\n+test_atom refs/mytrees/first body \"\"\n+test_atom refs/mytrees/first contents:body \"\"\n+test_atom refs/mytrees/first contents:signature \"\"\n+test_atom refs/mytrees/first contents \"\"\n+\n+test_atom refs/myblobs/first subject \"\"\n+test_atom refs/myblobs/first contents:subject \"\"\n+test_atom refs/myblobs/first body \"\"\n+test_atom refs/myblobs/first contents:body \"\"\n+test_atom refs/myblobs/first contents:signature \"\"\n+test_atom refs/myblobs/first contents \"\"\n+\n test_expect_success 'set up multiple-sort tags' '\n \tfor when in 100000 200000\n \tdo\n-- \n2.27.0.347.gb620d8b0da\n\n"},{"id":"401377","messageId":"xmqqblknt8yd.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200710164739.6616-2-chriscool@tuxfamily.org","subject":"Re: [PATCH v4 1/3] Documentation: clarify %(contents:XXXX) doc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-10T20:24:10Z","receivedAt":"2020-07-10T20:24:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Let's avoid a big dense paragraph by using an unordered\n> list for the %(contents:XXXX) format specifiers.\n>\n> While at it let's also make the following improvements:\n>\n> - Let's not describe %(contents) using \"complete message\"\n> as it's not clear what an incomplete message is.\n>\n> - Let's improve how the \"subject\" and \"body\" are\n> described.\n>\n> - Let's state that \"signature\" is only available for\n> tag objects.\n>\n> Suggested-by: Jeff King \n> Signed-off-by: Christian Couder \n> ---\n> Documentation/git-for-each-ref.txt | 24 ++++++++++++++++++------\n> 1 file changed, 18 insertions(+), 6 deletions(-)\n\nLooking good. Thanks.\n\n\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index 6dcd39f6f6..b739412c30 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 message in a commit or a tag object is `contents`, from which\n> +`contents:` can be used to extract various parts out of:\n> +\n> +contents:subject::\n> +\tThe first paragraph of the message, which typically is a\n> +\tsingle line, is taken as the \"subject\" of the commit or the\n> +\ttag message.\n> +\n> +contents:body::\n> +\tThe remainder of the commit or the tag message that follows\n> +\tthe \"subject\".\n> +\n> +contents:signature::\n> +\tThe optional GPG signature of the tag.\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":"401378","messageId":"xmqq7dvbt8xx.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200710164739.6616-3-chriscool@tuxfamily.org","subject":"Re: [PATCH v4 2/3] t6300: test refs pointing to tree and blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-10T20:24:26Z","receivedAt":"2020-07-10T20:24:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Adding tests for refs pointing to tree and blob shows that\n> we care about testing both positive (\"see, my shiny new toy\n> does work\") and negative (\"and it won't do nonsensical\n> things when given an input it is not designed to work with\")\n> cases.\n>\n> Signed-off-by: Christian Couder \n> ---\n> t/t6300-for-each-ref.sh | 19 +++++++++++++++++++\n> 1 file changed, 19 insertions(+)\n\nNice addition. Thanks.\n\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index da59fadc5d..e9f468d360 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -650,6 +650,25 @@ test_atom refs/tags/signed-long contents \"subject line\n> body contents\n> $sig\"\n> \n> +test_expect_success 'set up refs pointing to tree and blob' '\n> +\tgit update-ref refs/mytrees/first refs/heads/master^{tree} &&\n> +\tgit update-ref refs/myblobs/first refs/heads/master:one\n> +'\n> +\n> +test_atom refs/mytrees/first subject \"\"\n> +test_atom refs/mytrees/first contents:subject \"\"\n> +test_atom refs/mytrees/first body \"\"\n> +test_atom refs/mytrees/first contents:body \"\"\n> +test_atom refs/mytrees/first contents:signature \"\"\n> +test_atom refs/mytrees/first contents \"\"\n> +\n> +test_atom refs/myblobs/first subject \"\"\n> +test_atom refs/myblobs/first contents:subject \"\"\n> +test_atom refs/myblobs/first body \"\"\n> +test_atom refs/myblobs/first contents:body \"\"\n> +test_atom refs/myblobs/first contents:signature \"\"\n> +test_atom refs/myblobs/first contents \"\"\n> +\n> test_expect_success 'set up multiple-sort tags' '\n> \tfor when in 100000 200000\n> \tdo\n"},{"id":"401380","messageId":"xmqq365zt8at.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200710164739.6616-4-chriscool@tuxfamily.org","subject":"Re: [PATCH v4 3/3] ref-filter: add support for %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-10T20:38:18Z","receivedAt":"2020-07-10T20:38:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder 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)' refs/heads/my-branch | 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 output\n> with the output from `git cat-file`.\n>\n> As with %(contents), %(contents:size) is silently ignored, if a\n> ref points to something other than a commit or a tag:\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>\n> Signed-off-by: Christian Couder \n> ---\n> Documentation/git-for-each-ref.txt | 3 +++\n> ref-filter.c | 7 ++++++-\n> t/t6300-for-each-ref.sh | 19 +++++++++++++++++++\n> 3 files changed, 28 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index b739412c30..2ea71c5f6c 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 message in a commit or a tag object is `contents`, from which\n> `contents:` can be used to extract various parts out of:\n> \n> +contents:size::\n> +\tThe size in bytes of the commit or tag message.\n> +\n> contents:subject::\n> \tThe first paragraph of the message, which typically is a\n> \tsingle line, is taken as the \"subject\" of the commit or the\n\nOK.\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 8447cb09be..73d8bfa86d 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(\"%\"PRIuMAX, (uintmax_t)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)\n\nOK.\n\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index e9f468d360..467871ac10 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -52,6 +52,25 @@ test_atom() {\n\nYou need to stare at the precontext to see if the added lines are\ncorrect. We have these before the precontext of the patch:\n\n\tcase \"$1\" in\n\t\thead) ref=refs/heads/master ;;\n\t\t tag) ref=refs/tags/testtag ;;\n\t\t sym) ref=refs/heads/sym ;;\n\t\t *) ref=$1 ;;\n\tesac\n\tprintf '%s\\n' \"$3\" >expected\n\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 $2\" \"\n\t\tgit for-each-ref --format='%($2)' $ref >actual &&\n\nHere it uses \"$1\" for mere reporting on the test title, while using\n\"$ref\" as the reliable way to uniquely identify it as a full ref.\n\n> \t\tsanitize_pgp actual.clean &&\n> \t\ttest_cmp expected actual.clean\n> \t\"\n> +\t# Automatically test \"contents:size\" atom after testing \"contents\"\n> +\tif test \"$2\" = \"contents\"\n> +\tthen\n> +\t\tcase \"$1\" in\n> +\t\trefs/tags/signed-*)\n\nShouldn't this be $ref to be compared with full refnames like we see\nbelow?\n\nI know the callers won't pass 'head', 'tag' and 'sym' with\n'contents' to this helper so the distinction may not currently\nmatter in practice, but still this use of \"$1\" does not sound quite\nright, no? \n\nI actually was expecting you to switch on\n\n\tcase $(git cat-file -t \"$ref\") in\n\ttag)\n\t\t...;;\n\ttree | blob)\n\t\t...;;\n\tcommit)\n\t\t...;;\n\teasc\n\ninstead of the namespace, as %(contents:size) silently becomes empty\ndue to the underlying object type, not where the object that does\nnot support the \"method\" sits in the refs/ namespace.\n\n> +\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n> +\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n> +\t\trefs/mytrees/* | refs/myblobs/*)\n> +\t\t\texpect='' ;;\n\nThanks for catching my thinko; I think I wrote 0 here in my\nillustration.\n\n> +\t\t*)\n> +\t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n> +\t\tesac\n> +\t\t# Leave $expect unquoted to lose possible leading whitespaces\n> +\t\techo $expect >expected\n\nOK.\n\n> +\t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 $2:size\" \"\n> +\t\t\tgit for-each-ref --format='%($2:size)' $ref >actual &&\n> +\t\t\ttest_cmp expected actual\n> +\t\t\"\n\nThis is harder to read than necessary; let's not say \"$2\" when we\nknow it is 'contents' and nothing else. Also avoid double-quoted\ntest body when you can. The body is evaled and $ref we assigned is\nvisible inside the test just fine, so make it a habit to quote the\nbody with single quote pair, i.e.\n\n\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n\t\ttest_cmp expect actual\n\t'\n\nThanks.\n\n> +\tfi\n> }\n> \n> hexlen=$(test_oid hexsz)\n"},{"id":"401626","messageId":"20200716121940.21041-1-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200710164739.6616-1-chriscool@tuxfamily.org","subject":"[PATCH v5 0/3] Add support for %(contents:size) in ref-filter","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-16T12:19:37Z","receivedAt":"2020-07-16T12:19:57Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This is version 5 of a small patch series to teach ref-filter about\n%(contents:size).\n\nThis patch series is based on master at 4a0fcf9f76 (The seventh batch,\n2020-07-06).\n\nPrevious versions and related discussions are there:\n\nV1: https://lore.kernel.org/git/20200701132308.16691-1-chriscool@tuxfamily.org/\nV2: https://lore.kernel.org/git/20200702140845.24945-1-chriscool@tuxfamily.org/\nV3: https://lore.kernel.org/git/20200707174049.21714-1-chriscool@tuxfamily.org/\nV4: https://lore.kernel.org/git/20200710164739.6616-1-chriscool@tuxfamily.org/\n\nThanks to Junio and Peff for their reviews of this series!\n\nThe changes compared to V4 are the following:\n\n - Modified test_atom() in patch 3/3 to as suggested by Junio.\n\nThe range diff is:\n\n1: f750832fc7 = 1: f750832fc7 Documentation: clarify %(contents:XXXX) doc\n2: 51c72e09d2 = 2: 51c72e09d2 t6300: test refs pointing to tree and blob\n3: c2ed3e228b ! 3: cf6a60036e ref-filter: add support for %(contents:size)\n @@ t/t6300-for-each-ref.sh: test_atom() {\n + # Automatically test \"contents:size\" atom after testing \"contents\"\n + if test \"$2\" = \"contents\"\n + then\n -+ case \"$1\" in\n -+ refs/tags/signed-*)\n ++ case $(git cat-file -t \"$ref\") in\n ++ tag)\n + # We cannot use $3 as it expects sanitize_pgp to run\n + expect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n -+ refs/mytrees/* | refs/myblobs/*)\n ++ tree | blob)\n + expect='' ;;\n -+ *)\n ++ commit)\n + expect=$(printf '%s' \"$3\" | wc -c) ;;\n + esac\n + # Leave $expect unquoted to lose possible leading whitespaces\n + echo $expect >expected\n -+ test_expect_${4:-success} $PREREQ \"basic atom: $1 $2:size\" \"\n -+ git for-each-ref --format='%($2:size)' $ref >actual &&\n -+ test_cmp expected actual\n -+ \"\n ++ test_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n ++ git for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n ++ test_cmp expect actual\n ++ '\n + fi\n }\n \nChristian Couder (3):\n Documentation: clarify %(contents:XXXX) doc\n t6300: test refs pointing to tree and blob\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 | 38 ++++++++++++++++++++++++++++++\n 3 files changed, 65 insertions(+), 7 deletions(-)\n\n-- \n2.27.0.227.g757ac19d14.dirty\n\n"},{"id":"401627","messageId":"20200716121940.21041-2-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200716121940.21041-1-chriscool@tuxfamily.org","subject":"[PATCH v5 1/3] Documentation: clarify %(contents:XXXX) doc","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-16T12:19:38Z","receivedAt":"2020-07-16T12:19:59Z","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\nWhile at it let's also make the following improvements:\n\n - Let's not describe %(contents) using \"complete message\"\n as it's not clear what an incomplete message is.\n\n - Let's improve how the \"subject\" and \"body\" are\n described.\n\n - Let's state that \"signature\" is only available for\n tag objects.\n\nSuggested-by: Jeff King \nSigned-off-by: Christian Couder \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..b739412c30 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 message in a commit or a tag object is `contents`, from which\n+`contents:` can be used to extract various parts out of:\n+\n+contents:subject::\n+\tThe first paragraph of the message, which typically is a\n+\tsingle line, is taken as the \"subject\" of the commit or the\n+\ttag message.\n+\n+contents:body::\n+\tThe remainder of the commit or the tag message that follows\n+\tthe \"subject\".\n+\n+contents:signature::\n+\tThe optional GPG signature of the tag.\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.227.g757ac19d14.dirty\n\n"},{"id":"401628","messageId":"20200716121940.21041-3-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200716121940.21041-1-chriscool@tuxfamily.org","subject":"[PATCH v5 2/3] t6300: test refs pointing to tree and blob","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-16T12:19:39Z","receivedAt":"2020-07-16T12:20:00Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Adding tests for refs pointing to tree and blob shows that\nwe care about testing both positive (\"see, my shiny new toy\ndoes work\") and negative (\"and it won't do nonsensical\nthings when given an input it is not designed to work with\")\ncases.\n\nSigned-off-by: Christian Couder \n---\n t/t6300-for-each-ref.sh | 19 +++++++++++++++++++\n 1 file changed, 19 insertions(+)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex da59fadc5d..e9f468d360 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -650,6 +650,25 @@ test_atom refs/tags/signed-long contents \"subject line\n body contents\n $sig\"\n \n+test_expect_success 'set up refs pointing to tree and blob' '\n+\tgit update-ref refs/mytrees/first refs/heads/master^{tree} &&\n+\tgit update-ref refs/myblobs/first refs/heads/master:one\n+'\n+\n+test_atom refs/mytrees/first subject \"\"\n+test_atom refs/mytrees/first contents:subject \"\"\n+test_atom refs/mytrees/first body \"\"\n+test_atom refs/mytrees/first contents:body \"\"\n+test_atom refs/mytrees/first contents:signature \"\"\n+test_atom refs/mytrees/first contents \"\"\n+\n+test_atom refs/myblobs/first subject \"\"\n+test_atom refs/myblobs/first contents:subject \"\"\n+test_atom refs/myblobs/first body \"\"\n+test_atom refs/myblobs/first contents:body \"\"\n+test_atom refs/myblobs/first contents:signature \"\"\n+test_atom refs/myblobs/first contents \"\"\n+\n test_expect_success 'set up multiple-sort tags' '\n \tfor when in 100000 200000\n \tdo\n-- \n2.27.0.227.g757ac19d14.dirty\n\n"},{"id":"401629","messageId":"20200716121940.21041-4-chriscool@tuxfamily.org","threadId":"53811","inReplyTo":"20200716121940.21041-1-chriscool@tuxfamily.org","subject":"[PATCH v5 3/3] ref-filter: add support for %(contents:size)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-16T12:19:40Z","receivedAt":"2020-07-16T12:20:02Z","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)' refs/heads/my-branch | 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 output\nwith the output from `git cat-file`.\n\nAs with %(contents), %(contents:size) is silently ignored, if a\nref points to something other than a commit or a tag:\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\nSigned-off-by: Christian Couder \n---\n Documentation/git-for-each-ref.txt | 3 +++\n ref-filter.c | 7 ++++++-\n t/t6300-for-each-ref.sh | 19 +++++++++++++++++++\n 3 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex b739412c30..2ea71c5f6c 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 message in a commit or a tag object is `contents`, from which\n `contents:` can be used to extract various parts out of:\n \n+contents:size::\n+\tThe size in bytes of the commit or tag message.\n+\n contents:subject::\n \tThe first paragraph of the message, which typically is a\n \tsingle line, is taken as the \"subject\" of the commit or the\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8447cb09be..73d8bfa86d 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(\"%\"PRIuMAX, (uintmax_t)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 e9f468d360..ea9bb6dade 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -52,6 +52,25 @@ test_atom() {\n \t\tsanitize_pgp actual.clean &&\n \t\ttest_cmp expected actual.clean\n \t\"\n+\t# Automatically test \"contents:size\" atom after testing \"contents\"\n+\tif test \"$2\" = \"contents\"\n+\tthen\n+\t\tcase $(git cat-file -t \"$ref\") in\n+\t\ttag)\n+\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n+\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n+\t\ttree | blob)\n+\t\t\texpect='' ;;\n+\t\tcommit)\n+\t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n+\t\tesac\n+\t\t# Leave $expect unquoted to lose possible leading whitespaces\n+\t\techo $expect >expected\n+\t\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n+\t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n+\t\t\ttest_cmp expect actual\n+\t\t'\n+\tfi\n }\n \n hexlen=$(test_oid hexsz)\n-- \n2.27.0.227.g757ac19d14.dirty\n\n"},{"id":"401657","messageId":"xmqq8sfjgxkl.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200716121940.21041-1-chriscool@tuxfamily.org","subject":"Re: [PATCH v5 0/3] Add support for %(contents:size) in ref-filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-16T17:48:58Z","receivedAt":"2020-07-16T17:49:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> The range diff is:\n>\n> 1: f750832fc7 = 1: f750832fc7 Documentation: clarify %(contents:XXXX) doc\n> 2: 51c72e09d2 = 2: 51c72e09d2 t6300: test refs pointing to tree and blob\n> 3: c2ed3e228b ! 3: cf6a60036e ref-filter: add support for %(contents:size)\n> @@ t/t6300-for-each-ref.sh: test_atom() {\n> + # Automatically test \"contents:size\" atom after testing \"contents\"\n> + if test \"$2\" = \"contents\"\n> + then\n> -+ case \"$1\" in\n> -+ refs/tags/signed-*)\n> ++ case $(git cat-file -t \"$ref\") in\n> ++ tag)\n> + # We cannot use $3 as it expects sanitize_pgp to run\n> + expect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n> -+ refs/mytrees/* | refs/myblobs/*)\n> ++ tree | blob)\n> + expect='' ;;\n> -+ *)\n> ++ commit)\n> + expect=$(printf '%s' \"$3\" | wc -c) ;;\n> + esac\n> + # Leave $expect unquoted to lose possible leading whitespaces\n> + echo $expect >expected\n> -+ test_expect_${4:-success} $PREREQ \"basic atom: $1 $2:size\" \"\n> -+ git for-each-ref --format='%($2:size)' $ref >actual &&\n> -+ test_cmp expected actual\n> -+ \"\n> ++ test_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n> ++ git for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n> ++ test_cmp expect actual\n> ++ '\n> + fi\n> }\n\nAh, I almost forgot about this topic X-<, but the above reminds me\nand it does read more clearly, at least to me.\n\nThanks, will replace.\n"},{"id":"402564","messageId":"21bb2dad-5845-8cee-8f6a-1089ef7cae3b@gmail.com","threadId":"53811","inReplyTo":"20200716121940.21041-4-chriscool@tuxfamily.org","subject":"Re: [PATCH v5 3/3] ref-filter: add support for %(contents:size)","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2020-07-31T17:37:22Z","receivedAt":"2020-07-31T17:37:27Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Hi Christian,\n\nLe 16/07/2020 à 14:19, Christian Couder a écrit :\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)' refs/heads/my-branch | 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 output\n> with the output from `git cat-file`.\n> \n> As with %(contents), %(contents:size) is silently ignored, if a\n> ref points to something other than a commit or a tag:\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> \n> Signed-off-by: Christian Couder \n> ---\n> Documentation/git-for-each-ref.txt | 3 +++\n> ref-filter.c | 7 ++++++-\n> t/t6300-for-each-ref.sh | 19 +++++++++++++++++++\n> 3 files changed, 28 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index b739412c30..2ea71c5f6c 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 message in a commit or a tag object is `contents`, from which\n> `contents:` can be used to extract various parts out of:\n> \n> +contents:size::\n> +\tThe size in bytes of the commit or tag message.\n> +\n> contents:subject::\n> \tThe first paragraph of the message, which typically is a\n> \tsingle line, is taken as the \"subject\" of the commit or the\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 8447cb09be..73d8bfa86d 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(\"%\"PRIuMAX, (uintmax_t)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)\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index e9f468d360..ea9bb6dade 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -52,6 +52,25 @@ test_atom() {\n> \t\tsanitize_pgp actual.clean &&\n> \t\ttest_cmp expected actual.clean\n> \t\"\n> +\t# Automatically test \"contents:size\" atom after testing \"contents\"\n> +\tif test \"$2\" = \"contents\"\n> +\tthen\n> +\t\tcase $(git cat-file -t \"$ref\") in\n> +\t\ttag)\n> +\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n> +\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n> +\t\ttree | blob)\n> +\t\t\texpect='' ;;\n> +\t\tcommit)\n> +\t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n> +\t\tesac\n> +\t\t# Leave $expect unquoted to lose possible leading whitespaces\n> +\t\techo $expect >expected\n> +\t\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n\nThere is a typo here, and $expect is written to `expected', but\n`test_cmp' wants `expect'. Fixing those mistakes does not reveal any\nbroken tests.\n\n> +\t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n> +\t\t\ttest_cmp expect actual\n> +\t\t'\n> +\tfi\n> }\n> \n> hexlen=$(test_oid hexsz)\n> \n\nAlban\n\n"},{"id":"402566","messageId":"20200731174509.9199-1-alban.gruin@gmail.com","threadId":"53811","inReplyTo":"21bb2dad-5845-8cee-8f6a-1089ef7cae3b@gmail.com","subject":"[PATCH v1] t6300: fix issues related to %(contents:size)","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2020-07-31T17:45:09Z","receivedAt":"2020-07-31T17:45:21Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"b6839fda68 (ref-filter: add support for %(contents:size), 2020-07-16)\nadded a new format for ref-filter, and added a function to generate\ntests for this new feature in t6300. Unfortunately, it tries to run\n`test_expect_sucess' instead of `test_expect_success', and writes\n$expect to `expected', but tries to read `expect'. Those two issues\nwere probably unnoticed because the script only printed errors, but did\nnot crash. This fixes these issues.\n\nSigned-off-by: Alban Gruin \n---\n t/t6300-for-each-ref.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex ea9bb6dade..bbec555977 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -65,8 +65,8 @@ test_atom() {\n \t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n \t\tesac\n \t\t# Leave $expect unquoted to lose possible leading whitespaces\n-\t\techo $expect >expected\n-\t\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n+\t\techo $expect >expect\n+\t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n \t\t\ttest_cmp expect actual\n \t\t'\n-- \n2.20.1\n\n"},{"id":"402567","messageId":"20200731174547.GC843002@coredump.intra.peff.net","threadId":"53811","inReplyTo":"21bb2dad-5845-8cee-8f6a-1089ef7cae3b@gmail.com","subject":"Re: [PATCH v5 3/3] ref-filter: add support for %(contents:size)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-31T17:45:47Z","receivedAt":"2020-07-31T17:45:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 31, 2020 at 07:37:22PM +0200, Alban Gruin wrote:\n\n> > +\t\t# Leave $expect unquoted to lose possible leading whitespaces\n> > +\t\techo $expect >expected\n> > +\t\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n> \n> There is a typo here, and $expect is written to `expected', but\n> `test_cmp' wants `expect'. Fixing those mistakes does not reveal any\n> broken tests.\n\nI thought at first you meant that the typo was s/expected/expect, and\nwondered how this could possibly have passed. But the typo is\ns/sucess/success/, so we were in fact not running the test at all (and\nwere generating \"test_expect_sucess: not found\" messages to stderr, but\noutside of any test block. Yikes.\n\nThanks for spotting.\n\n-Peff\n"},{"id":"402568","messageId":"20200731174709.GD843002@coredump.intra.peff.net","threadId":"53811","inReplyTo":"20200731174509.9199-1-alban.gruin@gmail.com","subject":"Re: [PATCH v1] t6300: fix issues related to %(contents:size)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-31T17:47:09Z","receivedAt":"2020-07-31T17:47:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 31, 2020 at 07:45:09PM +0200, Alban Gruin wrote:\n\n> b6839fda68 (ref-filter: add support for %(contents:size), 2020-07-16)\n> added a new format for ref-filter, and added a function to generate\n> tests for this new feature in t6300. Unfortunately, it tries to run\n> `test_expect_sucess' instead of `test_expect_success', and writes\n> $expect to `expected', but tries to read `expect'. Those two issues\n> were probably unnoticed because the script only printed errors, but did\n> not crash. This fixes these issues.\n\nOh, this just crossed with my mail. :)\n\nDefinitely fixes the issue, though I wonder:\n\n> -\t\techo $expect >expected\n> -\t\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n> +\t\techo $expect >expect\n> +\t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n> \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n> \t\t\ttest_cmp expect actual\n> \t\t'\n\nShould we instead switch the test_cmp to look at \"expected\" to be\nconsistent with the rest of the tests in this file?\n\n-Peff\n"},{"id":"402573","messageId":"e81ef39d-da2a-6263-787f-a8fe367b98e3@gmail.com","threadId":"53811","inReplyTo":"20200731174709.GD843002@coredump.intra.peff.net","subject":"Re: [PATCH v1] t6300: fix issues related to %(contents:size)","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2020-07-31T18:24:14Z","receivedAt":"2020-07-31T18:24:19Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Le 31/07/2020 à 19:47, Jeff King a écrit :\n> On Fri, Jul 31, 2020 at 07:45:09PM +0200, Alban Gruin wrote:\n> \n>> b6839fda68 (ref-filter: add support for %(contents:size), 2020-07-16)\n>> added a new format for ref-filter, and added a function to generate\n>> tests for this new feature in t6300. Unfortunately, it tries to run\n>> `test_expect_sucess' instead of `test_expect_success', and writes\n>> $expect to `expected', but tries to read `expect'. Those two issues\n>> were probably unnoticed because the script only printed errors, but did\n>> not crash. This fixes these issues.\n> \n> Oh, this just crossed with my mail. :)\n> \n> Definitely fixes the issue, though I wonder:\n> \n>> -\t\techo $expect >expected\n>> -\t\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n>> +\t\techo $expect >expect\n>> +\t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n>> \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n>> \t\t\ttest_cmp expect actual\n>> \t\t'\n> \n> Should we instead switch the test_cmp to look at \"expected\" to be\n> consistent with the rest of the tests in this file?\n> \n> -Peff\n> \n\nOK, I'm fixing that.\n\nAlban\n\n"},{"id":"402574","messageId":"20200731182607.15532-1-alban.gruin@gmail.com","threadId":"53811","inReplyTo":"20200731174509.9199-1-alban.gruin@gmail.com","subject":"[PATCH v2] t6300: fix issues related to %(contents:size)","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2020-07-31T18:26:07Z","receivedAt":"2020-07-31T18:26:23Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"b6839fda68 (ref-filter: add support for %(contents:size), 2020-07-16)\nadded a new format for ref-filter, and added a function to generate\ntests for this new feature in t6300. Unfortunately, it tries to run\n`test_expect_sucess' instead of `test_expect_success', and writes\n$expect to `expected', but tries to read `expect'. Those two issues\nwere probably unnoticed because the script only printed errors, but did\nnot crash. This fixes these issues.\n\nSigned-off-by: Alban Gruin \n---\n t/t6300-for-each-ref.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex ea9bb6dade..a83579fbdf 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -66,9 +66,9 @@ test_atom() {\n \t\tesac\n \t\t# Leave $expect unquoted to lose possible leading whitespaces\n \t\techo $expect >expected\n-\t\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n+\t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n-\t\t\ttest_cmp expect actual\n+\t\t\ttest_cmp expected actual\n \t\t'\n \tfi\n }\n-- \n2.20.1\n\n"},{"id":"402578","messageId":"20200731191532.GB848793@coredump.intra.peff.net","threadId":"53811","inReplyTo":"20200731182607.15532-1-alban.gruin@gmail.com","subject":"Re: [PATCH v2] t6300: fix issues related to %(contents:size)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-31T19:15:32Z","receivedAt":"2020-07-31T19:15:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 31, 2020 at 08:26:07PM +0200, Alban Gruin wrote:\n\n> b6839fda68 (ref-filter: add support for %(contents:size), 2020-07-16)\n> added a new format for ref-filter, and added a function to generate\n> tests for this new feature in t6300. Unfortunately, it tries to run\n> `test_expect_sucess' instead of `test_expect_success', and writes\n> $expect to `expected', but tries to read `expect'. Those two issues\n> were probably unnoticed because the script only printed errors, but did\n> not crash. This fixes these issues.\n> \n> Signed-off-by: Alban Gruin \n> ---\n> t/t6300-for-each-ref.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n\nLooks good. Good eyes for spotting this, and thanks for the quick fix.\n\n-Peff\n"},{"id":"402584","messageId":"xmqqpn8b30zp.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"20200731174709.GD843002@coredump.intra.peff.net","subject":"Re: [PATCH v1] t6300: fix issues related to %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-31T20:04:10Z","receivedAt":"2020-07-31T20:04:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Jul 31, 2020 at 07:45:09PM +0200, Alban Gruin wrote:\n>\n>> b6839fda68 (ref-filter: add support for %(contents:size), 2020-07-16)\n>> added a new format for ref-filter, and added a function to generate\n>> tests for this new feature in t6300. Unfortunately, it tries to run\n>> `test_expect_sucess' instead of `test_expect_success', and writes\n>> $expect to `expected', but tries to read `expect'. Those two issues\n>> were probably unnoticed because the script only printed errors, but did\n>> not crash. This fixes these issues.\n>\n> Oh, this just crossed with my mail. :)\n>\n> Definitely fixes the issue, though I wonder:\n>\n>> -\t\techo $expect >expected\n>> -\t\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n>> +\t\techo $expect >expect\n>> +\t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n>> \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n>> \t\t\ttest_cmp expect actual\n>> \t\t'\n>\n> Should we instead switch the test_cmp to look at \"expected\" to be\n> consistent with the rest of the tests in this file?\n\nIf I recall correctly, \"expect vs actual\" were more common when I\ncounted across all the tests last time. Matching local convention\nis fine, though.\n"},{"id":"402586","messageId":"CAP8UFD15+p+xKwJ=B9WVsrc+2TvLHKmu78SBCLUFZVSYoTtbbg@mail.gmail.com","threadId":"53811","inReplyTo":"20200731174547.GC843002@coredump.intra.peff.net","subject":"Re: [PATCH v5 3/3] ref-filter: add support for %(contents:size)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-07-31T20:12:13Z","receivedAt":"2020-07-31T20:12:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi Alban and Peff,\n\nOn Fri, Jul 31, 2020 at 7:45 PM Jeff King wrote:\n>\n> On Fri, Jul 31, 2020 at 07:37:22PM +0200, Alban Gruin wrote:\n>\n> > > + # Leave $expect unquoted to lose possible leading whitespaces\n> > > + echo $expect >expected\n> > > + test_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n> >\n> > There is a typo here, and $expect is written to `expected', but\n> > `test_cmp' wants `expect'. Fixing those mistakes does not reveal any\n> > broken tests.\n>\n> I thought at first you meant that the typo was s/expected/expect, and\n> wondered how this could possibly have passed. But the typo is\n> s/sucess/success/, so we were in fact not running the test at all (and\n> were generating \"test_expect_sucess: not found\" messages to stderr, but\n> outside of any test block. Yikes.\n>\n> Thanks for spotting.\n\nYeah, I copied a suggestion from Junio in the last iteration without\nproperly checking it. Sorry about that and thanks for spotting and\nfixing it.\n"},{"id":"402594","messageId":"20200731203004.GA1440843@coredump.intra.peff.net","threadId":"53811","inReplyTo":"xmqqpn8b30zp.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v1] t6300: fix issues related to %(contents:size)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-31T20:30:04Z","receivedAt":"2020-07-31T20:30:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 31, 2020 at 01:04:10PM -0700, Junio C Hamano wrote:\n\n> > Definitely fixes the issue, though I wonder:\n> >\n> >> -\t\techo $expect >expected\n> >> -\t\ttest_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n> >> +\t\techo $expect >expect\n> >> +\t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n> >> \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n> >> \t\t\ttest_cmp expect actual\n> >> \t\t'\n> >\n> > Should we instead switch the test_cmp to look at \"expected\" to be\n> > consistent with the rest of the tests in this file?\n> \n> If I recall correctly, \"expect vs actual\" were more common when I\n> counted across all the tests last time. Matching local convention\n> is fine, though.\n\nYes, I agree that \"expect\" is where we should be heading overall. I\nthink matching local convention is best here to avoid introducing new\nmistakes like this one, but I wouldn't be opposed to somebody switching\nout s/expected/expect/ in the whole file.\n\n-Peff\n"},{"id":"402595","messageId":"xmqqa6zf2zs4.fsf@gitster.c.googlers.com","threadId":"53811","inReplyTo":"CAP8UFD15+p+xKwJ=B9WVsrc+2TvLHKmu78SBCLUFZVSYoTtbbg@mail.gmail.com","subject":"Re: [PATCH v5 3/3] ref-filter: add support for %(contents:size)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-31T20:30:19Z","receivedAt":"2020-07-31T20:30:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Hi Alban and Peff,\n>\n> On Fri, Jul 31, 2020 at 7:45 PM Jeff King wrote:\n>>\n>> On Fri, Jul 31, 2020 at 07:37:22PM +0200, Alban Gruin wrote:\n>>\n>> > > + # Leave $expect unquoted to lose possible leading whitespaces\n>> > > + echo $expect >expected\n>> > > + test_expect_${4:-sucess} $PREREQ \"basic atom: $1 contents:size\" '\n>> >\n>> > There is a typo here, and $expect is written to `expected', but\n>> > `test_cmp' wants `expect'. Fixing those mistakes does not reveal any\n>> > broken tests.\n>>\n>> I thought at first you meant that the typo was s/expected/expect, and\n>> wondered how this could possibly have passed. But the typo is\n>> s/sucess/success/, so we were in fact not running the test at all (and\n>> were generating \"test_expect_sucess: not found\" messages to stderr, but\n>> outside of any test block. Yikes.\n>>\n>> Thanks for spotting.\n>\n> Yeah, I copied a suggestion from Junio in the last iteration without\n> properly checking it. Sorry about that and thanks for spotting and\n> fixing it.\n\nI probably should stop giving \"perhaps along the lines of this\"\nsuggestion too lightly and/or when I do not have enough time to\napply and test myself. Sorry for the gotcha.\n"},{"id":"402596","messageId":"20200731204057.GA1440890@coredump.intra.peff.net","threadId":"53811","inReplyTo":"xmqqa6zf2zs4.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 3/3] ref-filter: add support for %(contents:size)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-31T20:40:57Z","receivedAt":"2020-07-31T20:41:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 31, 2020 at 01:30:19PM -0700, Junio C Hamano wrote:\n\n> > Yeah, I copied a suggestion from Junio in the last iteration without\n> > properly checking it. Sorry about that and thanks for spotting and\n> > fixing it.\n> \n> I probably should stop giving \"perhaps along the lines of this\"\n> suggestion too lightly and/or when I do not have enough time to\n> apply and test myself. Sorry for the gotcha.\n\nI dunno. I appreciate getting them, especially in patch form. It's often\na more precise description than hand-wavy English, and being a patch\nmakes it easy to apply into my tree as a starting point. The real trick\nis that the receiver needs to know enough to distrust the suggestion and\ntake ownership of it. Maybe you just need a bigger disclaimer. ;)\n\n(Only half-joking; I do try to say \"not tested\" or \"not even compiled\"\nwhen that is the case in stuff I sent out, but I'm sure I'm not\nconsistent).\n\n-Peff\n"}]}