On Mon, Oct 02, 2017 at 01:03:51AM -0400, Jeff King wrote: > On Sun, Oct 01, 2017 at 05:33:04PM -0700, Taylor Blau wrote: > > > The %(contents) atom takes a contents "field" as its argument. Since > > "trailers" is one of those fields, extend contents_atom_parser to parse > > "trailers"'s arguments when used through "%(contents)", like: > > > > %(contents:trailers:unfold,only) > > > > A caveat: trailers_atom_parser expects NULL when no arguments are given > > (see: `parse_ref_filter_atom`). This is because string_list_split (given > > a maxsplit of -1) returns a 1-ary string_list* containing the given > > string if the delimiter could not be found using `strchr`. > > > > To simulate this behavior without teaching trailers_atom_parser to > > accept strings with length zero, conditionally pass NULL to > > trailers_atom_parser if the arguments portion of the argument to > > %(contents) is empty. > > 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 > > which seems a bit unfriendly. As we're on v6 here, I don't want to > suggest it as part of this series. But maybe a #leftoverbits candidate, > if others agree it's a good direction. I think #leftoverbits is fine here, but I think addressing this before 2.15 is reasonable. I can take a look at this in a future patch series. -- - Taylor
