git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers

From
Jeff King <peff@peff.net>
Date
Sep 30, 2017, 07:21 UTC
Message-ID
<20170930072143.h4mbd5c4hskvcrl3@sigill.intra.peff.net>
In-Reply-To
<20170930062238.87077-5-me@ttaylorr.com>
On Fri, Sep 29, 2017 at 11:22:37PM -0700, Taylor Blau wrote:
> In preparation to support additional sub-arguments given to the "%(trailers)"
> atom, use 'format_trailers_from_commit' in order to format trailers in the
> desired manner.

This isn't just in preparation, is it? It looks like the options are here (which I think is fine, but the commit message probably needs updated).

Show 6 quoted lines
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  Documentation/git-for-each-ref.txt |  6 +++++-
>  ref-filter.c                       | 31 ++++++++++++++++++++---------
>  t/t6300-for-each-ref.sh            | 40 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+), 10 deletions(-)
This patch didn't apply for me on top of the others. I get:
  Applying: ref-filter.c: use trailer_opts to format trailers
  error: patch failed: ref-filter.c:178
  error: ref-filter.c: patch does not apply
  Patch failed at 0004 ref-filter.c: use trailer_opts to format trailers
And then with "am -3":
  Applying: ref-filter.c: use trailer_opts to format trailers
  error: sha1 information is lacking or useless (ref-filter.c).
  error: could not build fake ancestor
  Patch failed at 0004 ref-filter.c: use trailer_opts to format trailers
Did it get corrupted in transit, or did you hand-edit it?
Show 14 quoted lines
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 03e187a10..b7325a25d 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -213,7 +213,11 @@ line is 'contents:body', where body is all of the lines after the first
>  blank line.  The optional GPG signature is `contents:signature`.  The
>  first `N` lines of the message is obtained using `contents:lines=N`.
>  Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]
> -are obtained as 'contents:trailers'.
> +are obtained as 'contents:trailers'. Non-trailer lines from the trailer block
> +can be omitted with 'trailers:only'. Whitespace-continuations can be removed
> +from trailers so that each trailer appears on a line by itself with its full
> +content with 'trailers:unfold'. Both can be used together as
> +'trailers:unfold,only'.

I know you copied the single-quote formatting from the existing line, but this may be a good opportunity to switch to backticks, which is what we usually prefer these days for literal phrases.

Show 28 quoted lines
> diff --git a/ref-filter.c b/ref-filter.c
> index 84f14093c..8573acbed 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -178,9 +178,23 @@ static void subject_atom_parser(struct used_atom *atom, const char *arg)
>  
>  static void trailers_atom_parser(struct used_atom *atom, const char *arg)
>  {
> -	if (arg)
> -		die(_("%%(trailers) does not take arguments"));
> +	struct string_list params = STRING_LIST_INIT_DUP;
> +	int i;
> +
> +	if (arg) {
> +		string_list_split(&params, arg, ',', -1);
> +		for (i = 0; i < params.nr; i++) {
> +			const char *s = params.items[i].string;
> +			if (!strcmp(s, "unfold"))
> +				atom->u.contents.trailer_opts.unfold = 1;
> +			else if (!strcmp(s, "only"))
> +				atom->u.contents.trailer_opts.only_trailers = 1;
> +			else
> +				die(_("unknown %%(trailers) argument: %s"), s);
> +		}
> +	}
>  	atom->u.contents.option = C_TRAILERS;
> +	string_list_clear(&params, 0);
>  }

This looks good (and so much nicer than the contortions from pretty.c). The error behavior matches what we currently do for %(align), which makes sense.

The trailer_opts should be zero-initialized to start with due to us calling memset on the whole used_atom struct.

Show 26 quoted lines
>  static void contents_atom_parser(struct used_atom *atom, const char *arg)
> @@ -1035,7 +1049,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
>  			name++;
>  		if (strcmp(name, "subject") &&
>  		    strcmp(name, "body") &&
> -		    strcmp(name, "trailers") &&
> +		    !starts_with(name, "trailers") &&
>  		    !starts_with(name, "contents"))
>  			continue;
>  		if (!subpos)
> @@ -1060,13 +1074,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
>  			append_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);
>  			v->s = strbuf_detach(&s, NULL);
>  		} else if (atom->u.contents.option == C_TRAILERS) {
> -			struct trailer_info info;
> +			struct strbuf s = STRBUF_INIT;
>  
> -			/* Search for trailer info */
> -			trailer_info_get(&info, subpos);
> -			v->s = xmemdupz(info.trailer_start,
> -					info.trailer_end - info.trailer_start);
> -			trailer_info_release(&info);
> +			/* Format the trailer info according to the trailer_opts given */
> +			format_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);
> +
> +			v->s = strbuf_detach(&s, NULL);
And this all looks straightforward and correct.
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 2a9fcf713..2bd0c5da7 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh

The tests are basically an adaptation of the ones from 58311c66fd (pretty: support normalization options for %(trailers), 2017-08-15), which make sense.

One thing I did notice:
Show 12 quoted lines
> @@ -610,6 +613,43 @@ test_expect_success 'set up trailers for next test' '
>  	EOF
>  '
>  
> +test_expect_success '%(trailers:unfold) unfolds trailers' '
> +  git for-each-ref --format="%(trailers:unfold)" refs/heads/master >actual &&
> +  {
> +    unfold <trailers
> +    echo
> +  } >expect &&
> +  test_cmp expect actual
> +'
This looks like two-space indentation, when it should be a tab.
Show 19 quoted lines
> +test_expect_success '%(trailers:only) shows only "key: value" trailers' '
> +	git for-each-ref --format="%(trailers:only)" refs/heads/master >actual &&
> +	{
> +		grep -v patch.description <trailers &&
> +		echo
> +	} >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '%(trailers:only) and %(trailers:unfold) work together' '
> +	git for-each-ref --format="%(trailers:only,unfold)" refs/heads/master >actual &&
> +	git for-each-ref --format="%(trailers:unfold,only)" refs/heads/master >reverse &&
> +	test_cmp actual reverse &&
> +	{
> +		grep -v patch.description <trailers | unfold &&
> +		echo
> +	} >expect &&
> +	test_cmp expect actual
> +'
These ones are tabs. GOod.
Show 7 quoted lines
> +test_expect_success '%(trailers) rejects unknown trailers arguments' '
> +	cat >expect <<-EOF &&
> +	fatal: unknown %(trailers) argument: unsupported
> +	EOF
> +  test_must_fail git for-each-ref --format="%(trailers:unsupported)" 2>actual &&
> +  test_cmp expect actual
> +'
But this one is mixed. :)
-Peff
Previous: Taylor BlauNext: Junio C Hamano
Message 9 of 78 in “Support %(trailers) arguments in for-each-ref(1)”
  1. 0/5 Support %(trailers) arguments in for-each-ref(1)Taylor Blau, Sep 30, 2017
  2. 1/5 pretty.c: delimit "%(trailers)" arguments with ","Taylor Blau, Sep 30, 2017
  3. Jeff KingSep 30, 2017
  4. 2/5 t6300: refactor %(trailers) testsTaylor Blau, Sep 30, 2017
  5. Jeff KingSep 30, 2017
  6. 3/5 ref-filter.c: add trailer options to used_atomTaylor Blau, Sep 30, 2017
  7. Jeff KingSep 30, 2017
  8. 4/5 ref-filter.c: use trailer_opts to format trailersTaylor Blau, Sep 30, 2017
  9. Jeff KingSep 30, 2017
  10. Junio C HamanoOct 1, 2017
  11. Junio C HamanoOct 1, 2017
  12. Jeff KingOct 2, 2017
  13. Taylor BlauOct 2, 2017
  14. 5/5 ref-filter.c: parse trailers arguments with %(contents) atomTaylor Blau, Sep 30, 2017
  15. Jeff KingSep 30, 2017
  16. Jeff KingSep 30, 2017
  17. 0/6 Support %(trailers) arguments in for-each-ref(1)Taylor Blau, Sep 30, 2017
  18. 1/6 pretty.c: delimit "%(trailers)" arguments with ","Taylor Blau, Sep 30, 2017
  19. 2/6 t6300: refactor %(trailers) testsTaylor Blau, Sep 30, 2017
  20. 4/6 doc: use modern "`"-style code fencingTaylor Blau, Sep 30, 2017
  21. 5/6 ref-filter.c: use trailer_opts to format trailersTaylor Blau, Sep 30, 2017
  22. 3/6 doc: 'trailers' is the preferred way to format trailersTaylor Blau, Sep 30, 2017
  23. 6/6 ref-filter.c: parse trailers arguments with %(contents) atomTaylor Blau, Sep 30, 2017
  24. Taylor BlauOct 1, 2017
  25. 1/6 pretty.c: delimit "%(trailers)" arguments with ","Taylor Blau, Oct 1, 2017
  26. 2/6 t6300: refactor %(trailers) testsTaylor Blau, Oct 1, 2017
  27. 3/6 doc: 'trailers' is the preferred way to format trailersTaylor Blau, Oct 1, 2017
  28. 4/6 doc: use modern "`"-style code fencingTaylor Blau, Oct 1, 2017
  29. 5/6 ref-filter.c: use trailer_opts to format trailersTaylor Blau, Oct 1, 2017
  30. 6/6 ref-filter.c: parse trailers arguments with %(contents) atomTaylor Blau, Oct 1, 2017
  31. Taylor BlauOct 1, 2017
  32. 1/6 pretty.c: delimit "%(trailers)" arguments with ","Taylor Blau, Oct 1, 2017
  33. 2/6 t6300: refactor %(trailers) testsTaylor Blau, Oct 1, 2017
  34. Junio C HamanoOct 2, 2017
  35. 3/6 doc: 'trailers' is the preferred way to format trailersTaylor Blau, Oct 1, 2017
  36. 4/6 doc: use modern "`"-style code fencingTaylor Blau, Oct 1, 2017
  37. Junio C HamanoOct 1, 2017
  38. Taylor BlauOct 2, 2017
  39. Junio C HamanoOct 2, 2017
  40. Jeff KingOct 2, 2017
  41. 5/6 ref-filter.c: use trailer_opts to format trailersTaylor Blau, Oct 1, 2017
  42. Junio C HamanoOct 2, 2017
  43. 6/6 ref-filter.c: parse trailers arguments with %(contents) atomTaylor Blau, Oct 1, 2017
  44. Junio C HamanoOct 2, 2017
  45. Junio C HamanoOct 2, 2017
  46. Jeff KingOct 2, 2017
  47. Taylor BlauOct 2, 2017
  48. 1/6 pretty.c: delimit "%(trailers)" arguments with ","Taylor Blau, Oct 2, 2017
  49. 2/6 t6300: refactor %(trailers) testsTaylor Blau, Oct 2, 2017
  50. 3/6 doc: 'trailers' is the preferred way to format trailersTaylor Blau, Oct 2, 2017
  51. 5/6 ref-filter.c: use trailer_opts to format trailersTaylor Blau, Oct 2, 2017
  52. 4/6 doc: use modern "`"-style code quotingTaylor Blau, Oct 2, 2017
  53. 6/6 ref-filter.c: parse trailers arguments with %(contents) atomTaylor Blau, Oct 2, 2017
  54. Junio C HamanoOct 2, 2017
  55. Junio C HamanoOct 2, 2017
  56. Taylor BlauOct 2, 2017
  57. Jeff KingOct 2, 2017
  58. Taylor BlauOct 2, 2017
  59. Jeff KingOct 2, 2017
  60. Taylor BlauOct 2, 2017
  61. 1/7 pretty.c: delimit "%(trailers)" arguments with ","Taylor Blau, Oct 2, 2017
  62. 2/7 t4205: unfold across multiple linesTaylor Blau, Oct 2, 2017
  63. 3/7 doc: 'trailers' is the preferred way to format trailersTaylor Blau, Oct 2, 2017
  64. 4/7 doc: use modern "`"-style code quotingTaylor Blau, Oct 2, 2017
  65. 6/7 ref-filter.c: use trailer_opts to format trailersTaylor Blau, Oct 2, 2017
  66. 7/7 ref-filter.c: parse trailers arguments with %(contents) atomTaylor Blau, Oct 2, 2017
  67. Jeff KingOct 2, 2017
  68. Junio C HamanoOct 2, 2017
  69. Taylor BlauOct 2, 2017
  70. Junio C HamanoOct 2, 2017
  71. 5/7 t6300: refactor %(trailers) testsTaylor Blau, Oct 2, 2017
  72. Jeff KingOct 2, 2017
  73. Junio C HamanoOct 3, 2017
  74. Jeff KingOct 3, 2017
  75. Junio C HamanoOct 3, 2017
  76. Junio C HamanoOct 2, 2017
  77. Junio C HamanoOct 2, 2017
  78. Taylor BlauOct 2, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.