{"thread":{"id":"46862","subject":"[PATCH 0/5] Support %(trailers) arguments in for-each-ref(1)","startedAt":"2017-09-30T06:22:53Z","lastAt":"2017-10-03T06:40:36Z","messageCount":78,"participants":["Taylor Blau","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"329230","messageId":"20170930062238.87077-1-me@ttaylorr.com","threadId":"46862","inReplyTo":null,"subject":"[PATCH 0/5] Support %(trailers) arguments in for-each-ref(1)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T06:22:33Z","receivedAt":"2017-09-30T06:22:53Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nAttached is a patch to extend Peff's recent work of adding parsing options to\n\"--pretty=%(trailers)\" by supporting those same options in git-for-each-ref(1).\n\nIn summary, this patch adds correct behavior for the following options, when\ngiven to git-for-each-ref(1):\n\n * --format=\"%(trailers:only)\"\n\n * --format=\"%(trailers:unfold,unfold)\"\n\n * --format=\"%(contents:trailers:only)\"\n\n * --format=\"%(contents:trailers:unfold,unfold)\"\n\nI have changed the syntax for specifying multiple sub-arguments in\n%(contents:trailers) and %(trailers) atoms to be \",\"-delimited instead of\n%\":\"-delimited. This is consistent with similar atoms, and is described in\n%greater detail in \"pretty.c: delimit \"%(trailers)\" arguments with \",\"\".\n\nI am also new around here: this is my first patch that I am sending to the\nmailing list, so this process is entirely new to me. My current focus is helping\nmaintain Git LFS [1] at GitHub. If I have made any mistakes in formatting these\npatches or sending them, please let me know :-).\n\nThank you in advance.\n\n--\n- Taylor\n\n[1]: https://git-lfs.github.com\n\nTaylor Blau (5):\n pretty.c: delimit \"%(trailers)\" arguments with \",\"\n t6300: refactor %(trailers) tests\n ref-filter.c: add trailer options to used_atom\n ref-filter.c: use trailer_opts to format trailers\n ref-filter.c: parse trailers arguments with %(contents) atom\n\n Documentation/git-for-each-ref.txt | 7 ++-\n pretty.c | 13 +++---\n ref-filter.c | 38 +++++++++++-----\n t/t4205-log-pretty-formats.sh | 4 +-\n t/t6300-for-each-ref.sh | 88 +++++++++++++++++++++++++++++++++++++-\n 5 files changed, 129 insertions(+), 21 deletions(-)\n\n--\n2.14.1.145.gb3622a4ee\n\n"},{"id":"329231","messageId":"20170930062238.87077-2-me@ttaylorr.com","threadId":"46862","inReplyTo":"20170930062238.87077-1-me@ttaylorr.com","subject":"[PATCH 1/5] pretty.c: delimit \"%(trailers)\" arguments with \",\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T06:22:34Z","receivedAt":"2017-09-30T06:22:55Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation for adding consistent \"%(trailers)\" atom options to\n`git-for-each-ref(1)`'s \"--format\" argument, change \"%(trailers)\" in pretty.c\nto separate sub-arguments with a \",\", instead of a \":\".\n\nMultiple sub-arguments are given either as \"%(trailers:unfold,only)\" or\n\"%(trailers:only,unfold)\".\n\nThis change disambiguates between \"top-level\" arguments, and arguments given to\nthe trailers atom itself. It is consistent with the behavior of \"%(upstream)\"\nand \"%(push)\" atoms.\n\nSigned-off-by: Taylor Blau \n---\n pretty.c | 13 ++++++++-----\n t/t4205-log-pretty-formats.sh | 4 ++--\n 2 files changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 0e23fe3c0..68f736912 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1265,11 +1265,14 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \n \tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n-\t\twhile (*arg == ':') {\n-\t\t\tif (skip_prefix(arg, \":only\", &arg))\n-\t\t\t\topts.only_trailers = 1;\n-\t\t\telse if (skip_prefix(arg, \":unfold\", &arg))\n-\t\t\t\topts.unfold = 1;\n+\t\tif (skip_prefix(arg, \":\", &arg)) {\n+\t\t\twhile (*arg != ')') {\n+\t\t\t\tskip_prefix(arg, \",\", &arg);\n+\t\t\t\tif (skip_prefix(arg, \"only\", &arg))\n+\t\t\t\t\topts.only_trailers = 1;\n+\t\t\t\telse if (skip_prefix(arg, \"unfold\", &arg))\n+\t\t\t\t\topts.unfold = 1;\n+\t\t\t}\n \t\t}\n \t\tif (*arg == ')') {\n \t\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex ec5f53010..977472f53 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -588,8 +588,8 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '\n '\n \n test_expect_success ':only and :unfold work together' '\n-\tgit log --no-walk --pretty=\"%(trailers:only:unfold)\" >actual &&\n-\tgit log --no-walk --pretty=\"%(trailers:unfold:only)\" >reverse &&\n+\tgit log --no-walk --pretty=\"%(trailers:only,unfold)\" >actual &&\n+\tgit log --no-walk --pretty=\"%(trailers:unfold,only)\" >reverse &&\n \ttest_cmp actual reverse &&\n \t{\n \t\tgrep -v patch.description \n---\n t/t6300-for-each-ref.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 834a9ed16..2a9fcf713 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -592,18 +592,25 @@ test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '\n cat >trailers <\n Signed-off-by: A U Thor \n+[ v2 updated patch description ]\n+Acked-by: A U Thor\n+ \n EOF\n \n-test_expect_success 'basic atom: head contents:trailers' '\n+\n+test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n \tgit add two &&\n-\tgit commit -F - <<-EOF &&\n+\tgit commit -F - <<-EOF\n \ttrailers: this commit message has trailers\n \n \tSome message contents\n \n \t$(cat trailers)\n \tEOF\n+'\n+\n+test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n \t# git for-each-ref ends with a blank line\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329233","messageId":"20170930062238.87077-4-me@ttaylorr.com","threadId":"46862","inReplyTo":"20170930062238.87077-1-me@ttaylorr.com","subject":"[PATCH 3/5] ref-filter.c: add trailer options to used_atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T06:22:36Z","receivedAt":"2017-09-30T06:23:00Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation for \"%(trailers)\" to take trailer parsing arguments, use Jeff's\nconvenience structure for trailer processing options introduced in 8abc89800c.\n\nWe will later populate this field from the arguments given to %(trailers), and\nthen use the trailer_opts instance to format ref trailers correctly using\n`format_trailer_from_commit`.\n\nSigned-off-by: Taylor Blau \n---\n ref-filter.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 467c0279c..84f14093c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -82,6 +82,7 @@ static struct used_atom {\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\tstruct process_trailer_options trailer_opts;\n \t\t\tunsigned int nlines;\n \t\t} contents;\n \t\tstruct {\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329234","messageId":"20170930062238.87077-5-me@ttaylorr.com","threadId":"46862","inReplyTo":"20170930062238.87077-1-me@ttaylorr.com","subject":"[PATCH 4/5] ref-filter.c: use trailer_opts to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T06:22:37Z","receivedAt":"2017-09-30T06:23:03Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation to support additional sub-arguments given to the \"%(trailers)\"\natom, use 'format_trailers_from_commit' in order to format trailers in the\ndesired manner.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 6 +++++-\n ref-filter.c | 31 ++++++++++++++++++++---------\n t/t6300-for-each-ref.sh | 40 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 67 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 03e187a10..b7325a25d 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -213,7 +213,11 @@ 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 Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n-are obtained as 'contents:trailers'.\n+are obtained as 'contents:trailers'. Non-trailer lines from the trailer block\n+can be omitted with 'trailers:only'. Whitespace-continuations can be removed\n+from trailers so that each trailer appears on a line by itself with its full\n+content with 'trailers:unfold'. Both can be used together as\n+'trailers:unfold,only'.\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 84f14093c..8573acbed 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -178,9 +178,23 @@ static void subject_atom_parser(struct used_atom *atom, const char *arg)\n \n static void trailers_atom_parser(struct used_atom *atom, const char *arg)\n {\n-\tif (arg)\n-\t\tdie(_(\"%%(trailers) does not take arguments\"));\n+\tstruct string_list params = STRING_LIST_INIT_DUP;\n+\tint i;\n+\n+\tif (arg) {\n+\t\tstring_list_split(¶ms, arg, ',', -1);\n+\t\tfor (i = 0; i < params.nr; i++) {\n+\t\t\tconst char *s = params.items[i].string;\n+\t\t\tif (!strcmp(s, \"unfold\"))\n+\t\t\t\tatom->u.contents.trailer_opts.unfold = 1;\n+\t\t\telse if (!strcmp(s, \"only\"))\n+\t\t\t\tatom->u.contents.trailer_opts.only_trailers = 1;\n+\t\t\telse\n+\t\t\t\tdie(_(\"unknown %%(trailers) argument: %s\"), s);\n+\t\t}\n+\t}\n \tatom->u.contents.option = C_TRAILERS;\n+\tstring_list_clear(¶ms, 0);\n }\n \n static void contents_atom_parser(struct used_atom *atom, const char *arg)\n@@ -1035,7 +1049,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tname++;\n \t\tif (strcmp(name, \"subject\") &&\n \t\t strcmp(name, \"body\") &&\n-\t\t strcmp(name, \"trailers\") &&\n+\t\t !starts_with(name, \"trailers\") &&\n \t\t !starts_with(name, \"contents\"))\n \t\t\tcontinue;\n \t\tif (!subpos)\n@@ -1060,13 +1074,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_TRAILERS) {\n-\t\t\tstruct trailer_info info;\n+\t\t\tstruct strbuf s = STRBUF_INIT;\n \n-\t\t\t/* Search for trailer info */\n-\t\t\ttrailer_info_get(&info, subpos);\n-\t\t\tv->s = xmemdupz(info.trailer_start,\n-\t\t\t\t\tinfo.trailer_end - info.trailer_start);\n-\t\t\ttrailer_info_release(&info);\n+\t\t\t/* Format the trailer info according to the trailer_opts given */\n+\t\t\tformat_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);\n+\n+\t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n \t}\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2a9fcf713..2bd0c5da7 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -597,6 +597,9 @@ Acked-by: A U Thor\n \n EOF\n \n+unfold () {\n+\tperl -0pe 's/\\n\\s+/ /'\n+}\n \n test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n@@ -610,6 +613,43 @@ test_expect_success 'set up trailers for next test' '\n \tEOF\n '\n \n+test_expect_success '%(trailers:unfold) unfolds trailers' '\n+ git for-each-ref --format=\"%(trailers:unfold)\" refs/heads/master >actual &&\n+ {\n+ unfold expect &&\n+ test_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+ test_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n+ test_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329235","messageId":"20170930062238.87077-6-me@ttaylorr.com","threadId":"46862","inReplyTo":"20170930062238.87077-1-me@ttaylorr.com","subject":"[PATCH 5/5] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T06:22:38Z","receivedAt":"2017-09-30T06:23:27Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The %(contents) atom takes a contents \"field\" as its argument. Since \"trailers\"\nis one of those fields, extend contents_atom_parser to parse \"trailers\"'s\narguments when used through \"%(contents)\", like:\n\n %(contents:trailers:unfold,only)\n\nA caveat: trailers_atom_parser expects NULL when no arguments are given (see:\n`parse_ref_filter_atom`). To simulate this behavior without teaching\ntrailers_atom_parser to accept strings with length zero, conditionally pass\nNULL to trailers_atom_parser if the arguments portion of the argument to\n%(contents) is empty.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 9 +++++----\n ref-filter.c | 6 ++++--\n t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 46 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex b7325a25d..0aaac8af9 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -214,10 +214,11 @@ blank line. The optional GPG signature is `contents:signature`. The\n first `N` lines of the message is obtained using `contents:lines=N`.\n Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n are obtained as 'contents:trailers'. Non-trailer lines from the trailer block\n-can be omitted with 'trailers:only'. Whitespace-continuations can be removed\n-from trailers so that each trailer appears on a line by itself with its full\n-content with 'trailers:unfold'. Both can be used together as\n-'trailers:unfold,only'.\n+can be omitted with 'trailers:only', or 'contents:trailers:only'.\n+Whitespace-continuations can be removed from trailers so that each trailer\n+appears on a line by itself with its full content with 'trailers:unfold' or\n+'contents:trailers:unfold'. Both can be used together as 'trailers:unfold,only',\n+or 'contents:trailers:unfold,only'.\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8573acbed..a8d4a52bd 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -207,8 +207,10 @@ static void contents_atom_parser(struct used_atom *atom, const char *arg)\n \t\tatom->u.contents.option = C_SIG;\n \telse if (!strcmp(arg, \"subject\"))\n \t\tatom->u.contents.option = C_SUB;\n-\telse if (!strcmp(arg, \"trailers\"))\n-\t\tatom->u.contents.option = C_TRAILERS;\n+\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n+\t\tskip_prefix(arg, \":\", &arg);\n+\t\ttrailers_atom_parser(atom, strlen(arg) ? arg : NULL);\n+\t}\n \telse if (skip_prefix(arg, \"lines=\", &arg)) {\n \t\tatom->u.contents.option = C_LINES;\n \t\tif (strtoul_ui(arg, 10, &atom->u.contents.nlines))\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2bd0c5da7..d9b71589f 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -642,6 +642,35 @@ test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers:unfold) unfolds trailers' '\n+ git for-each-ref --format=\"%(contents:trailers:unfold)\" refs/heads/master >actual &&\n+ {\n+ unfold expect &&\n+ test_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) and %(contents:trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%(trailers) rejects unknown trailers arguments' '\n \tcat >expect <<-EOF &&\n \tfatal: unknown %(trailers) argument: unsupported\n@@ -650,6 +679,14 @@ test_expect_success '%(trailers) rejects unknown trailers arguments' '\n test_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+ test_must_fail git for-each-ref --format=\"%(contents:trailers:unsupported)\" 2>actual &&\n+ test_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329236","messageId":"20170930064949.23plbzjtw2qupj6b@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20170930062238.87077-2-me@ttaylorr.com","subject":"Re: [PATCH 1/5] pretty.c: delimit \"%(trailers)\" arguments with \",\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-30T06:49:49Z","receivedAt":"2017-09-30T06:49:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 29, 2017 at 11:22:34PM -0700, Taylor Blau wrote:\n\n> In preparation for adding consistent \"%(trailers)\" atom options to\n> `git-for-each-ref(1)`'s \"--format\" argument, change \"%(trailers)\" in pretty.c\n> to separate sub-arguments with a \",\", instead of a \":\".\n> \n> Multiple sub-arguments are given either as \"%(trailers:unfold,only)\" or\n> \"%(trailers:only,unfold)\".\n> \n> This change disambiguates between \"top-level\" arguments, and arguments given to\n> the trailers atom itself. It is consistent with the behavior of \"%(upstream)\"\n> and \"%(push)\" atoms.\n\nGreat. I hope we'll eventually unify the pretty.c and ref-filter.c\nsyntaxes and implementations, so consistency is good. I'm sure there\nwill be some hackery to keep backwards compatibility in all cases, but\nthe fewer the better. The \"every option gets its own colon\" scheme was\njust something I invented, because there was no prior art in pretty.c. I\ndidn't think to check for similar cases in ref-filter.c.\n\nWe should be safe to change this unconditionally, without worrying about\nbreaking compatibility. These options were added in 58311c66fd (pretty:\nsupport normalization options for %(trailers), 2017-08-15), which hasn't\nbeen in any released version yet.\n\n> ---\n> pretty.c | 13 ++++++++-----\n> t/t4205-log-pretty-formats.sh | 4 ++--\n> 2 files changed, 10 insertions(+), 7 deletions(-)\n\nI think this needs an update to Documentation/pretty-formats.txt. Maybe:\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 6b4a12f028..40948d925b 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -199,11 +199,13 @@ endif::git-rev-list[]\n than given and there are spaces on its left, use those spaces\n - '%><()', '%><|()': similar to '% <()', '%<|()'\n respectively, but padding both sides (i.e. the text is centered)\n-- %(trailers): display the trailers of the body as interpreted by\n- linkgit:git-interpret-trailers[1]. If the `:only` option is given,\n- omit non-trailer lines from the trailer block. If the `:unfold`\n- option is given, behave as if interpret-trailer's `--unfold` option\n- was given. E.g., `%(trailers:only:unfold)` to do both.\n+- %(trailers[:options]): display the trailers of the body as interpreted\n+ by linkgit:git-interpret-trailers[1]. The `trailers` string may be\n+ followed by a colon and zero or more comma-separated options. If the\n+ `only` option is given, omit non-trailer lines from the trailer block.\n+ If the `unfold` option is given, behave as if interpret-trailer's\n+ `--unfold` option was given. E.g., `%(trailers:only,unfold)` to do\n+ both.\n \n NOTE: Some placeholders may depend on other options given to the\n revision traversal engine. For example, the `%g*` reflog options will\n\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex ec5f530102..977472f539 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -588,8 +588,8 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '\n '\n \n test_expect_success ':only and :unfold work together' '\n-\tgit log --no-walk --pretty=\"%(trailers:only:unfold)\" >actual &&\n-\tgit log --no-walk --pretty=\"%(trailers:unfold:only)\" >reverse &&\n+\tgit log --no-walk --pretty=\"%(trailers:only,unfold)\" >actual &&\n+\tgit log --no-walk --pretty=\"%(trailers:unfold,only)\" >reverse &&\n \ttest_cmp actual reverse &&\n \t{\n \t\tgrep -v patch.description diff --git a/pretty.c b/pretty.c\n> index 0e23fe3c0..68f736912 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -1265,11 +1265,14 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n> \n> \tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n> \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n> -\t\twhile (*arg == ':') {\n> -\t\t\tif (skip_prefix(arg, \":only\", &arg))\n> -\t\t\t\topts.only_trailers = 1;\n> -\t\t\telse if (skip_prefix(arg, \":unfold\", &arg))\n> -\t\t\t\topts.unfold = 1;\n> +\t\tif (skip_prefix(arg, \":\", &arg)) {\n> +\t\t\twhile (*arg != ')') {\n> +\t\t\t\tskip_prefix(arg, \",\", &arg);\n> +\t\t\t\tif (skip_prefix(arg, \"only\", &arg))\n> +\t\t\t\t\topts.only_trailers = 1;\n> +\t\t\t\telse if (skip_prefix(arg, \"unfold\", &arg))\n> +\t\t\t\t\topts.unfold = 1;\n> +\t\t\t}\n> \t\t}\n\nDo we always make forward progress with this loop condition?\n\nIf arg doesn't start with a close-paren, we'll keep looping. But if it's\nnot a comma or one of the option strings, then skip_prefix() won't move\nthe pointer forward. So I think \"%(trailers:foo)\" would loop forever.\n\nThe original would complain about that and even almost-right things like\n\"%(trailers:only-the-lonely\", though it's pretty subtle. The atom\nparsers in ref-filter.c use string_list_split() which is way more\nreadable and obvious. But I don't think we can do that here. ref-filter\nhas a separate parsing step, so it can afford to be slow. Here we're\nparsing the pretty format individually for each commit in \"git log\", and\nallocations would be noticeable.\n\nHere's what I came up with:\n\n static int match_placeholder_arg(const char *to_parse,\n \t\t\t\t const char *candidate,\n \t\t\t\t const char **end)\n {\n \tconst char *p;\n \tif (!skip_prefix(to_parse, candidate, &p))\n \t\treturn 0;\n \tif (*p == ',') {\n \t\t*end = p + 1;\n \t\treturn 1;\n \t}\n \tif (*p == ')') {\n \t\t*end = p;\n \t\treturn 1;\n \t}\n \treturn 0;\n }\n\n ...\n\n \tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n \t\tif (*arg == ':') {\n \t\t\targ++;\n \t\t\tfor (;;) {\n \t\t\t\tif (match_placeholder_arg(arg, \"only\", &arg))\n \t\t\t\t\topts.only_trailers = 1;\n \t\t\t\telse if (match_placeholder_arg(arg, \"unfold\", &arg))\n \t\t\t\t\topts.unfold = 1;\n \t\t\t\telse\n \t\t\t\t\tbreak;\n \t\t\t}\n \t\t}\n \t\tif (*arg == ')') {\n \t\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\n \t\t\treturn arg - placeholder + 1;\n \t\t}\n \t}\n\nI hate the \"for (;;)\", but at least it's easy to see that each iteration\neither makes progress or breaks out of the loop.\n\n-Peff\n"},{"id":"329237","messageId":"20170930070127.xvtn7dfyuoh26mhp@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20170930062238.87077-3-me@ttaylorr.com","subject":"Re: [PATCH 2/5] t6300: refactor %(trailers) tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-30T07:01:28Z","receivedAt":"2017-09-30T07:01:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 29, 2017 at 11:22:35PM -0700, Taylor Blau wrote:\n\n> We currently have one test for %(trailers) in `git-for-each-ref(1)`, through\n> \"%(contents:trailers)\". In preparation for more, let's add a few things:\n> \n> - Move the commit creation step to its own test so that it can be re-used.\n> \n> - Add a non-trailer to the commit's trailers to test that non-trailers aren't\n> shown using \"%(trailers:only)\".\n> \n> - Add a multi-line trailer to ensure that trailers are unfolded correctly\n> using \"%(trailers:unfold)\".\n\nThis is a minor nit, but since you invited formatting critique in your\ncover letter, I feel entitled. :)\n\nConsider wrapping your commit messages (and emails in general) at\n72 characters, rather than 80. That lets them show well on an 80-column\ndisplay even when indented by \"git log\" or by inline quoting in an email\nreply.\n\nI'm also of the opinion that while 80 characters is fine for code, it's\na bit wide for English text. You can find various claims online[1] from\npeople interested in typography that a line width of about 60-70\ncharacters is pleasant for reading.\n\n[1] E.g., https://baymard.com/blog/line-length-readability\n\n> Signed-off-by: Taylor Blau \n> ---\n> t/t6300-for-each-ref.sh | 11 +++++++++--\n> 1 file changed, 9 insertions(+), 2 deletions(-)\n\nThe patch itself looks fine. :)\n\n-Peff\n"},{"id":"329238","messageId":"20170930071057.3mosgarb5coje4pu@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20170930062238.87077-4-me@ttaylorr.com","subject":"Re: [PATCH 3/5] ref-filter.c: add trailer options to used_atom","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-30T07:10:57Z","receivedAt":"2017-09-30T07:11:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 29, 2017 at 11:22:36PM -0700, Taylor Blau wrote:\n\n> In preparation for \"%(trailers)\" to take trailer parsing arguments, use Jeff's\n> convenience structure for trailer processing options introduced in 8abc89800c.\n> \n> We will later populate this field from the arguments given to %(trailers), and\n> then use the trailer_opts instance to format ref trailers correctly using\n> `format_trailer_from_commit`.\n\nI think this should probably just be squashed in with the next patch,\nsince this does nothing at all without adding a caller that uses it.\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 467c0279c..84f14093c 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -82,6 +82,7 @@ static struct used_atom {\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\tstruct process_trailer_options trailer_opts;\n> \t\t\tunsigned int nlines;\n> \t\t} contents;\n\nThis contents struct is odd. The used_atom struct can have many types,\nand has has a big union of type-specific data. And then we break down\nthe \"contents\" type with a further enum, but don't actually put the\ntype-specific data into a union (not just your patch, but already\n\"nlines\" is specific only to C_LINES).\n\nIt's probably not worth caring about, though. The point of a union is to\nreduce the overall struct size, and here our type-specific data is\nfairly small. It would only change the overall struct size if it were\nlarger than other parts of the union (and remote_ref, for example, is\npretty big).\n\n-Peff\n\nPS As an aside, I find the whole %(contents:...) thing a bit\n unfortunate. I understand why the _implementation_ wants to group\n similar options together so that it can avoid parsing too much. But\n the user doesn't care about that. Just \"%(trailers)\" should be\n sufficient (as evidenced by the fact that we added it a separate\n alias). But none of that is new to your series.\n"},{"id":"329239","messageId":"20170930072143.h4mbd5c4hskvcrl3@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20170930062238.87077-5-me@ttaylorr.com","subject":"Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-30T07:21:43Z","receivedAt":"2017-09-30T07:21:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 29, 2017 at 11:22:37PM -0700, Taylor Blau wrote:\n\n> In preparation to support additional sub-arguments given to the \"%(trailers)\"\n> atom, use 'format_trailers_from_commit' in order to format trailers in the\n> desired manner.\n\nThis isn't just in preparation, is it? It looks like the options are\nhere (which I think is fine, but the commit message probably needs\nupdated).\n\n> Signed-off-by: Taylor Blau \n> ---\n> Documentation/git-for-each-ref.txt | 6 +++++-\n> ref-filter.c | 31 ++++++++++++++++++++---------\n> t/t6300-for-each-ref.sh | 40 ++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 67 insertions(+), 10 deletions(-)\n\nThis patch didn't apply for me on top of the others. I get:\n\n Applying: ref-filter.c: use trailer_opts to format trailers\n error: patch failed: ref-filter.c:178\n error: ref-filter.c: patch does not apply\n Patch failed at 0004 ref-filter.c: use trailer_opts to format trailers\n\nAnd then with \"am -3\":\n\n Applying: ref-filter.c: use trailer_opts to format trailers\n error: sha1 information is lacking or useless (ref-filter.c).\n error: could not build fake ancestor\n Patch failed at 0004 ref-filter.c: use trailer_opts to format trailers\n\nDid it get corrupted in transit, or did you hand-edit it?\n\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index 03e187a10..b7325a25d 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -213,7 +213,11 @@ 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> Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n> -are obtained as 'contents:trailers'.\n> +are obtained as 'contents:trailers'. Non-trailer lines from the trailer block\n> +can be omitted with 'trailers:only'. Whitespace-continuations can be removed\n> +from trailers so that each trailer appears on a line by itself with its full\n> +content with 'trailers:unfold'. Both can be used together as\n> +'trailers:unfold,only'.\n\nI know you copied the single-quote formatting from the existing line,\nbut this may be a good opportunity to switch to backticks, which is what\nwe usually prefer these days for literal phrases.\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 84f14093c..8573acbed 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -178,9 +178,23 @@ static void subject_atom_parser(struct used_atom *atom, const char *arg)\n> \n> static void trailers_atom_parser(struct used_atom *atom, const char *arg)\n> {\n> -\tif (arg)\n> -\t\tdie(_(\"%%(trailers) does not take arguments\"));\n> +\tstruct string_list params = STRING_LIST_INIT_DUP;\n> +\tint i;\n> +\n> +\tif (arg) {\n> +\t\tstring_list_split(¶ms, arg, ',', -1);\n> +\t\tfor (i = 0; i < params.nr; i++) {\n> +\t\t\tconst char *s = params.items[i].string;\n> +\t\t\tif (!strcmp(s, \"unfold\"))\n> +\t\t\t\tatom->u.contents.trailer_opts.unfold = 1;\n> +\t\t\telse if (!strcmp(s, \"only\"))\n> +\t\t\t\tatom->u.contents.trailer_opts.only_trailers = 1;\n> +\t\t\telse\n> +\t\t\t\tdie(_(\"unknown %%(trailers) argument: %s\"), s);\n> +\t\t}\n> +\t}\n> \tatom->u.contents.option = C_TRAILERS;\n> +\tstring_list_clear(¶ms, 0);\n> }\n\nThis looks good (and so much nicer than the contortions from pretty.c).\nThe error behavior matches what we currently do for %(align), which\nmakes sense.\n\nThe trailer_opts should be zero-initialized to start with due to us\ncalling memset on the whole used_atom struct.\n\n> static void contents_atom_parser(struct used_atom *atom, const char *arg)\n> @@ -1035,7 +1049,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n> \t\t\tname++;\n> \t\tif (strcmp(name, \"subject\") &&\n> \t\t strcmp(name, \"body\") &&\n> -\t\t strcmp(name, \"trailers\") &&\n> +\t\t !starts_with(name, \"trailers\") &&\n> \t\t !starts_with(name, \"contents\"))\n> \t\t\tcontinue;\n> \t\tif (!subpos)\n> @@ -1060,13 +1074,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n> \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n> \t\t\tv->s = strbuf_detach(&s, NULL);\n> \t\t} else if (atom->u.contents.option == C_TRAILERS) {\n> -\t\t\tstruct trailer_info info;\n> +\t\t\tstruct strbuf s = STRBUF_INIT;\n> \n> -\t\t\t/* Search for trailer info */\n> -\t\t\ttrailer_info_get(&info, subpos);\n> -\t\t\tv->s = xmemdupz(info.trailer_start,\n> -\t\t\t\t\tinfo.trailer_end - info.trailer_start);\n> -\t\t\ttrailer_info_release(&info);\n> +\t\t\t/* Format the trailer info according to the trailer_opts given */\n> +\t\t\tformat_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);\n> +\n> +\t\t\tv->s = strbuf_detach(&s, NULL);\n\nAnd this all looks straightforward and correct.\n\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 2a9fcf713..2bd0c5da7 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n\nThe tests are basically an adaptation of the ones from 58311c66fd\n(pretty: support normalization options for %(trailers), 2017-08-15),\nwhich make sense.\n\nOne thing I did notice:\n\n> @@ -610,6 +613,43 @@ test_expect_success 'set up trailers for next test' '\n> \tEOF\n> '\n> \n> +test_expect_success '%(trailers:unfold) unfolds trailers' '\n> + git for-each-ref --format=\"%(trailers:unfold)\" refs/heads/master >actual &&\n> + {\n> + unfold + echo\n> + } >expect &&\n> + test_cmp expect actual\n> +'\n\nThis looks like two-space indentation, when it should be a tab.\n\n> +test_expect_success '%(trailers:only) shows only \"key: value\" trailers' '\n> +\tgit for-each-ref --format=\"%(trailers:only)\" refs/heads/master >actual &&\n> +\t{\n> +\t\tgrep -v patch.description +\t\techo\n> +\t} >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n> +\tgit for-each-ref --format=\"%(trailers:only,unfold)\" refs/heads/master >actual &&\n> +\tgit for-each-ref --format=\"%(trailers:unfold,only)\" refs/heads/master >reverse &&\n> +\ttest_cmp actual reverse &&\n> +\t{\n> +\t\tgrep -v patch.description +\t\techo\n> +\t} >expect &&\n> +\ttest_cmp expect actual\n> +'\n\nThese ones are tabs. GOod.\n\n\n> +test_expect_success '%(trailers) rejects unknown trailers arguments' '\n> +\tcat >expect <<-EOF &&\n> +\tfatal: unknown %(trailers) argument: unsupported\n> +\tEOF\n> + test_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n> + test_cmp expect actual\n> +'\n\nBut this one is mixed. :)\n\n-Peff\n"},{"id":"329240","messageId":"20170930073615.js3o57qmxlqnkpp5@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20170930062238.87077-6-me@ttaylorr.com","subject":"Re: [PATCH 5/5] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-30T07:36:15Z","receivedAt":"2017-09-30T07:36:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 29, 2017 at 11:22:38PM -0700, Taylor Blau wrote:\n\n> The %(contents) atom takes a contents \"field\" as its argument. Since \"trailers\"\n> is one of those fields, extend contents_atom_parser to parse \"trailers\"'s\n> arguments when used through \"%(contents)\", like:\n> \n> %(contents:trailers:unfold,only)\n> \n> A caveat: trailers_atom_parser expects NULL when no arguments are given (see:\n> `parse_ref_filter_atom`). To simulate this behavior without teaching\n> trailers_atom_parser to accept strings with length zero, conditionally pass\n> NULL to trailers_atom_parser if the arguments portion of the argument to\n> %(contents) is empty.\n\nYeah, this is a weird effect of trailers_atom_parser() doing double-duty\nto parse both \"%(contents:trailers)\" and \"%(trailers)\".\n\nThough I think trailers_atom_parser() does do the sensible thing with an\nempty string (there are no options, so nothing to parse). I.e., I'd\nexpect the same thing out of:\n\n %(trailers:)\n\nand\n\n %(trailers)\n\neven though one gets a NULL \"arg\" field and the other gets an empty\nstring.\n\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index b7325a25d..0aaac8af9 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -214,10 +214,11 @@ blank line. The optional GPG signature is `contents:signature`. The\n> first `N` lines of the message is obtained using `contents:lines=N`.\n> Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n> are obtained as 'contents:trailers'. Non-trailer lines from the trailer block\n> -can be omitted with 'trailers:only'. Whitespace-continuations can be removed\n> -from trailers so that each trailer appears on a line by itself with its full\n> -content with 'trailers:unfold'. Both can be used together as\n> -'trailers:unfold,only'.\n> +can be omitted with 'trailers:only', or 'contents:trailers:only'.\n> +Whitespace-continuations can be removed from trailers so that each trailer\n> +appears on a line by itself with its full content with 'trailers:unfold' or\n> +'contents:trailers:unfold'. Both can be used together as 'trailers:unfold,only',\n> +or 'contents:trailers:unfold,only'.\n\nRather than enumerate each, we might do better to just say explicitly\n\"contents:trailers\" and \"trailers\" are aliases of one another. It looks\nlike we don't even document %(trailers) at all here. I'd actually be in\nfavor of just declaring %(trailers) the official spelling, and calling\n\"%(contents:trailers)\" a historical alias.\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 8573acbed..a8d4a52bd 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -207,8 +207,10 @@ static void contents_atom_parser(struct used_atom *atom, const char *arg)\n> \t\tatom->u.contents.option = C_SIG;\n> \telse if (!strcmp(arg, \"subject\"))\n> \t\tatom->u.contents.option = C_SUB;\n> -\telse if (!strcmp(arg, \"trailers\"))\n> -\t\tatom->u.contents.option = C_TRAILERS;\n> +\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n> +\t\tskip_prefix(arg, \":\", &arg);\n> +\t\ttrailers_atom_parser(atom, strlen(arg) ? arg : NULL);\n> +\t}\n\nWe usually spell \"is this an empty string?\" as \"*arg\" rather than\ncalling strlen().\n\nHowever, I'm not sure we need to check. As I said above, I think\ntrailers_atom_parser() does the sensible thing with an empty string.\n\nAnd if we _did_ want to distinguish between\n\n %(contents:trailers:)\n\nand\n\n %(contents:trailers)\n\nthen I don't think this quite does it. It passes NULL for both cases. So\nif we really want to emulate how parse_ref_filter_atom calls it, we'd\nwant:\n\n if (!skip_prefix(arg, \":\", &arg))\n\targ = NULL;\n trailers_atom_parser(atom, arg);\n\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 2bd0c5da7..d9b71589f 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -642,6 +642,35 @@ test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success '%(contents:trailers:unfold) unfolds trailers' '\n> + git for-each-ref --format=\"%(contents:trailers:unfold)\" refs/heads/master >actual &&\n> + {\n\nThis has the same spaces/tabs thing going on as the previous commit.\n\n-Peff\n"},{"id":"329241","messageId":"20170930073800.67jko2bns6mccevk@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20170930062238.87077-1-me@ttaylorr.com","subject":"Re: [PATCH 0/5] Support %(trailers) arguments in for-each-ref(1)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-30T07:38:01Z","receivedAt":"2017-09-30T07:38:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 29, 2017 at 11:22:33PM -0700, Taylor Blau wrote:\n\n> Attached is a patch to extend Peff's recent work of adding parsing options to\n> \"--pretty=%(trailers)\" by supporting those same options in git-for-each-ref(1).\n\nThanks for working on this. The direction and general sketch of the\nimplementation look right to me. I noted a few problems inline, so this\nisn't quite right to be picked up, but I think a v2 should get us pretty\nclose, if not all the way there.\n\n-Peff\n"},{"id":"329251","messageId":"20170930184151.GB43975@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"20170930062238.87077-1-me@ttaylorr.com","subject":"[PATCH v2 0/6] Support %(trailers) arguments in for-each-ref(1)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T18:41:51Z","receivedAt":"2017-09-30T18:41:59Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nAttached is v2 of my patch-set \"Support %(trailers) arguments in\nfor-each-ref(1)\".\n\nIt includes the following changes since v1:\n\n * Accept \"empty-sub-argument\" lists, like %(contents:trailers:).\n\n * Fix incorrect tabs/spaces formatting in t6300.\n\n * Modern-ize code fencing in Documentation/git-for-each-ref.txt\n\n * Squash commit adding trailer_opts to used_atom structure.\n\n---\n\n [1/6]: pretty.c: delimit \"%(trailers)\" arguments with \",\"\n [2/6]: t6300: refactor %(trailers) tests\n [3/6]: doc: 'trailers' is the preferred way to format trailers\n [4/6]: doc: use modern \"`\"-style code fencing\n [5/6]: ref-filter.c: use trailer_opts to format trailers\n [6/6]: ref-filter.c: parse trailers arguments with %(contents) atom\n\nThanks in advance :-).\n\n--\n- Taylor\n"},{"id":"329252","messageId":"20170930184629.75900-1-me@ttaylorr.com","threadId":"46862","inReplyTo":"20170930184151.GB43975@D-10-157-251-166.dhcp4.washington.edu","subject":"[PATCH v2 1/6] pretty.c: delimit \"%(trailers)\" arguments with \",\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T18:46:24Z","receivedAt":"2017-09-30T18:46:43Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation for adding consistent \"%(trailers)\" atom options to\n`git-for-each-ref(1)`'s \"--format\" argument, change \"%(trailers)\" in\npretty.c to separate sub-arguments with a \",\", instead of a \":\".\n\nMultiple sub-arguments are given either as \"%(trailers:unfold,only)\" or\n\"%(trailers:only,unfold)\".\n\nThis change disambiguates between \"top-level\" arguments, and arguments\ngiven to the trailers atom itself. It is consistent with the behavior of\n\"%(upstream)\" and \"%(push)\" atoms.\n\nSigned-off-by: Taylor Blau \n---\n pretty.c | 34 +++++++++++++++++++++++++++++-----\n t/t4205-log-pretty-formats.sh | 4 ++--\n 2 files changed, 31 insertions(+), 7 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 0e23fe3c0..eeb51746c 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1036,6 +1036,25 @@ static size_t parse_padding_placeholder(struct strbuf *sb,\n \treturn 0;\n }\n \n+static int match_placeholder_arg(const char *to_parse,\n+\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\tconst char *candidate,\n+\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\tconst char **end)\n+{\n+\tconst char *p;\n+\tif (!(skip_prefix(to_parse, candidate, &p)))\n+\t\treturn 0;\n+\tif (*p == ',') {\n+\t\t*end = p + 1;\n+\t\treturn 1;\n+\t}\n+\tif (*p == ')') {\n+\t\t*end = p;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+\n static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\t\tconst char *placeholder,\n \t\t\t\tvoid *context)\n@@ -1265,11 +1284,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \n \tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n-\t\twhile (*arg == ':') {\n-\t\t\tif (skip_prefix(arg, \":only\", &arg))\n-\t\t\t\topts.only_trailers = 1;\n-\t\t\telse if (skip_prefix(arg, \":unfold\", &arg))\n-\t\t\t\topts.unfold = 1;\n+\t\tif (*arg == ':') {\n+\t\t\targ++;\n+\t\t\tfor (;;) {\n+\t\t\t\tif (match_placeholder_arg(arg, \"only\", &arg))\n+\t\t\t\t\topts.only_trailers = 1;\n+\t\t\t\telse if (match_placeholder_arg(arg, \"unfold\", &arg))\n+\t\t\t\t\topts.unfold = 1;\n+\t\t\t\telse\n+\t\t\t\t\tbreak;\n+\t\t\t}\n \t\t}\n \t\tif (*arg == ')') {\n \t\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex ec5f53010..977472f53 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -588,8 +588,8 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '\n '\n \n test_expect_success ':only and :unfold work together' '\n-\tgit log --no-walk --pretty=\"%(trailers:only:unfold)\" >actual &&\n-\tgit log --no-walk --pretty=\"%(trailers:unfold:only)\" >reverse &&\n+\tgit log --no-walk --pretty=\"%(trailers:only,unfold)\" >actual &&\n+\tgit log --no-walk --pretty=\"%(trailers:unfold,only)\" >reverse &&\n \ttest_cmp actual reverse &&\n \t{\n \t\tgrep -v patch.description \n---\n t/t6300-for-each-ref.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 834a9ed16..2a9fcf713 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -592,18 +592,25 @@ test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '\n cat >trailers <\n Signed-off-by: A U Thor \n+[ v2 updated patch description ]\n+Acked-by: A U Thor\n+ \n EOF\n \n-test_expect_success 'basic atom: head contents:trailers' '\n+\n+test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n \tgit add two &&\n-\tgit commit -F - <<-EOF &&\n+\tgit commit -F - <<-EOF\n \ttrailers: this commit message has trailers\n \n \tSome message contents\n \n \t$(cat trailers)\n \tEOF\n+'\n+\n+test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n \t# git for-each-ref ends with a blank line\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329254","messageId":"20170930184629.75900-4-me@ttaylorr.com","threadId":"46862","inReplyTo":"20170930184629.75900-1-me@ttaylorr.com","subject":"[PATCH v2 4/6] doc: use modern \"`\"-style code fencing","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T18:46:27Z","receivedAt":"2017-09-30T18:46:50Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"\"'\"- (single-quote) styled code fencing is no longer considered modern\nwithin the \"Documentation/\" subtree.\n\nIn preparation for adding additional information to this section of\ngit-for-each-ref(1)'s documentation, update old-style code fencing to\nuse \"`\"-style fencing instead.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 6b38d9a22..b6492820b 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -209,12 +209,12 @@ and `date` to extract the named component.\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+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 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').\n+are obtained as `trailers` (or by using the historical alias\n+`contents:trailers`).\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329255","messageId":"20170930184629.75900-5-me@ttaylorr.com","threadId":"46862","inReplyTo":"20170930184629.75900-1-me@ttaylorr.com","subject":"[PATCH v2 5/6] ref-filter.c: use trailer_opts to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T18:46:28Z","receivedAt":"2017-09-30T18:46:55Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Fill trailer_opts with \"unfold\" and \"only\" to match the sub-arguments\ngiven to the \"%(trailers)\" atom. Then, let's use the filled trailer_opts\ninstance with 'format_trailers_from_commit' in order to format trailers\nin the desired manner.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 5 ++++-\n ref-filter.c | 32 +++++++++++++++++++++---------\n t/t6300-for-each-ref.sh | 40 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 67 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex b6492820b..a1d964182 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -214,7 +214,10 @@ blank line. The optional GPG signature is `contents:signature`. The\n first `N` lines of the message is obtained using `contents:lines=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`).\n+`contents:trailers`). Non-trailer lines from the trailer block can be omitted\n+with `trailers:only`. Whitespace-continuations can be removed from trailers so\n+that each trailer appears on a line by itself with its full content with\n+`trailers:unfold`. Both can be used together as `trailers:unfold,only`.\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 467c0279c..8573acbed 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -82,6 +82,7 @@ static struct used_atom {\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\tstruct process_trailer_options trailer_opts;\n \t\t\tunsigned int nlines;\n \t\t} contents;\n \t\tstruct {\n@@ -177,9 +178,23 @@ static void subject_atom_parser(struct used_atom *atom, const char *arg)\n \n static void trailers_atom_parser(struct used_atom *atom, const char *arg)\n {\n-\tif (arg)\n-\t\tdie(_(\"%%(trailers) does not take arguments\"));\n+\tstruct string_list params = STRING_LIST_INIT_DUP;\n+\tint i;\n+\n+\tif (arg) {\n+\t\tstring_list_split(¶ms, arg, ',', -1);\n+\t\tfor (i = 0; i < params.nr; i++) {\n+\t\t\tconst char *s = params.items[i].string;\n+\t\t\tif (!strcmp(s, \"unfold\"))\n+\t\t\t\tatom->u.contents.trailer_opts.unfold = 1;\n+\t\t\telse if (!strcmp(s, \"only\"))\n+\t\t\t\tatom->u.contents.trailer_opts.only_trailers = 1;\n+\t\t\telse\n+\t\t\t\tdie(_(\"unknown %%(trailers) argument: %s\"), s);\n+\t\t}\n+\t}\n \tatom->u.contents.option = C_TRAILERS;\n+\tstring_list_clear(¶ms, 0);\n }\n \n static void contents_atom_parser(struct used_atom *atom, const char *arg)\n@@ -1034,7 +1049,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tname++;\n \t\tif (strcmp(name, \"subject\") &&\n \t\t strcmp(name, \"body\") &&\n-\t\t strcmp(name, \"trailers\") &&\n+\t\t !starts_with(name, \"trailers\") &&\n \t\t !starts_with(name, \"contents\"))\n \t\t\tcontinue;\n \t\tif (!subpos)\n@@ -1059,13 +1074,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_TRAILERS) {\n-\t\t\tstruct trailer_info info;\n+\t\t\tstruct strbuf s = STRBUF_INIT;\n \n-\t\t\t/* Search for trailer info */\n-\t\t\ttrailer_info_get(&info, subpos);\n-\t\t\tv->s = xmemdupz(info.trailer_start,\n-\t\t\t\t\tinfo.trailer_end - info.trailer_start);\n-\t\t\ttrailer_info_release(&info);\n+\t\t\t/* Format the trailer info according to the trailer_opts given */\n+\t\t\tformat_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);\n+\n+\t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n \t}\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2a9fcf713..6538e7b90 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -597,6 +597,9 @@ Acked-by: A U Thor\n \n EOF\n \n+unfold () {\n+\tperl -0pe 's/\\n\\s+/ /'\n+}\n \n test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n@@ -610,6 +613,43 @@ test_expect_success 'set up trailers for next test' '\n \tEOF\n '\n \n+test_expect_success '%(trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+ test_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n+ test_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329256","messageId":"20170930184629.75900-3-me@ttaylorr.com","threadId":"46862","inReplyTo":"20170930184629.75900-1-me@ttaylorr.com","subject":"[PATCH v2 3/6] doc: 'trailers' is the preferred way to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T18:46:26Z","receivedAt":"2017-09-30T18:46:57Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The documentation makes reference to 'contents:trailers' as an example\nto dig the trailers out of a commit. 'trailers' is an unmentioned\nalternative, which is treated as an alias of 'contents:trailers'.\n\nSince 'trailers' is easier to type, prefer that as the designated way to\ndig out trailers information.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 03e187a10..6b38d9a22 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -213,7 +213,8 @@ 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 Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n-are obtained as 'contents:trailers'.\n+are obtained as 'trailers' (or by using the historical alias\n+'contents:trailers').\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329257","messageId":"20170930184629.75900-6-me@ttaylorr.com","threadId":"46862","inReplyTo":"20170930184629.75900-1-me@ttaylorr.com","subject":"[PATCH v2 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-09-30T18:46:29Z","receivedAt":"2017-09-30T18:46:58Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The %(contents) atom takes a contents \"field\" as its argument. Since\n\"trailers\" is one of those fields, extend contents_atom_parser to parse\n\"trailers\"'s arguments when used through \"%(contents)\", like:\n\n %(contents:trailers:unfold,only)\n\nA caveat: trailers_atom_parser expects NULL when no arguments are given\n(see: `parse_ref_filter_atom`). To simulate this behavior without\nteaching trailers_atom_parser to accept strings with length zero,\nconditionally pass NULL to trailers_atom_parser if the arguments portion\nof the argument to %(contents) is empty.\n\nSigned-off-by: Taylor Blau \n---\n ref-filter.c | 6 ++++--\n t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 2 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8573acbed..f5bde79f3 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -207,8 +207,10 @@ static void contents_atom_parser(struct used_atom *atom, const char *arg)\n \t\tatom->u.contents.option = C_SIG;\n \telse if (!strcmp(arg, \"subject\"))\n \t\tatom->u.contents.option = C_SUB;\n-\telse if (!strcmp(arg, \"trailers\"))\n-\t\tatom->u.contents.option = C_TRAILERS;\n+\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n+\t\tskip_prefix(arg, \":\", &arg);\n+\t\ttrailers_atom_parser(atom, arg);\n+\t}\n \telse if (skip_prefix(arg, \"lines=\", &arg)) {\n \t\tatom->u.contents.option = C_LINES;\n \t\tif (strtoul_ui(arg, 10, &atom->u.contents.nlines))\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 6538e7b90..8c960ec24 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -642,6 +642,35 @@ test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) and %(contents:trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%(trailers) rejects unknown trailers arguments' '\n \tcat >expect <<-EOF &&\n \tfatal: unknown %(trailers) argument: unsupported\n@@ -650,6 +679,14 @@ test_expect_success '%(trailers) rejects unknown trailers arguments' '\n test_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+ test_must_fail git for-each-ref --format=\"%(contents:trailers:unsupported)\" 2>actual &&\n+ test_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329261","messageId":"20171001000647.GA20767@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"20170930184151.GB43975@D-10-157-251-166.dhcp4.washington.edu","subject":"Re: [PATCH v2 0/6] Support %(trailers) arguments in for-each-ref(1)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T00:06:47Z","receivedAt":"2017-10-01T00:06:56Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nAttached is v3 of my patch-set \"Support %(trailers) arguments in\nfor-each-ref(1)\".\n\nIt includes all of the changes in V2:\n> * Accept \"empty-sub-argument\" lists, like %(contents:trailers:).\n>\n> * Fix incorrect tabs/spaces formatting in t6300.\n>\n> * Modern-ize code fencing in Documentation/git-for-each-ref.txt\n>\n> * Squash commit adding trailer_opts to used_atom structure.\n\nAs well as fixing:\n\n * Incorrect indentation in pretty.c (specifically: \"pretty.c: delimit\n \"%(trailers)\" arguments with \",\"\")\n\n * Issues with --format=\"%(contents:trailers:)\", where\n trailers_atom_parser did not correctly handle empty string (\"\") and\n NULL's the same way.\n\nThanks in advance :-).\n\n--\n- Taylor\n"},{"id":"329262","messageId":"20171001001037.23361-1-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001000647.GA20767@D-10-157-251-166.dhcp4.washington.edu","subject":"[PATCH v3 1/6] pretty.c: delimit \"%(trailers)\" arguments with \",\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T00:10:32Z","receivedAt":"2017-10-01T00:10:49Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation for adding consistent \"%(trailers)\" atom options to\n`git-for-each-ref(1)`'s \"--format\" argument, change \"%(trailers)\" in\npretty.c to separate sub-arguments with a \",\", instead of a \":\".\n\nMultiple sub-arguments are given either as \"%(trailers:unfold,only)\" or\n\"%(trailers:only,unfold)\".\n\nThis change disambiguates between \"top-level\" arguments, and arguments\ngiven to the trailers atom itself. It is consistent with the behavior of\n\"%(upstream)\" and \"%(push)\" atoms.\n\nSigned-off-by: Taylor Blau \n---\n pretty.c | 34 +++++++++++++++++++++++++++++-----\n t/t4205-log-pretty-formats.sh | 4 ++--\n 2 files changed, 31 insertions(+), 7 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 0e23fe3c0..9f8d75a84 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1036,6 +1036,25 @@ static size_t parse_padding_placeholder(struct strbuf *sb,\n \treturn 0;\n }\n \n+static int match_placeholder_arg(const char *to_parse,\n+\t\t\t\tconst char *candidate,\n+\t\t\t\tconst char **end)\n+{\n+\tconst char *p;\n+\tif (!(skip_prefix(to_parse, candidate, &p)))\n+\t\treturn 0;\n+\tif (*p == ',') {\n+\t\t*end = p + 1;\n+\t\treturn 1;\n+\t}\n+\tif (*p == ')') {\n+\t\t*end = p;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+\n static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\t\tconst char *placeholder,\n \t\t\t\tvoid *context)\n@@ -1265,11 +1284,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \n \tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n-\t\twhile (*arg == ':') {\n-\t\t\tif (skip_prefix(arg, \":only\", &arg))\n-\t\t\t\topts.only_trailers = 1;\n-\t\t\telse if (skip_prefix(arg, \":unfold\", &arg))\n-\t\t\t\topts.unfold = 1;\n+\t\tif (*arg == ':') {\n+\t\t\targ++;\n+\t\t\tfor (;;) {\n+\t\t\t\tif (match_placeholder_arg(arg, \"only\", &arg))\n+\t\t\t\t\topts.only_trailers = 1;\n+\t\t\t\telse if (match_placeholder_arg(arg, \"unfold\", &arg))\n+\t\t\t\t\topts.unfold = 1;\n+\t\t\t\telse\n+\t\t\t\t\tbreak;\n+\t\t\t}\n \t\t}\n \t\tif (*arg == ')') {\n \t\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex ec5f53010..977472f53 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -588,8 +588,8 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '\n '\n \n test_expect_success ':only and :unfold work together' '\n-\tgit log --no-walk --pretty=\"%(trailers:only:unfold)\" >actual &&\n-\tgit log --no-walk --pretty=\"%(trailers:unfold:only)\" >reverse &&\n+\tgit log --no-walk --pretty=\"%(trailers:only,unfold)\" >actual &&\n+\tgit log --no-walk --pretty=\"%(trailers:unfold,only)\" >reverse &&\n \ttest_cmp actual reverse &&\n \t{\n \t\tgrep -v patch.description \n---\n t/t6300-for-each-ref.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 834a9ed16..2a9fcf713 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -592,18 +592,25 @@ test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '\n cat >trailers <\n Signed-off-by: A U Thor \n+[ v2 updated patch description ]\n+Acked-by: A U Thor\n+ \n EOF\n \n-test_expect_success 'basic atom: head contents:trailers' '\n+\n+test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n \tgit add two &&\n-\tgit commit -F - <<-EOF &&\n+\tgit commit -F - <<-EOF\n \ttrailers: this commit message has trailers\n \n \tSome message contents\n \n \t$(cat trailers)\n \tEOF\n+'\n+\n+test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n \t# git for-each-ref ends with a blank line\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329264","messageId":"20171001001037.23361-3-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001001037.23361-1-me@ttaylorr.com","subject":"[PATCH v3 3/6] doc: 'trailers' is the preferred way to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T00:10:34Z","receivedAt":"2017-10-01T00:10:55Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The documentation makes reference to 'contents:trailers' as an example\nto dig the trailers out of a commit. 'trailers' is an unmentioned\nalternative, which is treated as an alias of 'contents:trailers'.\n\nSince 'trailers' is easier to type, prefer that as the designated way to\ndig out trailers information.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 03e187a10..6b38d9a22 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -213,7 +213,8 @@ 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 Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n-are obtained as 'contents:trailers'.\n+are obtained as 'trailers' (or by using the historical alias\n+'contents:trailers').\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329265","messageId":"20171001001037.23361-4-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001001037.23361-1-me@ttaylorr.com","subject":"[PATCH v3 4/6] doc: use modern \"`\"-style code fencing","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T00:10:35Z","receivedAt":"2017-10-01T00:10:57Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"\"'\"- (single-quote) styled code fencing is no longer considered modern\nwithin the \"Documentation/\" subtree.\n\nIn preparation for adding additional information to this section of\ngit-for-each-ref(1)'s documentation, update old-style code fencing to\nuse \"`\"-style fencing instead.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 6b38d9a22..b6492820b 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -209,12 +209,12 @@ and `date` to extract the named component.\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+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 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').\n+are obtained as `trailers` (or by using the historical alias\n+`contents:trailers`).\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329266","messageId":"20171001001037.23361-5-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001001037.23361-1-me@ttaylorr.com","subject":"[PATCH v3 5/6] ref-filter.c: use trailer_opts to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T00:10:36Z","receivedAt":"2017-10-01T00:11:03Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Fill trailer_opts with \"unfold\" and \"only\" to match the sub-arguments\ngiven to the \"%(trailers)\" atom. Then, let's use the filled trailer_opts\ninstance with 'format_trailers_from_commit' in order to format trailers\nin the desired manner.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 5 ++++-\n ref-filter.c | 32 +++++++++++++++++++++---------\n t/t6300-for-each-ref.sh | 40 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 67 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex b6492820b..a1d964182 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -214,7 +214,10 @@ blank line. The optional GPG signature is `contents:signature`. The\n first `N` lines of the message is obtained using `contents:lines=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`).\n+`contents:trailers`). Non-trailer lines from the trailer block can be omitted\n+with `trailers:only`. Whitespace-continuations can be removed from trailers so\n+that each trailer appears on a line by itself with its full content with\n+`trailers:unfold`. Both can be used together as `trailers:unfold,only`.\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 467c0279c..8573acbed 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -82,6 +82,7 @@ static struct used_atom {\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\tstruct process_trailer_options trailer_opts;\n \t\t\tunsigned int nlines;\n \t\t} contents;\n \t\tstruct {\n@@ -177,9 +178,23 @@ static void subject_atom_parser(struct used_atom *atom, const char *arg)\n \n static void trailers_atom_parser(struct used_atom *atom, const char *arg)\n {\n-\tif (arg)\n-\t\tdie(_(\"%%(trailers) does not take arguments\"));\n+\tstruct string_list params = STRING_LIST_INIT_DUP;\n+\tint i;\n+\n+\tif (arg) {\n+\t\tstring_list_split(¶ms, arg, ',', -1);\n+\t\tfor (i = 0; i < params.nr; i++) {\n+\t\t\tconst char *s = params.items[i].string;\n+\t\t\tif (!strcmp(s, \"unfold\"))\n+\t\t\t\tatom->u.contents.trailer_opts.unfold = 1;\n+\t\t\telse if (!strcmp(s, \"only\"))\n+\t\t\t\tatom->u.contents.trailer_opts.only_trailers = 1;\n+\t\t\telse\n+\t\t\t\tdie(_(\"unknown %%(trailers) argument: %s\"), s);\n+\t\t}\n+\t}\n \tatom->u.contents.option = C_TRAILERS;\n+\tstring_list_clear(¶ms, 0);\n }\n \n static void contents_atom_parser(struct used_atom *atom, const char *arg)\n@@ -1034,7 +1049,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tname++;\n \t\tif (strcmp(name, \"subject\") &&\n \t\t strcmp(name, \"body\") &&\n-\t\t strcmp(name, \"trailers\") &&\n+\t\t !starts_with(name, \"trailers\") &&\n \t\t !starts_with(name, \"contents\"))\n \t\t\tcontinue;\n \t\tif (!subpos)\n@@ -1059,13 +1074,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_TRAILERS) {\n-\t\t\tstruct trailer_info info;\n+\t\t\tstruct strbuf s = STRBUF_INIT;\n \n-\t\t\t/* Search for trailer info */\n-\t\t\ttrailer_info_get(&info, subpos);\n-\t\t\tv->s = xmemdupz(info.trailer_start,\n-\t\t\t\t\tinfo.trailer_end - info.trailer_start);\n-\t\t\ttrailer_info_release(&info);\n+\t\t\t/* Format the trailer info according to the trailer_opts given */\n+\t\t\tformat_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);\n+\n+\t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n \t}\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2a9fcf713..6538e7b90 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -597,6 +597,9 @@ Acked-by: A U Thor\n \n EOF\n \n+unfold () {\n+\tperl -0pe 's/\\n\\s+/ /'\n+}\n \n test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n@@ -610,6 +613,43 @@ test_expect_success 'set up trailers for next test' '\n \tEOF\n '\n \n+test_expect_success '%(trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+ test_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n+ test_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329267","messageId":"20171001001037.23361-6-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001001037.23361-1-me@ttaylorr.com","subject":"[PATCH v3 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T00:10:37Z","receivedAt":"2017-10-01T00:11:07Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The %(contents) atom takes a contents \"field\" as its argument. Since\n\"trailers\" is one of those fields, extend contents_atom_parser to parse\n\"trailers\"'s arguments when used through \"%(contents)\", like:\n\n %(contents:trailers:unfold,only)\n\nA caveat: trailers_atom_parser expects NULL when no arguments are given\n(see: `parse_ref_filter_atom`). To simulate this behavior without\nteaching trailers_atom_parser to accept strings with length zero,\nconditionally pass NULL to trailers_atom_parser if the arguments portion\nof the argument to %(contents) is empty.\n\nSigned-off-by: Taylor Blau \n---\n ref-filter.c | 6 ++++--\n t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 2 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8573acbed..b5747e969 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -207,8 +207,10 @@ static void contents_atom_parser(struct used_atom *atom, const char *arg)\n \t\tatom->u.contents.option = C_SIG;\n \telse if (!strcmp(arg, \"subject\"))\n \t\tatom->u.contents.option = C_SUB;\n-\telse if (!strcmp(arg, \"trailers\"))\n-\t\tatom->u.contents.option = C_TRAILERS;\n+\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n+\t\tskip_prefix(arg, \":\", &arg);\n+\t\ttrailers_atom_parser(atom, *arg ? NULL : arg);\n+\t}\n \telse if (skip_prefix(arg, \"lines=\", &arg)) {\n \t\tatom->u.contents.option = C_LINES;\n \t\tif (strtoul_ui(arg, 10, &atom->u.contents.nlines))\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 6538e7b90..8c960ec24 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -642,6 +642,35 @@ test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) and %(contents:trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%(trailers) rejects unknown trailers arguments' '\n \tcat >expect <<-EOF &&\n \tfatal: unknown %(trailers) argument: unsupported\n@@ -650,6 +679,14 @@ test_expect_success '%(trailers) rejects unknown trailers arguments' '\n test_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+ test_must_fail git for-each-ref --format=\"%(contents:trailers:unsupported)\" 2>actual &&\n+ test_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329276","messageId":"xmqqbmlrutsm.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20170930062238.87077-5-me@ttaylorr.com","subject":"Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-01T09:00:25Z","receivedAt":"2017-10-01T09:00:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 2a9fcf713..2bd0c5da7 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -597,6 +597,9 @@ Acked-by: A U Thor\n> \n> EOF\n> \n> +unfold () {\n> +\tperl -0pe 's/\\n\\s+/ /'\n> +}\n\nFor the purpose of the current shape of the test, the above might be\nsufficient, but the lack of \"/g\" at the end means that the script\nwill happily stop after unfolding just one line, which probably is\nnot what you intended.\n"},{"id":"329277","messageId":"xmqq7ewfutew.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20170930072143.h4mbd5c4hskvcrl3@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-01T09:08:39Z","receivedAt":"2017-10-01T09:08:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This patch didn't apply for me on top of the others. I get:\n>\n> Applying: ref-filter.c: use trailer_opts to format trailers\n> error: patch failed: ref-filter.c:178\n> error: ref-filter.c: patch does not apply\n> Patch failed at 0004 ref-filter.c: use trailer_opts to format trailers\n>\n> And then with \"am -3\":\n>\n> Applying: ref-filter.c: use trailer_opts to format trailers\n> error: sha1 information is lacking or useless (ref-filter.c).\n> error: could not build fake ancestor\n> Patch failed at 0004 ref-filter.c: use trailer_opts to format trailers\n>\n> Did it get corrupted in transit, or did you hand-edit it?\n\nAnd the corresponding patch in v3 seems to have the same issue.\n"},{"id":"329306","messageId":"20171001161725.GA66172@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"20171001000647.GA20767@D-10-157-251-166.dhcp4.washington.edu","subject":"Re: [PATCH v4 0/6] Support %(trailers) arguments in for-each-ref(1)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T16:17:25Z","receivedAt":"2017-10-01T16:17:34Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nAttached is the fourth revision of my patch-set \"Support %(trailers)\narguments in for-each-ref(1)\".\n\nIt includes the following changes since v3:\n\n * Teach unfold() to unfold multiple lines.\n\n * Rebase patches to apply on top of master.\n\nThanks in advance, and thanks for all of your help thus far :-).\n\n\n- Taylor\n"},{"id":"329307","messageId":"20171001161852.84571-2-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001161852.84571-1-me@ttaylorr.com","subject":"[PATCH v4 2/6] t6300: refactor %(trailers) tests","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T16:18:48Z","receivedAt":"2017-10-01T16:19:08Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"We currently have one test for %(trailers) in `git-for-each-ref(1)`,\nthrough \"%(contents:trailers)\". In preparation for more, let's add a few\nthings:\n\n - Move the commit creation step to its own test so that it can be\n re-used.\n\n - Add a non-trailer to the commit's trailers to test that non-trailers\n aren't shown using \"%(trailers:only)\".\n\n - Add a multi-line trailer to ensure that trailers are unfolded\n correctly using \"%(trailers:unfold)\".\n\nSigned-off-by: Taylor Blau \n---\n t/t6300-for-each-ref.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2274a4b73..39431908d 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -605,18 +605,25 @@ test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '\n cat >trailers <\n Signed-off-by: A U Thor \n+[ v2 updated patch description ]\n+Acked-by: A U Thor\n+ \n EOF\n \n-test_expect_success 'basic atom: head contents:trailers' '\n+\n+test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n \tgit add two &&\n-\tgit commit -F - <<-EOF &&\n+\tgit commit -F - <<-EOF\n \ttrailers: this commit message has trailers\n \n \tSome message contents\n \n \t$(cat trailers)\n \tEOF\n+'\n+\n+test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n \t# git for-each-ref ends with a blank line\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329308","messageId":"20171001161852.84571-3-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001161852.84571-1-me@ttaylorr.com","subject":"[PATCH v4 3/6] doc: 'trailers' is the preferred way to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T16:18:49Z","receivedAt":"2017-10-01T16:19:10Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The documentation makes reference to 'contents:trailers' as an example\nto dig the trailers out of a commit. 'trailers' is an unmentioned\nalternative, which is treated as an alias of 'contents:trailers'.\n\nSince 'trailers' is easier to type, prefer that as the designated way to\ndig out trailers information.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 66b4e0a40..323ce07de 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -217,7 +217,8 @@ 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 Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n-are obtained as 'contents:trailers'.\n+are obtained as 'trailers' (or by using the historical alias\n+'contents:trailers').\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329309","messageId":"20171001161852.84571-4-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001161852.84571-1-me@ttaylorr.com","subject":"[PATCH v4 4/6] doc: use modern \"`\"-style code fencing","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T16:18:50Z","receivedAt":"2017-10-01T16:19:11Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"\"'\"- (single-quote) styled code fencing is no longer considered modern\nwithin the \"Documentation/\" subtree.\n\nIn preparation for adding additional information to this section of\ngit-for-each-ref(1)'s documentation, update old-style code fencing to\nuse \"`\"-style fencing instead.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 323ce07de..1279b9733 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -213,12 +213,12 @@ and `date` to extract the named component.\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+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 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').\n+are obtained as `trailers` (or by using the historical alias\n+`contents:trailers`).\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329310","messageId":"20171001161852.84571-5-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001161852.84571-1-me@ttaylorr.com","subject":"[PATCH v4 5/6] ref-filter.c: use trailer_opts to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T16:18:51Z","receivedAt":"2017-10-01T16:19:14Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Fill trailer_opts with \"unfold\" and \"only\" to match the sub-arguments\ngiven to the \"%(trailers)\" atom. Then, let's use the filled trailer_opts\ninstance with 'format_trailers_from_commit' in order to format trailers\nin the desired manner.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 5 ++++-\n ref-filter.c | 32 +++++++++++++++++++++---------\n t/t6300-for-each-ref.sh | 40 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 67 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 1279b9733..4a2c851e6 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -218,7 +218,10 @@ blank line. The optional GPG signature is `contents:signature`. The\n first `N` lines of the message is obtained using `contents:lines=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`).\n+`contents:trailers`). Non-trailer lines from the trailer block can be omitted\n+with `trailers:only`. Whitespace-continuations can be removed from trailers so\n+that each trailer appears on a line by itself with its full content with\n+`trailers:unfold`. Both can be used together as `trailers:unfold,only`.\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\ndiff --git a/ref-filter.c b/ref-filter.c\nindex bc591f4f3..43ed10a5e 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -82,6 +82,7 @@ static struct used_atom {\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\tstruct process_trailer_options trailer_opts;\n \t\t\tunsigned int nlines;\n \t\t} contents;\n \t\tstruct {\n@@ -182,9 +183,23 @@ static void subject_atom_parser(const struct ref_format *format, struct used_ato\n \n static void trailers_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)\n {\n-\tif (arg)\n-\t\tdie(_(\"%%(trailers) does not take arguments\"));\n+\tstruct string_list params = STRING_LIST_INIT_DUP;\n+\tint i;\n+\n+\tif (arg) {\n+\t\tstring_list_split(¶ms, arg, ',', -1);\n+\t\tfor (i = 0; i < params.nr; i++) {\n+\t\t\tconst char *s = params.items[i].string;\n+\t\t\tif (!strcmp(s, \"unfold\"))\n+\t\t\t\tatom->u.contents.trailer_opts.unfold = 1;\n+\t\t\telse if (!strcmp(s, \"only\"))\n+\t\t\t\tatom->u.contents.trailer_opts.only_trailers = 1;\n+\t\t\telse\n+\t\t\t\tdie(_(\"unknown %%(trailers) argument: %s\"), s);\n+\t\t}\n+\t}\n \tatom->u.contents.option = C_TRAILERS;\n+\tstring_list_clear(¶ms, 0);\n }\n \n static void contents_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)\n@@ -1042,7 +1057,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tname++;\n \t\tif (strcmp(name, \"subject\") &&\n \t\t strcmp(name, \"body\") &&\n-\t\t strcmp(name, \"trailers\") &&\n+\t\t !starts_with(name, \"trailers\") &&\n \t\t !starts_with(name, \"contents\"))\n \t\t\tcontinue;\n \t\tif (!subpos)\n@@ -1067,13 +1082,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_TRAILERS) {\n-\t\t\tstruct trailer_info info;\n+\t\t\tstruct strbuf s = STRBUF_INIT;\n \n-\t\t\t/* Search for trailer info */\n-\t\t\ttrailer_info_get(&info, subpos);\n-\t\t\tv->s = xmemdupz(info.trailer_start,\n-\t\t\t\t\tinfo.trailer_end - info.trailer_start);\n-\t\t\ttrailer_info_release(&info);\n+\t\t\t/* Format the trailer info according to the trailer_opts given */\n+\t\t\tformat_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);\n+\n+\t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n \t}\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 39431908d..e4ce9c550 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -610,6 +610,9 @@ Acked-by: A U Thor\n \n EOF\n \n+unfold () {\n+\tperl -0pe 's/\\n\\s+/ /g'\n+}\n \n test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n@@ -623,6 +626,43 @@ test_expect_success 'set up trailers for next test' '\n \tEOF\n '\n \n+test_expect_success '%(trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+ test_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n+ test_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329311","messageId":"20171001161852.84571-6-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001161852.84571-1-me@ttaylorr.com","subject":"[PATCH v4 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T16:18:52Z","receivedAt":"2017-10-01T16:19:19Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The %(contents) atom takes a contents \"field\" as its argument. Since\n\"trailers\" is one of those fields, extend contents_atom_parser to parse\n\"trailers\"'s arguments when used through \"%(contents)\", like:\n\n %(contents:trailers:unfold,only)\n\nA caveat: trailers_atom_parser expects NULL when no arguments are given\n(see: `parse_ref_filter_atom`). To simulate this behavior without\nteaching trailers_atom_parser to accept strings with length zero,\nconditionally pass NULL to trailers_atom_parser if the arguments portion\nof the argument to %(contents) is empty.\n\nSigned-off-by: Taylor Blau \n---\n ref-filter.c | 6 ++++--\n t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 2 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 43ed10a5e..2c03c2bf5 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -212,8 +212,10 @@ static void contents_atom_parser(const struct ref_format *format, struct used_at\n \t\tatom->u.contents.option = C_SIG;\n \telse if (!strcmp(arg, \"subject\"))\n \t\tatom->u.contents.option = C_SUB;\n-\telse if (!strcmp(arg, \"trailers\"))\n-\t\tatom->u.contents.option = C_TRAILERS;\n+\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n+\t\tskip_prefix(arg, \":\", &arg);\n+\t\ttrailers_atom_parser(atom, *arg ? NULL : arg);\n+\t}\n \telse if (skip_prefix(arg, \"lines=\", &arg)) {\n \t\tatom->u.contents.option = C_LINES;\n \t\tif (strtoul_ui(arg, 10, &atom->u.contents.nlines))\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex e4ce9c550..ab95fe427 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -655,6 +655,35 @@ test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) and %(contents:trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%(trailers) rejects unknown trailers arguments' '\n \tcat >expect <<-EOF &&\n \tfatal: unknown %(trailers) argument: unsupported\n@@ -663,6 +692,14 @@ test_expect_success '%(trailers) rejects unknown trailers arguments' '\n test_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+ test_must_fail git for-each-ref --format=\"%(contents:trailers:unsupported)\" 2>actual &&\n+ test_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329312","messageId":"20171001161852.84571-1-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171001161725.GA66172@D-10-157-251-166.dhcp4.washington.edu","subject":"[PATCH v4 1/6] pretty.c: delimit \"%(trailers)\" arguments with \",\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-01T16:18:47Z","receivedAt":"2017-10-01T16:19:29Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation for adding consistent \"%(trailers)\" atom options to\n`git-for-each-ref(1)`'s \"--format\" argument, change \"%(trailers)\" in\npretty.c to separate sub-arguments with a \",\", instead of a \":\".\n\nMultiple sub-arguments are given either as \"%(trailers:unfold,only)\" or\n\"%(trailers:only,unfold)\".\n\nThis change disambiguates between \"top-level\" arguments, and arguments\ngiven to the trailers atom itself. It is consistent with the behavior of\n\"%(upstream)\" and \"%(push)\" atoms.\n\nSigned-off-by: Taylor Blau \n---\n pretty.c | 34 +++++++++++++++++++++++++++++-----\n t/t4205-log-pretty-formats.sh | 4 ++--\n 2 files changed, 31 insertions(+), 7 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 94eab5c89..eec128bc1 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1056,6 +1056,25 @@ static size_t parse_padding_placeholder(struct strbuf *sb,\n \treturn 0;\n }\n \n+static int match_placeholder_arg(const char *to_parse,\n+\t\t\t\tconst char *candidate,\n+\t\t\t\tconst char **end)\n+{\n+\tconst char *p;\n+\tif (!(skip_prefix(to_parse, candidate, &p)))\n+\t\treturn 0;\n+\tif (*p == ',') {\n+\t\t*end = p + 1;\n+\t\treturn 1;\n+\t}\n+\tif (*p == ')') {\n+\t\t*end = p;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+\n static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\t\tconst char *placeholder,\n \t\t\t\tvoid *context)\n@@ -1285,11 +1304,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \n \tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n-\t\twhile (*arg == ':') {\n-\t\t\tif (skip_prefix(arg, \":only\", &arg))\n-\t\t\t\topts.only_trailers = 1;\n-\t\t\telse if (skip_prefix(arg, \":unfold\", &arg))\n-\t\t\t\topts.unfold = 1;\n+\t\tif (*arg == ':') {\n+\t\t\targ++;\n+\t\t\tfor (;;) {\n+\t\t\t\tif (match_placeholder_arg(arg, \"only\", &arg))\n+\t\t\t\t\topts.only_trailers = 1;\n+\t\t\t\telse if (match_placeholder_arg(arg, \"unfold\", &arg))\n+\t\t\t\t\topts.unfold = 1;\n+\t\t\t\telse\n+\t\t\t\t\tbreak;\n+\t\t\t}\n \t\t}\n \t\tif (*arg == ')') {\n \t\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex ec5f53010..977472f53 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -588,8 +588,8 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '\n '\n \n test_expect_success ':only and :unfold work together' '\n-\tgit log --no-walk --pretty=\"%(trailers:only:unfold)\" >actual &&\n-\tgit log --no-walk --pretty=\"%(trailers:unfold:only)\" >reverse &&\n+\tgit log --no-walk --pretty=\"%(trailers:only,unfold)\" >actual &&\n+\tgit log --no-walk --pretty=\"%(trailers:unfold,only)\" >reverse &&\n \ttest_cmp actual reverse &&\n \t{\n \t\tgrep -v patch.description writes:\n\n> \"'\"- (single-quote) styled code fencing is no longer considered modern\n> within the \"Documentation/\" subtree.\n>\n> In preparation for adding additional information to this section of\n> git-for-each-ref(1)'s documentation, update old-style code fencing to\n> use \"`\"-style fencing instead.\n\nIs this just me who wants to do s/fenc/quot/g? Unless somebody\nobjects, I'd do so while queuing.\n\nThanks.\n\n>\n> Signed-off-by: Taylor Blau \n> ---\n> Documentation/git-for-each-ref.txt | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index 323ce07de..1279b9733 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -213,12 +213,12 @@ and `date` to extract the named component.\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> +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> 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').\n> +are obtained as `trailers` (or by using the historical alias\n> +`contents:trailers`).\n> \n> For sorting purposes, fields with numeric values sort in numeric order\n> (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n"},{"id":"329351","messageId":"20171002000630.GA63556@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"xmqqlgkutoc6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/6] doc: use modern \"`\"-style code fencing","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T00:06:30Z","receivedAt":"2017-10-02T00:06:38Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 02, 2017 at 08:55:53AM +0900, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > \"'\"- (single-quote) styled code fencing is no longer considered modern\n> > within the \"Documentation/\" subtree.\n> >\n> > In preparation for adding additional information to this section of\n> > git-for-each-ref(1)'s documentation, update old-style code fencing to\n> > use \"`\"-style fencing instead.\n>\n> Is this just me who wants to do s/fenc/quot/g? Unless somebody\n> objects, I'd do so while queuing.\n\nI don't object, I think that fencing is less appropriate than quoting.\nI couldn't find the term myself when writing this commit :-).\n\nI am happy to send out v5 of this patch series with this commit\nre-written, or you can change it while queuing. Whichever is easier for\nyou.\n\nThanks again.\n\n\n--\n- Taylor\n"},{"id":"329352","messageId":"xmqqbmlqtnll.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171001161852.84571-1-me@ttaylorr.com","subject":"Re: [PATCH v4 1/6] pretty.c: delimit \"%(trailers)\" arguments with \",\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T00:11:50Z","receivedAt":"2017-10-02T00:11:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> In preparation for adding consistent \"%(trailers)\" atom options to\n> `git-for-each-ref(1)`'s \"--format\" argument, change \"%(trailers)\" in\n> pretty.c to separate sub-arguments with a \",\", instead of a \":\".\n>\n> Multiple sub-arguments are given either as \"%(trailers:unfold,only)\" or\n> \"%(trailers:only,unfold)\".\n>\n> This change disambiguates between \"top-level\" arguments, and arguments\n> given to the trailers atom itself. It is consistent with the behavior of\n> \"%(upstream)\" and \"%(push)\" atoms.\n\nLooks good. \n\nIgnore the remainder unless you are interested in the recent \"make\nstyle\" discussion.\n\n>\n> Signed-off-by: Taylor Blau \n> ---\n> pretty.c | 34 +++++++++++++++++++++++++++++-----\n> t/t4205-log-pretty-formats.sh | 4 ++--\n> 2 files changed, 31 insertions(+), 7 deletions(-)\n>\n> diff --git a/pretty.c b/pretty.c\n> index 94eab5c89..eec128bc1 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -1056,6 +1056,25 @@ static size_t parse_padding_placeholder(struct strbuf *sb,\n> \treturn 0;\n> }\n> \n> +static int match_placeholder_arg(const char *to_parse,\n> +\t\t\t\tconst char *candidate,\n> +\t\t\t\tconst char **end)\n\n\"make style\" wants to format this like so:\n\nstatic int match_placeholder_arg(const char *to_parse, const char *candidate,\n\t\t\t\t const char **end)\n\nI think I can live with either versions ;-)\n\n> +{\n> +\tconst char *p;\n> +\tif (!(skip_prefix(to_parse, candidate, &p)))\n> +\t\treturn 0;\n> +\tif (*p == ',') {\n> +\t\t*end = p + 1;\n> +\t\treturn 1;\n> +\t}\n> +\tif (*p == ')') {\n> +\t\t*end = p;\n> +\t\treturn 1;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> +\n\n\"make style\" seems to be unhappy to see double blank here, and I\ntend to agree.\n\n> static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n> \t\t\t\tconst char *placeholder,\n> \t\t\t\tvoid *context)\n> @@ -1285,11 +1304,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n> \n> \tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n> \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n> -\t\twhile (*arg == ':') {\n> -\t\t\tif (skip_prefix(arg, \":only\", &arg))\n> -\t\t\t\topts.only_trailers = 1;\n> -\t\t\telse if (skip_prefix(arg, \":unfold\", &arg))\n> -\t\t\t\topts.unfold = 1;\n> +\t\tif (*arg == ':') {\n> +\t\t\targ++;\n> +\t\t\tfor (;;) {\n> +\t\t\t\tif (match_placeholder_arg(arg, \"only\", &arg))\n> +\t\t\t\t\topts.only_trailers = 1;\n> +\t\t\t\telse if (match_placeholder_arg(arg, \"unfold\", &arg))\n> +\t\t\t\t\topts.unfold = 1;\n> +\t\t\t\telse\n> +\t\t\t\t\tbreak;\n> +\t\t\t}\n> \t\t}\n> \t\tif (*arg == ')') {\n> \t\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\n> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n> index ec5f53010..977472f53 100755\n> --- a/t/t4205-log-pretty-formats.sh\n> +++ b/t/t4205-log-pretty-formats.sh\n> @@ -588,8 +588,8 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '\n> '\n> \n> test_expect_success ':only and :unfold work together' '\n> -\tgit log --no-walk --pretty=\"%(trailers:only:unfold)\" >actual &&\n> -\tgit log --no-walk --pretty=\"%(trailers:unfold:only)\" >reverse &&\n> +\tgit log --no-walk --pretty=\"%(trailers:only,unfold)\" >actual &&\n> +\tgit log --no-walk --pretty=\"%(trailers:unfold,only)\" >reverse &&\n> \ttest_cmp actual reverse &&\n> \t{\n> \t\tgrep -v patch.description writes:\n\n> +test_expect_success '%(trailers) rejects unknown trailers arguments' '\n> +\tcat >expect <<-EOF &&\n> +\tfatal: unknown %(trailers) argument: unsupported\n> +\tEOF\n> + test_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n> + test_cmp expect actual\n\nI'll have to remember to fix these two lines up before queuing.\nPlease yell at me if the version I'll push out later forgets to do\nso.\n\n\nThanks.\n"},{"id":"329355","messageId":"xmqqy3ous8nx.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171001161852.84571-6-me@ttaylorr.com","subject":"Re: [PATCH v4 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T00:19:46Z","receivedAt":"2017-10-02T00:20:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> The %(contents) atom takes a contents \"field\" as its argument. Since\n> \"trailers\" is one of those fields, extend contents_atom_parser to parse\n> \"trailers\"'s arguments when used through \"%(contents)\", like:\n>\n> %(contents:trailers:unfold,only)\n>\n> A caveat: trailers_atom_parser expects NULL when no arguments are given\n> (see: `parse_ref_filter_atom`). To simulate this behavior without\n> teaching trailers_atom_parser to accept strings with length zero,\n> conditionally pass NULL to trailers_atom_parser if the arguments portion\n> of the argument to %(contents) is empty.\n\nI got an impression during the earlier rounds' review discussion\nthat trailers_atom_parser() will happily accept an empty string and\nwork correctly, so this paragraph and the conditional *arg ? NULL :\narg were both unneeded.\n\n - arg == \"\" is not NULL, so we first do string_list_split() on ','\n - which yields an empty list i.e. params.nr == 0\n - we do not loop and leave atom->u.contents.trailer_opts untouched.\n - and we set u.contents.option to C_TRAILERS\n - and we clear ¶ms string list before we leave.\n\nwhich is exactly the same as what happens when arg == NULL. Sooo....\n\n>\n> Signed-off-by: Taylor Blau \n> ---\n> ref-filter.c | 6 ++++--\n> t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++\n> 2 files changed, 41 insertions(+), 2 deletions(-)\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 43ed10a5e..2c03c2bf5 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -212,8 +212,10 @@ static void contents_atom_parser(const struct ref_format *format, struct used_at\n> \t\tatom->u.contents.option = C_SIG;\n> \telse if (!strcmp(arg, \"subject\"))\n> \t\tatom->u.contents.option = C_SUB;\n> -\telse if (!strcmp(arg, \"trailers\"))\n> -\t\tatom->u.contents.option = C_TRAILERS;\n> +\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n> +\t\tskip_prefix(arg, \":\", &arg);\n> +\t\ttrailers_atom_parser(atom, *arg ? NULL : arg);\n> +\t}\n> \telse if (skip_prefix(arg, \"lines=\", &arg)) {\n\nMerge these two lines i.e.\n\n\t} else if (skip_prefix(arg, \"lines=\", ...) {\n\n> +test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '\n> +\tcat >expect <<-EOF &&\n> +\tfatal: unknown %(trailers) argument: unsupported\n> +\tEOF\n> + test_must_fail git for-each-ref --format=\"%(contents:trailers:unsupported)\" 2>actual &&\n> + test_cmp expect actual\n\nFunny indentation.\n"},{"id":"329357","messageId":"20171002003116.GA69902@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"20171001161725.GA66172@D-10-157-251-166.dhcp4.washington.edu","subject":"Re: [PATCH v5 0/6] Support %(trailers) arguments in for-each-ref(1)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T00:31:16Z","receivedAt":"2017-10-02T00:31:23Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nAttached is the fifth revision of my patch-set \"Support %(trailers)\narguments in for-each-ref(1)\".\n\nIt includes the following changes since v4:\n\n * Clarified \"ref-filter.c: parse trailers arguments with %(contents)\n atom\" to include reasoning for passing NULL as \"\" empty string in\n contents_atom_parser.\n\n * Changed instances \"fencing\" to \"quoting\" in \"doc: use modern\n \"`\"-style code fencing\".\n\n * Changed indentation in \"pretty.c: delimit \"%(trailers)\" arguments\n with \",\"\" to silence the output from `make style`.\n\n * Fix incorrect indentation in \"ref-filter.c: use trailer_opts to\n format trailers\".\n\nThanks again.\n\n\n- Taylor\n"},{"id":"329358","messageId":"20171002003304.77514-1-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002003116.GA69902@D-10-157-251-166.dhcp4.washington.edu","subject":"[PATCH v5 1/6] pretty.c: delimit \"%(trailers)\" arguments with \",\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T00:32:59Z","receivedAt":"2017-10-02T00:33:15Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation for adding consistent \"%(trailers)\" atom options to\n`git-for-each-ref(1)`'s \"--format\" argument, change \"%(trailers)\" in\npretty.c to separate sub-arguments with a \",\", instead of a \":\".\n\nMultiple sub-arguments are given either as \"%(trailers:unfold,only)\" or\n\"%(trailers:only,unfold)\".\n\nThis change disambiguates between \"top-level\" arguments, and arguments\ngiven to the trailers atom itself. It is consistent with the behavior of\n\"%(upstream)\" and \"%(push)\" atoms.\n\nSigned-off-by: Taylor Blau \n---\n pretty.c | 32 +++++++++++++++++++++++++++-----\n t/t4205-log-pretty-formats.sh | 4 ++--\n 2 files changed, 29 insertions(+), 7 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 94eab5c89..7500fe694 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1056,6 +1056,23 @@ static size_t parse_padding_placeholder(struct strbuf *sb,\n \treturn 0;\n }\n \n+static int match_placeholder_arg(const char *to_parse, const char *candidate,\n+\t\t\t\tconst char **end)\n+{\n+\tconst char *p;\n+\tif (!(skip_prefix(to_parse, candidate, &p)))\n+\t\treturn 0;\n+\tif (*p == ',') {\n+\t\t*end = p + 1;\n+\t\treturn 1;\n+\t}\n+\tif (*p == ')') {\n+\t\t*end = p;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\t\tconst char *placeholder,\n \t\t\t\tvoid *context)\n@@ -1285,11 +1302,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \n \tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n-\t\twhile (*arg == ':') {\n-\t\t\tif (skip_prefix(arg, \":only\", &arg))\n-\t\t\t\topts.only_trailers = 1;\n-\t\t\telse if (skip_prefix(arg, \":unfold\", &arg))\n-\t\t\t\topts.unfold = 1;\n+\t\tif (*arg == ':') {\n+\t\t\targ++;\n+\t\t\tfor (;;) {\n+\t\t\t\tif (match_placeholder_arg(arg, \"only\", &arg))\n+\t\t\t\t\topts.only_trailers = 1;\n+\t\t\t\telse if (match_placeholder_arg(arg, \"unfold\", &arg))\n+\t\t\t\t\topts.unfold = 1;\n+\t\t\t\telse\n+\t\t\t\t\tbreak;\n+\t\t\t}\n \t\t}\n \t\tif (*arg == ')') {\n \t\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex ec5f53010..977472f53 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -588,8 +588,8 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '\n '\n \n test_expect_success ':only and :unfold work together' '\n-\tgit log --no-walk --pretty=\"%(trailers:only:unfold)\" >actual &&\n-\tgit log --no-walk --pretty=\"%(trailers:unfold:only)\" >reverse &&\n+\tgit log --no-walk --pretty=\"%(trailers:only,unfold)\" >actual &&\n+\tgit log --no-walk --pretty=\"%(trailers:unfold,only)\" >reverse &&\n \ttest_cmp actual reverse &&\n \t{\n \t\tgrep -v patch.description \n---\n t/t6300-for-each-ref.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2274a4b73..39431908d 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -605,18 +605,25 @@ test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '\n cat >trailers <\n Signed-off-by: A U Thor \n+[ v2 updated patch description ]\n+Acked-by: A U Thor\n+ \n EOF\n \n-test_expect_success 'basic atom: head contents:trailers' '\n+\n+test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n \tgit add two &&\n-\tgit commit -F - <<-EOF &&\n+\tgit commit -F - <<-EOF\n \ttrailers: this commit message has trailers\n \n \tSome message contents\n \n \t$(cat trailers)\n \tEOF\n+'\n+\n+test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n \t# git for-each-ref ends with a blank line\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329360","messageId":"20171002003304.77514-3-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002003304.77514-1-me@ttaylorr.com","subject":"[PATCH v5 3/6] doc: 'trailers' is the preferred way to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T00:33:01Z","receivedAt":"2017-10-02T00:33:19Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The documentation makes reference to 'contents:trailers' as an example\nto dig the trailers out of a commit. 'trailers' is an unmentioned\nalternative, which is treated as an alias of 'contents:trailers'.\n\nSince 'trailers' is easier to type, prefer that as the designated way to\ndig out trailers information.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 66b4e0a40..323ce07de 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -217,7 +217,8 @@ 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 Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n-are obtained as 'contents:trailers'.\n+are obtained as 'trailers' (or by using the historical alias\n+'contents:trailers').\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329361","messageId":"20171002003304.77514-5-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002003304.77514-1-me@ttaylorr.com","subject":"[PATCH v5 5/6] ref-filter.c: use trailer_opts to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T00:33:03Z","receivedAt":"2017-10-02T00:33:23Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Fill trailer_opts with \"unfold\" and \"only\" to match the sub-arguments\ngiven to the \"%(trailers)\" atom. Then, let's use the filled trailer_opts\ninstance with 'format_trailers_from_commit' in order to format trailers\nin the desired manner.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 5 ++++-\n ref-filter.c | 32 +++++++++++++++++++++---------\n t/t6300-for-each-ref.sh | 40 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 67 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 1279b9733..4a2c851e6 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -218,7 +218,10 @@ blank line. The optional GPG signature is `contents:signature`. The\n first `N` lines of the message is obtained using `contents:lines=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`).\n+`contents:trailers`). Non-trailer lines from the trailer block can be omitted\n+with `trailers:only`. Whitespace-continuations can be removed from trailers so\n+that each trailer appears on a line by itself with its full content with\n+`trailers:unfold`. Both can be used together as `trailers:unfold,only`.\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\ndiff --git a/ref-filter.c b/ref-filter.c\nindex bc591f4f3..43ed10a5e 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -82,6 +82,7 @@ static struct used_atom {\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\tstruct process_trailer_options trailer_opts;\n \t\t\tunsigned int nlines;\n \t\t} contents;\n \t\tstruct {\n@@ -182,9 +183,23 @@ static void subject_atom_parser(const struct ref_format *format, struct used_ato\n \n static void trailers_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)\n {\n-\tif (arg)\n-\t\tdie(_(\"%%(trailers) does not take arguments\"));\n+\tstruct string_list params = STRING_LIST_INIT_DUP;\n+\tint i;\n+\n+\tif (arg) {\n+\t\tstring_list_split(¶ms, arg, ',', -1);\n+\t\tfor (i = 0; i < params.nr; i++) {\n+\t\t\tconst char *s = params.items[i].string;\n+\t\t\tif (!strcmp(s, \"unfold\"))\n+\t\t\t\tatom->u.contents.trailer_opts.unfold = 1;\n+\t\t\telse if (!strcmp(s, \"only\"))\n+\t\t\t\tatom->u.contents.trailer_opts.only_trailers = 1;\n+\t\t\telse\n+\t\t\t\tdie(_(\"unknown %%(trailers) argument: %s\"), s);\n+\t\t}\n+\t}\n \tatom->u.contents.option = C_TRAILERS;\n+\tstring_list_clear(¶ms, 0);\n }\n \n static void contents_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)\n@@ -1042,7 +1057,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tname++;\n \t\tif (strcmp(name, \"subject\") &&\n \t\t strcmp(name, \"body\") &&\n-\t\t strcmp(name, \"trailers\") &&\n+\t\t !starts_with(name, \"trailers\") &&\n \t\t !starts_with(name, \"contents\"))\n \t\t\tcontinue;\n \t\tif (!subpos)\n@@ -1067,13 +1082,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_TRAILERS) {\n-\t\t\tstruct trailer_info info;\n+\t\t\tstruct strbuf s = STRBUF_INIT;\n \n-\t\t\t/* Search for trailer info */\n-\t\t\ttrailer_info_get(&info, subpos);\n-\t\t\tv->s = xmemdupz(info.trailer_start,\n-\t\t\t\t\tinfo.trailer_end - info.trailer_start);\n-\t\t\ttrailer_info_release(&info);\n+\t\t\t/* Format the trailer info according to the trailer_opts given */\n+\t\t\tformat_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);\n+\n+\t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n \t}\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 39431908d..54e52d4e9 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -610,6 +610,9 @@ Acked-by: A U Thor\n \n EOF\n \n+unfold () {\n+\tperl -0pe 's/\\n\\s+/ /g'\n+}\n \n test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n@@ -623,6 +626,43 @@ test_expect_success 'set up trailers for next test' '\n \tEOF\n '\n \n+test_expect_success '%(trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+\ttest_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329362","messageId":"20171002003304.77514-4-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002003304.77514-1-me@ttaylorr.com","subject":"[PATCH v5 4/6] doc: use modern \"`\"-style code quoting","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T00:33:02Z","receivedAt":"2017-10-02T00:33:26Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"\"'\"- (single-quote) styled code quoting is no longer considered modern\nwithin the \"Documentation/\" subtree.\n\nIn preparation for adding additional information to this section of\ngit-for-each-ref(1)'s documentation, update old-style code quoting to\nuse \"`\"-style quoting instead.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 323ce07de..1279b9733 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -213,12 +213,12 @@ and `date` to extract the named component.\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+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 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').\n+are obtained as `trailers` (or by using the historical alias\n+`contents:trailers`).\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329363","messageId":"20171002003304.77514-6-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002003304.77514-1-me@ttaylorr.com","subject":"[PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T00:33:04Z","receivedAt":"2017-10-02T00:33:28Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The %(contents) atom takes a contents \"field\" as its argument. Since\n\"trailers\" is one of those fields, extend contents_atom_parser to parse\n\"trailers\"'s arguments when used through \"%(contents)\", like:\n\n %(contents:trailers:unfold,only)\n\nA caveat: trailers_atom_parser expects NULL when no arguments are given\n(see: `parse_ref_filter_atom`). This is because string_list_split (given\na maxsplit of -1) returns a 1-ary string_list* containing the given\nstring if the delimiter could not be found using `strchr`.\n\nTo simulate this behavior without teaching trailers_atom_parser to\naccept strings with length zero, conditionally pass NULL to\ntrailers_atom_parser if the arguments portion of the argument to\n%(contents) is empty.\n\nSigned-off-by: Taylor Blau \n---\n ref-filter.c | 7 ++++---\n t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 3 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 43ed10a5e..5b64de6ea 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -212,9 +212,10 @@ static void contents_atom_parser(const struct ref_format *format, struct used_at\n \t\tatom->u.contents.option = C_SIG;\n \telse if (!strcmp(arg, \"subject\"))\n \t\tatom->u.contents.option = C_SUB;\n-\telse if (!strcmp(arg, \"trailers\"))\n-\t\tatom->u.contents.option = C_TRAILERS;\n-\telse if (skip_prefix(arg, \"lines=\", &arg)) {\n+\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n+\t\tskip_prefix(arg, \":\", &arg);\n+\t\ttrailers_atom_parser(atom, *arg ? NULL : arg);\n+\t} else if (skip_prefix(arg, \"lines=\", &arg)) {\n \t\tatom->u.contents.option = C_LINES;\n \t\tif (strtoul_ui(arg, 10, &atom->u.contents.nlines))\n \t\t\tdie(_(\"positive value expected contents:lines=%s\"), arg);\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 54e52d4e9..872973b95 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -655,6 +655,35 @@ test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) and %(contents:trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%(trailers) rejects unknown trailers arguments' '\n \tcat >expect <<-EOF &&\n \tfatal: unknown %(trailers) argument: unsupported\n@@ -663,6 +692,14 @@ test_expect_success '%(trailers) rejects unknown trailers arguments' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+\ttest_must_fail git for-each-ref --format=\"%(contents:trailers:unsupported)\" 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329368","messageId":"xmqqtvziqqku.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171002000630.GA63556@D-10-157-251-166.dhcp4.washington.edu","subject":"Re: [PATCH v4 4/6] doc: use modern \"`\"-style code fencing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T01:35:45Z","receivedAt":"2017-10-02T01:35:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n>> Is this just me who wants to do s/fenc/quot/g? Unless somebody\n>> objects, I'd do so while queuing.\n>\n> I don't object, I think that fencing is less appropriate than quoting.\n> I couldn't find the term myself when writing this commit :-).\n>\n> I am happy to send out v5 of this patch series with this commit\n> re-written, or you can change it while queuing. Whichever is easier for\n> you.\n\nJust FYI, here is what I ended up with. I do not think this is\nabout \"modern\" vs \"old style\"; it is more about using the more\nappropriate mark-up and our desire has always been to use `literal`\nfor things that users need to type literally.\n\nI notice that I didn't remove 'modern' from the title in the versoin\nbelow, though.\n\nThanks.\n\ncommit a184d836bcf1a557520095a90b22edf659ee88f2\nAuthor: Taylor Blau \nDate: Sun Oct 1 09:18:50 2017 -0700\n\n doc: use modern \"``\"-style quoting for literal strings\n \n \"''\"-style quoting is not appropriate when quoting literal\n strings in the \"Documentation/\" subtree.\n \n In preparation for adding additional information to this section of\n git-for-each-ref(1)'s documentation, update them to use \"``\"\n instead.\n \n Signed-off-by: Taylor Blau \n Signed-off-by: Junio C Hamano \n"},{"id":"329379","messageId":"xmqqo9pqp442.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171002003304.77514-6-me@ttaylorr.com","subject":"Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T04:26:21Z","receivedAt":"2017-10-02T04:26:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> A caveat: trailers_atom_parser expects NULL when no arguments are given\n> (see: `parse_ref_filter_atom`). This is because string_list_split (given\n> a maxsplit of -1) returns a 1-ary string_list* containing the given\n> string if the delimiter could not be found using `strchr`.\n\nOK. That's unfortunate, but the solution here is far cleaner and\nsimpler than fixing string-list-split. Thanks for an updated\nexplanation here.\n\n"},{"id":"329380","messageId":"xmqqk20ep2yo.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171002003304.77514-6-me@ttaylorr.com","subject":"Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T04:51:11Z","receivedAt":"2017-10-02T04:51:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> @@ -212,9 +212,10 @@ static void contents_atom_parser(const struct ref_format *format, struct used_at\n> \t\tatom->u.contents.option = C_SIG;\n> \telse if (!strcmp(arg, \"subject\"))\n> \t\tatom->u.contents.option = C_SUB;\n> -\telse if (!strcmp(arg, \"trailers\"))\n> -\t\tatom->u.contents.option = C_TRAILERS;\n> -\telse if (skip_prefix(arg, \"lines=\", &arg)) {\n> +\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n> +\t\tskip_prefix(arg, \":\", &arg);\n> +\t\ttrailers_atom_parser(atom, *arg ? NULL : arg);\n\nA parameter for the call is missing. I think you want 'format'\nthere.\n"},{"id":"329381","messageId":"20171002045307.c66fctdduivicqqm@sigill.intra.peff.net","threadId":"46862","inReplyTo":"xmqqtvziqqku.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/6] doc: use modern \"`\"-style code fencing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-02T04:53:07Z","receivedAt":"2017-10-02T04:53:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 02, 2017 at 10:35:45AM +0900, Junio C Hamano wrote:\n\n> Taylor Blau writes:\n> \n> >> Is this just me who wants to do s/fenc/quot/g? Unless somebody\n> >> objects, I'd do so while queuing.\n> >\n> > I don't object, I think that fencing is less appropriate than quoting.\n> > I couldn't find the term myself when writing this commit :-).\n> >\n> > I am happy to send out v5 of this patch series with this commit\n> > re-written, or you can change it while queuing. Whichever is easier for\n> > you.\n> \n> Just FYI, here is what I ended up with. I do not think this is\n> about \"modern\" vs \"old style\"; it is more about using the more\n> appropriate mark-up and our desire has always been to use `literal`\n> for things that users need to type literally.\n\nI think it was my earlier comment that led to the modern/old notion. A\nlot of the old code does use single-quotes haphazardly. I'm not sure if\nit's \"old style\" or just \"we were less careful\" then. Either way, the\npatch looks good to me.\n\nI also think fencing isn't the right word; that word implies to me a\nmulti-line block like\n\n---------------------------\nthis is fenced\n---------------------------\n\n-Peff\n"},{"id":"329382","messageId":"20171002050005.6hwzky3p3rmrdmgc@sigill.intra.peff.net","threadId":"46862","inReplyTo":"xmqqbmlqtnll.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 1/6] pretty.c: delimit \"%(trailers)\" arguments with \",\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-02T05:00:05Z","receivedAt":"2017-10-02T05:00:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 02, 2017 at 09:11:50AM +0900, Junio C Hamano wrote:\n\n> > diff --git a/pretty.c b/pretty.c\n> > index 94eab5c89..eec128bc1 100644\n> > --- a/pretty.c\n> > +++ b/pretty.c\n> > @@ -1056,6 +1056,25 @@ static size_t parse_padding_placeholder(struct strbuf *sb,\n> > \treturn 0;\n> > }\n> > \n> > +static int match_placeholder_arg(const char *to_parse,\n> > +\t\t\t\tconst char *candidate,\n> > +\t\t\t\tconst char **end)\n> \n> \"make style\" wants to format this like so:\n> \n> static int match_placeholder_arg(const char *to_parse, const char *candidate,\n> \t\t\t\t const char **end)\n> \n> I think I can live with either versions ;-)\n\nI can live with either, too, though I do prefer the 3-line form (which\nis unsurprising since it comes from what I wrote in an earlier thread).\nI don't think it conveys a huge amount of meaning here, but there are\ndefinitely times when I would tweak the wrapping of long argument lists\nto convey meaning. E.g.:\n\n void some_function(struct foo *foo,\n const char *buf, size_t len,\n\t\t unsigned flags);\n\nrather than\n\n void some_function(struct foo *foo, const char *buf,\n size_t len, unsigned flags);\n\nIt's a little sad to be beholden to a tool that would mindlessly drop\nthat grouping. But at the same time, that tool may be the lesser evil if\nit saves time adjusting (and reviewing) style. I'll continue to observe\nbefore forming more inclusions. Thanks for reporting on your \"make\nstyle\" experiments.\n\n-Peff\n"},{"id":"329383","messageId":"20171002050351.imuntcy3xnq4wxux@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20171002003304.77514-6-me@ttaylorr.com","subject":"Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-02T05:03:51Z","receivedAt":"2017-10-02T05:03:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 01, 2017 at 05:33:04PM -0700, Taylor Blau wrote:\n\n> The %(contents) atom takes a contents \"field\" as its argument. Since\n> \"trailers\" is one of those fields, extend contents_atom_parser to parse\n> \"trailers\"'s arguments when used through \"%(contents)\", like:\n> \n> %(contents:trailers:unfold,only)\n> \n> A caveat: trailers_atom_parser expects NULL when no arguments are given\n> (see: `parse_ref_filter_atom`). This is because string_list_split (given\n> a maxsplit of -1) returns a 1-ary string_list* containing the given\n> string if the delimiter could not be found using `strchr`.\n> \n> To simulate this behavior without teaching trailers_atom_parser to\n> accept strings with length zero, conditionally pass NULL to\n> trailers_atom_parser if the arguments portion of the argument to\n> %(contents) is empty.\n\nDoh, that string_list behavior is what I was missing in my earlier\ncomments. I agree this is probably the best way of doing it. I'm tempted\nto say that parse_ref_filter_atom() should do a similar thing. Right now\nwe've got:\n\n $ git for-each-ref --format='%(refname)' | wc\n 2206 2206 79929\n $ git for-each-ref --format='%(refname:short)' | wc\n 2206 2206 53622\n $ git for-each-ref --format='%(refname:)' | wc\n fatal: unrecognized %(refname:) argument:\n 0 0 0\n\nwhich seems a bit unfriendly. As we're on v6 here, I don't want to\nsuggest it as part of this series. But maybe a #leftoverbits candidate,\nif others agree it's a good direction.\n\n-Peff\n"},{"id":"329384","messageId":"20171002050507.i27l2iulfbskmnpx@sigill.intra.peff.net","threadId":"46862","inReplyTo":"xmqqbmlrutsm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-02T05:05:07Z","receivedAt":"2017-10-02T05:05:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 01, 2017 at 06:00:25PM +0900, Junio C Hamano wrote:\n\n> Taylor Blau writes:\n> \n> > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> > index 2a9fcf713..2bd0c5da7 100755\n> > --- a/t/t6300-for-each-ref.sh\n> > +++ b/t/t6300-for-each-ref.sh\n> > @@ -597,6 +597,9 @@ Acked-by: A U Thor\n> > \n> > EOF\n> > \n> > +unfold () {\n> > +\tperl -0pe 's/\\n\\s+/ /'\n> > +}\n> \n> For the purpose of the current shape of the test, the above might be\n> sufficient, but the lack of \"/g\" at the end means that the script\n> will happily stop after unfolding just one line, which probably is\n> not what you intended.\n\nThis is indirectly my fault, since this was copied from my t4205\nversion. It might be worth fixing while we're thinking about it, as it's\na potential trap for future changes.\n\n-Peff\n"},{"id":"329387","messageId":"20171002051137.GA5566@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"20171002050507.i27l2iulfbskmnpx@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:11:37Z","receivedAt":"2017-10-02T05:11:43Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 02, 2017 at 01:05:07AM -0400, Jeff King wrote:\n> On Sun, Oct 01, 2017 at 06:00:25PM +0900, Junio C Hamano wrote:\n>\n> > Taylor Blau writes:\n> >\n> > > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> > > index 2a9fcf713..2bd0c5da7 100755\n> > > --- a/t/t6300-for-each-ref.sh\n> > > +++ b/t/t6300-for-each-ref.sh\n> > > @@ -597,6 +597,9 @@ Acked-by: A U Thor\n> > > \n> > > EOF\n> > >\n> > > +unfold () {\n> > > +\tperl -0pe 's/\\n\\s+/ /'\n> > > +}\n> >\n> > For the purpose of the current shape of the test, the above might be\n> > sufficient, but the lack of \"/g\" at the end means that the script\n> > will happily stop after unfolding just one line, which probably is\n> > not what you intended.\n>\n> This is indirectly my fault, since this was copied from my t4205\n> version. It might be worth fixing while we're thinking about it, as it's\n> a potential trap for future changes.\n\nI agree. I'll include a fix for this as an additional commit in v6.\n\n--\n- Taylor\n"},{"id":"329388","messageId":"20171002051216.GB5566@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"20171002050351.imuntcy3xnq4wxux@sigill.intra.peff.net","subject":"Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:12:16Z","receivedAt":"2017-10-02T05:12:23Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 02, 2017 at 01:03:51AM -0400, Jeff King wrote:\n> On Sun, Oct 01, 2017 at 05:33:04PM -0700, Taylor Blau wrote:\n>\n> > The %(contents) atom takes a contents \"field\" as its argument. Since\n> > \"trailers\" is one of those fields, extend contents_atom_parser to parse\n> > \"trailers\"'s arguments when used through \"%(contents)\", like:\n> >\n> > %(contents:trailers:unfold,only)\n> >\n> > A caveat: trailers_atom_parser expects NULL when no arguments are given\n> > (see: `parse_ref_filter_atom`). This is because string_list_split (given\n> > a maxsplit of -1) returns a 1-ary string_list* containing the given\n> > string if the delimiter could not be found using `strchr`.\n> >\n> > To simulate this behavior without teaching trailers_atom_parser to\n> > accept strings with length zero, conditionally pass NULL to\n> > trailers_atom_parser if the arguments portion of the argument to\n> > %(contents) is empty.\n>\n> Doh, that string_list behavior is what I was missing in my earlier\n> comments. I agree this is probably the best way of doing it. I'm tempted\n> to say that parse_ref_filter_atom() should do a similar thing. Right now\n> we've got:\n>\n> $ git for-each-ref --format='%(refname)' | wc\n> 2206 2206 79929\n> $ git for-each-ref --format='%(refname:short)' | wc\n> 2206 2206 53622\n> $ git for-each-ref --format='%(refname:)' | wc\n> fatal: unrecognized %(refname:) argument:\n> 0 0 0\n>\n> which seems a bit unfriendly. As we're on v6 here, I don't want to\n> suggest it as part of this series. But maybe a #leftoverbits candidate,\n> if others agree it's a good direction.\n\nI think #leftoverbits is fine here, but I think addressing this before\n2.15 is reasonable. I can take a look at this in a future patch series.\n\n--\n- Taylor\n"},{"id":"329389","messageId":"20171002051336.GA7266@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"xmqqk20ep2yo.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:13:36Z","receivedAt":"2017-10-02T05:13:42Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 02, 2017 at 01:51:11PM +0900, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > @@ -212,9 +212,10 @@ static void contents_atom_parser(const struct ref_format *format, struct used_at\n> > \t\tatom->u.contents.option = C_SIG;\n> > \telse if (!strcmp(arg, \"subject\"))\n> > \t\tatom->u.contents.option = C_SUB;\n> > -\telse if (!strcmp(arg, \"trailers\"))\n> > -\t\tatom->u.contents.option = C_TRAILERS;\n> > -\telse if (skip_prefix(arg, \"lines=\", &arg)) {\n> > +\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n> > +\t\tskip_prefix(arg, \":\", &arg);\n> > +\t\ttrailers_atom_parser(atom, *arg ? NULL : arg);\n>\n> A parameter for the call is missing. I think you want 'format'\n> there.\n\nThanks for pointing this out. I have fixed this in v6, which I am\nsending out shortly. I think that this revision should be ready for\nqueueing.\n\n--\n- Taylor\n"},{"id":"329390","messageId":"20171002051417.hrdbxowk7eoczpcd@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20171002051216.GB5566@D-10-157-251-166.dhcp4.washington.edu","subject":"Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-02T05:14:17Z","receivedAt":"2017-10-02T05:14:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 01, 2017 at 10:12:16PM -0700, Taylor Blau wrote:\n\n> > Doh, that string_list behavior is what I was missing in my earlier\n> > comments. I agree this is probably the best way of doing it. I'm tempted\n> > to say that parse_ref_filter_atom() should do a similar thing. Right now\n> > we've got:\n> >\n> > $ git for-each-ref --format='%(refname)' | wc\n> > 2206 2206 79929\n> > $ git for-each-ref --format='%(refname:short)' | wc\n> > 2206 2206 53622\n> > $ git for-each-ref --format='%(refname:)' | wc\n> > fatal: unrecognized %(refname:) argument:\n> > 0 0 0\n> >\n> > which seems a bit unfriendly. As we're on v6 here, I don't want to\n> > suggest it as part of this series. But maybe a #leftoverbits candidate,\n> > if others agree it's a good direction.\n> \n> I think #leftoverbits is fine here, but I think addressing this before\n> 2.15 is reasonable. I can take a look at this in a future patch series.\n\nI think it would be to just do it as a standalone patch immediately. I\njust didn't want to hold your series hostage to a potential\ndisagreement.\n\n-Peff\n"},{"id":"329392","messageId":"20171002052326.GA10729@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"20171002003116.GA69902@D-10-157-251-166.dhcp4.washington.edu","subject":"Re: [PATCH v6 0/7] Support %(trailers) arguments in for-each-ref(1)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:23:26Z","receivedAt":"2017-10-02T05:23:34Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nAttached is the sixth revision of my patch-set \"Support %(trailers)\narguments in for-each-ref(1)\".\n\nIn includes the following changes since v5:\n\n * Added an additional patch to change t4205 to harden `unfold()`\n against multi-line trailer folding.\n\n * Added a missing parameter call in ref-filter.c to\n `trailers_atom_parser` through `contents_atom_parser`.\n\nI believe that this version of the series should be ready for queueing.\n\nI am going to address Peff's follow-up for teaching\n`parse_ref_atom_filter` to accept \"empty\" argument lists as\n`%(refname:)` in a follow-up series later this evening.\n\nThanks again :-).\n\n\n--\n- Taylor\n"},{"id":"329393","messageId":"20171002052524.12627-1-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002052326.GA10729@D-10-157-251-166.dhcp4.washington.edu","subject":"[PATCH v6 1/7] pretty.c: delimit \"%(trailers)\" arguments with \",\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:25:18Z","receivedAt":"2017-10-02T05:25:34Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation for adding consistent \"%(trailers)\" atom options to\n`git-for-each-ref(1)`'s \"--format\" argument, change \"%(trailers)\" in\npretty.c to separate sub-arguments with a \",\", instead of a \":\".\n\nMultiple sub-arguments are given either as \"%(trailers:unfold,only)\" or\n\"%(trailers:only,unfold)\".\n\nThis change disambiguates between \"top-level\" arguments, and arguments\ngiven to the trailers atom itself. It is consistent with the behavior of\n\"%(upstream)\" and \"%(push)\" atoms.\n\nSigned-off-by: Taylor Blau \n---\n pretty.c | 32 +++++++++++++++++++++++++++-----\n t/t4205-log-pretty-formats.sh | 4 ++--\n 2 files changed, 29 insertions(+), 7 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 94eab5c89..7500fe694 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1056,6 +1056,23 @@ static size_t parse_padding_placeholder(struct strbuf *sb,\n \treturn 0;\n }\n \n+static int match_placeholder_arg(const char *to_parse, const char *candidate,\n+\t\t\t\tconst char **end)\n+{\n+\tconst char *p;\n+\tif (!(skip_prefix(to_parse, candidate, &p)))\n+\t\treturn 0;\n+\tif (*p == ',') {\n+\t\t*end = p + 1;\n+\t\treturn 1;\n+\t}\n+\tif (*p == ')') {\n+\t\t*end = p;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\t\tconst char *placeholder,\n \t\t\t\tvoid *context)\n@@ -1285,11 +1302,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \n \tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n-\t\twhile (*arg == ':') {\n-\t\t\tif (skip_prefix(arg, \":only\", &arg))\n-\t\t\t\topts.only_trailers = 1;\n-\t\t\telse if (skip_prefix(arg, \":unfold\", &arg))\n-\t\t\t\topts.unfold = 1;\n+\t\tif (*arg == ':') {\n+\t\t\targ++;\n+\t\t\tfor (;;) {\n+\t\t\t\tif (match_placeholder_arg(arg, \"only\", &arg))\n+\t\t\t\t\topts.only_trailers = 1;\n+\t\t\t\telse if (match_placeholder_arg(arg, \"unfold\", &arg))\n+\t\t\t\t\topts.unfold = 1;\n+\t\t\t\telse\n+\t\t\t\t\tbreak;\n+\t\t\t}\n \t\t}\n \t\tif (*arg == ')') {\n \t\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex ec5f53010..977472f53 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -588,8 +588,8 @@ test_expect_success '%(trailers:unfold) unfolds trailers' '\n '\n \n test_expect_success ':only and :unfold work together' '\n-\tgit log --no-walk --pretty=\"%(trailers:only:unfold)\" >actual &&\n-\tgit log --no-walk --pretty=\"%(trailers:unfold:only)\" >reverse &&\n+\tgit log --no-walk --pretty=\"%(trailers:only,unfold)\" >actual &&\n+\tgit log --no-walk --pretty=\"%(trailers:unfold,only)\" >reverse &&\n \ttest_cmp actual reverse &&\n \t{\n \t\tgrep -v patch.description \n---\n t/t4205-log-pretty-formats.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 977472f53..591f35daa 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -544,7 +544,7 @@ Signed-off-by: A U Thor\n EOF\n \n unfold () {\n-\tperl -0pe 's/\\n\\s+/ /'\n+\tperl -0pe 's/\\n\\s+/ /g'\n }\n \n test_expect_success 'set up trailer tests' '\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329395","messageId":"20171002052524.12627-3-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002052524.12627-1-me@ttaylorr.com","subject":"[PATCH v6 3/7] doc: 'trailers' is the preferred way to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:25:20Z","receivedAt":"2017-10-02T05:25:38Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The documentation makes reference to 'contents:trailers' as an example\nto dig the trailers out of a commit. 'trailers' is an unmentioned\nalternative, which is treated as an alias of 'contents:trailers'.\n\nSince 'trailers' is easier to type, prefer that as the designated way to\ndig out trailers information.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 66b4e0a40..323ce07de 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -217,7 +217,8 @@ 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 Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n-are obtained as 'contents:trailers'.\n+are obtained as 'trailers' (or by using the historical alias\n+'contents:trailers').\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329396","messageId":"20171002052524.12627-4-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002052524.12627-1-me@ttaylorr.com","subject":"[PATCH v6 4/7] doc: use modern \"`\"-style code quoting","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:25:21Z","receivedAt":"2017-10-02T05:25:43Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"\"'\"- (single-quote) styled code quoting is no longer considered modern\nwithin the \"Documentation/\" subtree.\n\nIn preparation for adding additional information to this section of\ngit-for-each-ref(1)'s documentation, update old-style code quoting to\nuse \"`\"-style quoting instead.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 323ce07de..1279b9733 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -213,12 +213,12 @@ and `date` to extract the named component.\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+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 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').\n+are obtained as `trailers` (or by using the historical alias\n+`contents:trailers`).\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329397","messageId":"20171002052524.12627-6-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002052524.12627-1-me@ttaylorr.com","subject":"[PATCH v6 6/7] ref-filter.c: use trailer_opts to format trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:25:23Z","receivedAt":"2017-10-02T05:25:47Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Fill trailer_opts with \"unfold\" and \"only\" to match the sub-arguments\ngiven to the \"%(trailers)\" atom. Then, let's use the filled trailer_opts\ninstance with 'format_trailers_from_commit' in order to format trailers\nin the desired manner.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-for-each-ref.txt | 5 ++++-\n ref-filter.c | 32 +++++++++++++++++++++---------\n t/t6300-for-each-ref.sh | 40 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 67 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 1279b9733..4a2c851e6 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -218,7 +218,10 @@ blank line. The optional GPG signature is `contents:signature`. The\n first `N` lines of the message is obtained using `contents:lines=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`).\n+`contents:trailers`). Non-trailer lines from the trailer block can be omitted\n+with `trailers:only`. Whitespace-continuations can be removed from trailers so\n+that each trailer appears on a line by itself with its full content with\n+`trailers:unfold`. Both can be used together as `trailers:unfold,only`.\n \n For sorting purposes, fields with numeric values sort in numeric order\n (`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).\ndiff --git a/ref-filter.c b/ref-filter.c\nindex bc591f4f3..43ed10a5e 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -82,6 +82,7 @@ static struct used_atom {\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\tstruct process_trailer_options trailer_opts;\n \t\t\tunsigned int nlines;\n \t\t} contents;\n \t\tstruct {\n@@ -182,9 +183,23 @@ static void subject_atom_parser(const struct ref_format *format, struct used_ato\n \n static void trailers_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)\n {\n-\tif (arg)\n-\t\tdie(_(\"%%(trailers) does not take arguments\"));\n+\tstruct string_list params = STRING_LIST_INIT_DUP;\n+\tint i;\n+\n+\tif (arg) {\n+\t\tstring_list_split(¶ms, arg, ',', -1);\n+\t\tfor (i = 0; i < params.nr; i++) {\n+\t\t\tconst char *s = params.items[i].string;\n+\t\t\tif (!strcmp(s, \"unfold\"))\n+\t\t\t\tatom->u.contents.trailer_opts.unfold = 1;\n+\t\t\telse if (!strcmp(s, \"only\"))\n+\t\t\t\tatom->u.contents.trailer_opts.only_trailers = 1;\n+\t\t\telse\n+\t\t\t\tdie(_(\"unknown %%(trailers) argument: %s\"), s);\n+\t\t}\n+\t}\n \tatom->u.contents.option = C_TRAILERS;\n+\tstring_list_clear(¶ms, 0);\n }\n \n static void contents_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)\n@@ -1042,7 +1057,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tname++;\n \t\tif (strcmp(name, \"subject\") &&\n \t\t strcmp(name, \"body\") &&\n-\t\t strcmp(name, \"trailers\") &&\n+\t\t !starts_with(name, \"trailers\") &&\n \t\t !starts_with(name, \"contents\"))\n \t\t\tcontinue;\n \t\tif (!subpos)\n@@ -1067,13 +1082,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_TRAILERS) {\n-\t\t\tstruct trailer_info info;\n+\t\t\tstruct strbuf s = STRBUF_INIT;\n \n-\t\t\t/* Search for trailer info */\n-\t\t\ttrailer_info_get(&info, subpos);\n-\t\t\tv->s = xmemdupz(info.trailer_start,\n-\t\t\t\t\tinfo.trailer_end - info.trailer_start);\n-\t\t\ttrailer_info_release(&info);\n+\t\t\t/* Format the trailer info according to the trailer_opts given */\n+\t\t\tformat_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);\n+\n+\t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n \t}\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 39431908d..54e52d4e9 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -610,6 +610,9 @@ Acked-by: A U Thor\n \n EOF\n \n+unfold () {\n+\tperl -0pe 's/\\n\\s+/ /g'\n+}\n \n test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n@@ -623,6 +626,43 @@ test_expect_success 'set up trailers for next test' '\n \tEOF\n '\n \n+test_expect_success '%(trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+\ttest_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329398","messageId":"20171002052524.12627-7-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002052524.12627-1-me@ttaylorr.com","subject":"[PATCH v6 7/7] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:25:24Z","receivedAt":"2017-10-02T05:25:48Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The %(contents) atom takes a contents \"field\" as its argument. Since\n\"trailers\" is one of those fields, extend contents_atom_parser to parse\n\"trailers\"'s arguments when used through \"%(contents)\", like:\n\n %(contents:trailers:unfold,only)\n\nA caveat: trailers_atom_parser expects NULL when no arguments are given\n(see: `parse_ref_filter_atom`). This is because string_list_split (given\na maxsplit of -1) returns a 1-ary string_list* containing the given\nstring if the delimiter could not be found using `strchr`.\n\nTo simulate this behavior without teaching trailers_atom_parser to\naccept strings with length zero, conditionally pass NULL to\ntrailers_atom_parser if the arguments portion of the argument to\n%(contents) is empty.\n\nSigned-off-by: Taylor Blau \n---\n ref-filter.c | 7 ++++---\n t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 3 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 43ed10a5e..6c26b4733 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -212,9 +212,10 @@ static void contents_atom_parser(const struct ref_format *format, struct used_at\n \t\tatom->u.contents.option = C_SIG;\n \telse if (!strcmp(arg, \"subject\"))\n \t\tatom->u.contents.option = C_SUB;\n-\telse if (!strcmp(arg, \"trailers\"))\n-\t\tatom->u.contents.option = C_TRAILERS;\n-\telse if (skip_prefix(arg, \"lines=\", &arg)) {\n+\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n+\t\tskip_prefix(arg, \":\", &arg);\n+\t\ttrailers_atom_parser(format, atom, *arg ? NULL : arg);\n+\t} else if (skip_prefix(arg, \"lines=\", &arg)) {\n \t\tatom->u.contents.option = C_LINES;\n \t\tif (strtoul_ui(arg, 10, &atom->u.contents.nlines))\n \t\t\tdie(_(\"positive value expected contents:lines=%s\"), arg);\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 54e52d4e9..872973b95 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -655,6 +655,35 @@ test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers:unfold) unfolds trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold)\" refs/heads/master >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) shows only \"key: value\" trailers' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only)\" refs/heads/master >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(contents:trailers:only) and %(contents:trailers:unfold) work together' '\n+\tgit for-each-ref --format=\"%(contents:trailers:only,unfold)\" refs/heads/master >actual &&\n+\tgit for-each-ref --format=\"%(contents:trailers:unfold,only)\" refs/heads/master >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%(trailers) rejects unknown trailers arguments' '\n \tcat >expect <<-EOF &&\n \tfatal: unknown %(trailers) argument: unsupported\n@@ -663,6 +692,14 @@ test_expect_success '%(trailers) rejects unknown trailers arguments' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '\n+\tcat >expect <<-EOF &&\n+\tfatal: unknown %(trailers) argument: unsupported\n+\tEOF\n+\ttest_must_fail git for-each-ref --format=\"%(contents:trailers:unsupported)\" 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329399","messageId":"20171002052524.12627-5-me@ttaylorr.com","threadId":"46862","inReplyTo":"20171002052524.12627-1-me@ttaylorr.com","subject":"[PATCH v6 5/7] t6300: refactor %(trailers) tests","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T05:25:22Z","receivedAt":"2017-10-02T05:25:51Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"We currently have one test for %(trailers) in `git-for-each-ref(1)`,\nthrough \"%(contents:trailers)\". In preparation for more, let's add a few\nthings:\n\n - Move the commit creation step to its own test so that it can be\n re-used.\n\n - Add a non-trailer to the commit's trailers to test that non-trailers\n aren't shown using \"%(trailers:only)\".\n\n - Add a multi-line trailer to ensure that trailers are unfolded\n correctly using \"%(trailers:unfold)\".\n\nSigned-off-by: Taylor Blau \n---\n t/t6300-for-each-ref.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2274a4b73..39431908d 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -605,18 +605,25 @@ test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '\n cat >trailers <\n Signed-off-by: A U Thor \n+[ v2 updated patch description ]\n+Acked-by: A U Thor\n+ \n EOF\n \n-test_expect_success 'basic atom: head contents:trailers' '\n+\n+test_expect_success 'set up trailers for next test' '\n \techo \"Some contents\" > two &&\n \tgit add two &&\n-\tgit commit -F - <<-EOF &&\n+\tgit commit -F - <<-EOF\n \ttrailers: this commit message has trailers\n \n \tSome message contents\n \n \t$(cat trailers)\n \tEOF\n+'\n+\n+test_expect_success 'basic atom: head contents:trailers' '\n \tgit for-each-ref --format=\"%(contents:trailers)\" refs/heads/master >actual &&\n \tsanitize_pgp actual.clean &&\n \t# git for-each-ref ends with a blank line\n-- \n2.14.1.145.gb3622a4ee\n\n"},{"id":"329417","messageId":"20171002065100.cjawz6pvxjj2w5nr@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20171002052524.12627-7-me@ttaylorr.com","subject":"Re: [PATCH v6 7/7] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-02T06:51:00Z","receivedAt":"2017-10-02T06:51:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 01, 2017 at 10:25:24PM -0700, Taylor Blau wrote:\n\n> The %(contents) atom takes a contents \"field\" as its argument. Since\n> \"trailers\" is one of those fields, extend contents_atom_parser to parse\n> \"trailers\"'s arguments when used through \"%(contents)\", like:\n> \n> %(contents:trailers:unfold,only)\n> \n> A caveat: trailers_atom_parser expects NULL when no arguments are given\n> (see: `parse_ref_filter_atom`). This is because string_list_split (given\n> a maxsplit of -1) returns a 1-ary string_list* containing the given\n> string if the delimiter could not be found using `strchr`.\n> \n> To simulate this behavior without teaching trailers_atom_parser to\n> accept strings with length zero, conditionally pass NULL to\n> trailers_atom_parser if the arguments portion of the argument to\n> %(contents) is empty.\n> \n> Signed-off-by: Taylor Blau \n> ---\n> ref-filter.c | 7 ++++---\n> t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++\n> 2 files changed, 41 insertions(+), 3 deletions(-)\n\nThis patch seems to fail a bunch of tests in t6300.\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 43ed10a5e..6c26b4733 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -212,9 +212,10 @@ static void contents_atom_parser(const struct ref_format *format, struct used_at\n> \t\tatom->u.contents.option = C_SIG;\n> \telse if (!strcmp(arg, \"subject\"))\n> \t\tatom->u.contents.option = C_SUB;\n> -\telse if (!strcmp(arg, \"trailers\"))\n> -\t\tatom->u.contents.option = C_TRAILERS;\n> -\telse if (skip_prefix(arg, \"lines=\", &arg)) {\n> +\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n> +\t\tskip_prefix(arg, \":\", &arg);\n> +\t\ttrailers_atom_parser(format, atom, *arg ? NULL : arg);\n\nI think your logic is flipped. You want \"*arg ? arg : NULL\";\n\n-Peff\n"},{"id":"329418","messageId":"20171002065621.ob3icxhlkefpsm4v@sigill.intra.peff.net","threadId":"46862","inReplyTo":"20171002052326.GA10729@D-10-157-251-166.dhcp4.washington.edu","subject":"Re: [PATCH v6 0/7] Support %(trailers) arguments in for-each-ref(1)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-02T06:56:21Z","receivedAt":"2017-10-02T06:56:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 01, 2017 at 10:23:26PM -0700, Taylor Blau wrote:\n\n> Attached is the sixth revision of my patch-set \"Support %(trailers)\n> arguments in for-each-ref(1)\".\n> \n> In includes the following changes since v5:\n> \n> * Added an additional patch to change t4205 to harden `unfold()`\n> against multi-line trailer folding.\n> \n> * Added a missing parameter call in ref-filter.c to\n> `trailers_atom_parser` through `contents_atom_parser`.\n> \n> I believe that this version of the series should be ready for queueing.\n\nThis looks good to me, modulo the flipped logic in the final patch.\n\nSince that's the only thing I noticed, let's hold off on a reroll for\nthe moment to see if there are any more comments (and I won't be\nsurprised if Junio just picks it up with the tweak, but we'll see).\n\nPlease do make sure that \"make test\" runs clean before posting (I\nusually run it on each commit to catch any \"oops, we broke this and then\nrefixed it\" in the middle of the series, too).\n\n-Peff\n"},{"id":"329420","messageId":"xmqq7eweotup.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171002052326.GA10729@D-10-157-251-166.dhcp4.washington.edu","subject":"Re: [PATCH v6 0/7] Support %(trailers) arguments in for-each-ref(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T08:07:58Z","receivedAt":"2017-10-02T08:08:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> Attached is the sixth revision of my patch-set \"Support %(trailers)\n> arguments in for-each-ref(1)\".\n>\n> In includes the following changes since v5:\n>\n> * Added an additional patch to change t4205 to harden `unfold()`\n> against multi-line trailer folding.\n>\n> * Added a missing parameter call in ref-filter.c to\n> `trailers_atom_parser` through `contents_atom_parser`.\n>\n> I believe that this version of the series should be ready for queueing.\n>\n> I am going to address Peff's follow-up for teaching\n> `parse_ref_atom_filter` to accept \"empty\" argument lists as\n> `%(refname:)` in a follow-up series later this evening.\n>\n> Thanks again :-).\n\nThanks. t6300 seems to show that %(contents:trailers:unfold) does\nnot quite work, among other things.\n\n https://travis-ci.org/git/git/jobs/282126607#L3658\n\nI didn't have a chance to look into it myself.\n"},{"id":"329426","messageId":"xmqqa819op12.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171002065100.cjawz6pvxjj2w5nr@sigill.intra.peff.net","subject":"Re: [PATCH v6 7/7] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T09:52:09Z","receivedAt":"2017-10-02T09:52:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> \t\tatom->u.contents.option = C_SUB;\n>> -\telse if (!strcmp(arg, \"trailers\"))\n>> -\t\tatom->u.contents.option = C_TRAILERS;\n>> -\telse if (skip_prefix(arg, \"lines=\", &arg)) {\n>> +\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n>> +\t\tskip_prefix(arg, \":\", &arg);\n>> +\t\ttrailers_atom_parser(format, atom, *arg ? NULL : arg);\n>\n> I think your logic is flipped. You want \"*arg ? arg : NULL\";\n\nAhh, I was blind. Will fix locally and push the results out.\n\nThanks.\n\n"},{"id":"329437","messageId":"xmqqy3otn3uj.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"xmqq7eweotup.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v6 0/7] Support %(trailers) arguments in for-each-ref(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T12:15:00Z","receivedAt":"2017-10-02T12:15:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Thanks. t6300 seems to show that %(contents:trailers:unfold) does\n> not quite work, among other things.\n>\n> https://travis-ci.org/git/git/jobs/282126607#L3658\n>\n> I didn't have a chance to look into it myself.\n\nPeff's \"oops, your logic is backwards\" fixes the above failure.\n\nWe also need this on top to pass the gettext-poison build.\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 872973b954..3bdfa02559 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -685,19 +685,21 @@ test_expect_success '%(contents:trailers:only) and %(contents:trailers:unfold) w\n '\n \n test_expect_success '%(trailers) rejects unknown trailers arguments' '\n+\t# error message cannot be checked under i18n\n \tcat >expect <<-EOF &&\n \tfatal: unknown %(trailers) argument: unsupported\n \tEOF\n \ttest_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n-\ttest_cmp expect actual\n+\ttest_i18ncmp expect actual\n '\n \n test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '\n+\t# error message cannot be checked under i18n\n \tcat >expect <<-EOF &&\n \tfatal: unknown %(trailers) argument: unsupported\n \tEOF\n \ttest_must_fail git for-each-ref --format=\"%(contents:trailers:unsupported)\" 2>actual &&\n-\ttest_cmp expect actual\n+\ttest_i18ncmp expect actual\n '\n \n test_expect_success 'basic atom: head contents:trailers' '\n"},{"id":"329457","messageId":"20171002154916.GA39723@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"20171002065100.cjawz6pvxjj2w5nr@sigill.intra.peff.net","subject":"Re: [PATCH v6 7/7] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T15:49:16Z","receivedAt":"2017-10-02T15:49:25Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 02, 2017 at 02:51:00AM -0400, Jeff King wrote:\n> > diff --git a/ref-filter.c b/ref-filter.c\n> > index 43ed10a5e..6c26b4733 100644\n> > --- a/ref-filter.c\n> > +++ b/ref-filter.c\n> > @@ -212,9 +212,10 @@ static void contents_atom_parser(const struct ref_format *format, struct used_at\n> > \t\tatom->u.contents.option = C_SIG;\n> > \telse if (!strcmp(arg, \"subject\"))\n> > \t\tatom->u.contents.option = C_SUB;\n> > -\telse if (!strcmp(arg, \"trailers\"))\n> > -\t\tatom->u.contents.option = C_TRAILERS;\n> > -\telse if (skip_prefix(arg, \"lines=\", &arg)) {\n> > +\telse if (skip_prefix(arg, \"trailers\", &arg)) {\n> > +\t\tskip_prefix(arg, \":\", &arg);\n> > +\t\ttrailers_atom_parser(format, atom, *arg ? NULL : arg);\n>\n> I think your logic is flipped. You want \"*arg ? arg : NULL\";\n\nThank you for pointing this out. I should have run \"make test\" on this\npatch set (or, as you suggested, `git rebase -x \"make test\" HEAD~7`)\nbefore sending it out. I appreciate you catching my mistake, and I'll\nmake sure to run \"make test\" more diligently in the future :-).\n\nIt sounds like Junio picked this up while queueing.\n\n--\n- Taylor\n"},{"id":"329458","messageId":"20171002160718.GB39723@D-10-157-251-166.dhcp4.washington.edu","threadId":"46862","inReplyTo":"xmqqy3otn3uj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v6 0/7] Support %(trailers) arguments in for-each-ref(1)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2017-10-02T16:07:18Z","receivedAt":"2017-10-02T16:07:29Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 02, 2017 at 09:15:00PM +0900, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n> > Thanks. t6300 seems to show that %(contents:trailers:unfold) does\n> > not quite work, among other things.\n> >\n> > https://travis-ci.org/git/git/jobs/282126607#L3658\n> >\n> > I didn't have a chance to look into it myself.\n>\n> Peff's \"oops, your logic is backwards\" fixes the above failure.\n>\n> We also need this on top to pass the gettext-poison build.\n>\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 872973b954..3bdfa02559 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -685,19 +685,21 @@ test_expect_success '%(contents:trailers:only) and %(contents:trailers:unfold) w\n> '\n>\n> test_expect_success '%(trailers) rejects unknown trailers arguments' '\n> +\t# error message cannot be checked under i18n\n> \tcat >expect <<-EOF &&\n> \tfatal: unknown %(trailers) argument: unsupported\n> \tEOF\n> \ttest_must_fail git for-each-ref --format=\"%(trailers:unsupported)\" 2>actual &&\n> -\ttest_cmp expect actual\n> +\ttest_i18ncmp expect actual\n> '\n>\n> test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '\n> +\t# error message cannot be checked under i18n\n> \tcat >expect <<-EOF &&\n> \tfatal: unknown %(trailers) argument: unsupported\n> \tEOF\n> \ttest_must_fail git for-each-ref --format=\"%(contents:trailers:unsupported)\" 2>actual &&\n> -\ttest_cmp expect actual\n> +\ttest_i18ncmp expect actual\n> '\n>\n> test_expect_success 'basic atom: head contents:trailers' '\n\nThank you for pointing this out. I am not well-versed on gettext, and\nits usage within Git. I am happy to send out v7 of this series, or you\ncan apply these changes in queueing. Whichever is easier :-).\n\n--\n- Taylor\n"},{"id":"329496","messageId":"xmqqr2ulm7wy.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171002154916.GA39723@D-10-157-251-166.dhcp4.washington.edu","subject":"Re: [PATCH v6 7/7] ref-filter.c: parse trailers arguments with %(contents) atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T23:44:45Z","receivedAt":"2017-10-02T23:44:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> Thank you for pointing this out. I should have run \"make test\" on this\n> patch set (or, as you suggested, `git rebase -x \"make test\" HEAD~7`)\n> before sending it out. I appreciate you catching my mistake, and I'll\n> make sure to run \"make test\" more diligently in the future :-).\n>\n> It sounds like Junio picked this up while queueing.\n\n\"Yup\" in the sense that \"you do not have to worry about it unless\nthere are other things you want to fix by rerolling\" but I didn't\nspot it myself and noticed the breakage; I tweaked it only after\nPeff pointed out what is wrong.\n"},{"id":"329531","messageId":"xmqqtvzgiw9i.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171002065621.ob3icxhlkefpsm4v@sigill.intra.peff.net","subject":"Re: [PATCH v6 0/7] Support %(trailers) arguments in for-each-ref(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T06:24:41Z","receivedAt":"2017-10-03T06:24:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Since that's the only thing I noticed, let's hold off on a reroll for\n> the moment to see if there are any more comments (and I won't be\n> surprised if Junio just picks it up with the tweak, but we'll see).\n>\n> Please do make sure that \"make test\" runs clean before posting (I\n> usually run it on each commit to catch any \"oops, we broke this and then\n> refixed it\" in the middle of the series, too).\n\nOK, I think fixes for both the \"flipped\" and the \"gettext-poison\"\nbreakages are solved locally in my tree already, so I guess this is\nready to be merged to 'next'.\n"},{"id":"329534","messageId":"20171003063605.laenmn66swwyac6z@sigill.intra.peff.net","threadId":"46862","inReplyTo":"xmqqtvzgiw9i.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v6 0/7] Support %(trailers) arguments in for-each-ref(1)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T06:36:06Z","receivedAt":"2017-10-03T06:36:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 03, 2017 at 03:24:41PM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Since that's the only thing I noticed, let's hold off on a reroll for\n> > the moment to see if there are any more comments (and I won't be\n> > surprised if Junio just picks it up with the tweak, but we'll see).\n> >\n> > Please do make sure that \"make test\" runs clean before posting (I\n> > usually run it on each commit to catch any \"oops, we broke this and then\n> > refixed it\" in the middle of the series, too).\n> \n> OK, I think fixes for both the \"flipped\" and the \"gettext-poison\"\n> breakages are solved locally in my tree already, so I guess this is\n> ready to be merged to 'next'.\n\nYes, I think so.\n\nOut of curiosity, do you frequently test with GETTEXT_POISON, or did you\njust guess at a potential problem after reading the tests? Proper use\nof test_i18ncmp is definitely something we ought to be looking for\nduring review, but I confess it's something I often miss.\n\n-Peff\n"},{"id":"329535","messageId":"xmqqpoa4ivj6.fsf@gitster.mtv.corp.google.com","threadId":"46862","inReplyTo":"20171003063605.laenmn66swwyac6z@sigill.intra.peff.net","subject":"Re: [PATCH v6 0/7] Support %(trailers) arguments in for-each-ref(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T06:40:29Z","receivedAt":"2017-10-03T06:40:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Out of curiosity, do you frequently test with GETTEXT_POISON, or did you\n> just guess at a potential problem after reading the tests? Proper use\n> of test_i18ncmp is definitely something we ought to be looking for\n> during review, but I confess it's something I often miss.\n\nI learn of breakages after the fact, when Travis reports.\n"}]}