On Wed, Oct 04, 2017 at 05:46:21PM +0900, Junio C Hamano wrote:
> Jeff King <p...@peff.net> writes:
>
> >> - pretty.c: delimit "%(trailers)" arguments with ","
> >>
> >> "git for-each-ref --format=..." learned a new format element,
> >> %(trailers), to show only the commit log trailer part of the log
> >> message.
> >>
> >> Will merge to 'next'.
> >
> > I think we want the first patch of this series to graduate before v2.15,
> > even if the rest doesn't make it. It tweaks a new syntax introduced
> > earlier in this cycle by jk/trailers-parse. If we ship without the
> > tweak, then we'll have to support the colon-delimiter to remain
> > backwards-compatible.
>
> Yeah, thanks for reminding me. I actually was hoping that this will
> prove to be stable enough by the time -rc1 gets tagged, but yes, the
> bottom one looks innocuous/safe enough and should be fast-tracked to
> 'master' soonish.
It may make sense to send my other series to 'master' as well
("ref-filter.c: pass empty-string as NULL to atom parsers").
The series you're discussing here adds support for "empty" sub-agruments
(via: --format=%(contents:trailers:)), but Peff points out that this is
not a consistent user experience:
> Doh, that string_list behavior is what I was missing in my earlier
> comments. I agree this is probably the best way of doing it. I'm tempted
> to say that parse_ref_filter_atom() should do a similar thing. Right now
> we've got:
>
> $ git for-each-ref --format='%(refname)' | wc
> 2206 2206 79929
> $ git for-each-ref --format='%(refname:short)' | wc
> 2206 2206 53622
> $ git for-each-ref --format='%(refname:)' | wc
> fatal: unrecognized %(refname:) argument:
> 0 0 0
"ref-filter.c: pass empty-string as NULL to atom parsers" makes this
behavior of allowing empty sub-argument atom formats in
git-for-each-ref(1) consistently OK.
To avoid introducing a case where %(atom:) is sometimes allowed and
sometimes not, I would recommend that both of these patches be applied
to master at the same time.
--
- Taylor
