Re: What's cooking in git.git (Oct 2017, #01; Wed, 4)

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 4, 2017, 18:07 UTC
Message-ID: <20171004180737.GA90224@D-173-250-188-46.dhcp4.washington.edu>
In-Reply-To: <xmqqr2ujb8rm.fsf@gitster.mtv.corp.google.com>

On Wed, Oct 04, 2017 at 05:46:21PM +0900, Junio C Hamano wrote:

Show 20 quoted lines

> Jeff King <peff@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:

Show 12 quoted lines

> 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