Re: [PATCH 4/7] shortlog: support arbitrary commit format `--group`s

[Taylor Blau <me@ttaylorr.com>]

On Fri, Oct 21, 2022 at 01:21:30AM -0400, Jeff King wrote:
> On Thu, Oct 20, 2022 at 10:39:54PM -0400, Taylor Blau wrote:
>
> > --- 8< ---
> > diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
> > index 0abe5354fc..566d581e1b 100755
> > --- a/t/t4201-shortlog.sh
> > +++ b/t/t4201-shortlog.sh
> > @@ -356,6 +356,19 @@ test_expect_success 'shortlog can match multiple groups' '
> >  	test_cmp expect actual
> >  '
> >
> > +test_expect_success 'shortlog can match multiple format groups' '
> > +	cat >expect <<-\EOF &&
> > +	     2	User B <b@example.com>
> > +	     1	A U Thor <author@example.com>
> > +	     1	User A <a@example.com>
> > +	EOF
> > +	git shortlog -ns \
> > +		--group="%(trailers:valueonly,separator=%0x00,key=some-trailer)" \
> > +		--group="%(trailers:valueonly,separator=%0x00,key=another-trailer)" \
> > +		-2 HEAD >actual &&
> > +	test_cmp expect actual
> > +'
> > +
> >  test_expect_success 'set up option selection tests' '
> >  	git commit --allow-empty -F - <<-\EOF
> >  	subject
> > --- >8 ---
> >
> > The gross bit there really is the 'separator=%0x00' thing, since the
> > "trailers" pretty format gives us a NL character already. I suppose that
> > you could do something like this on top instead:
>
> IMHO the new test should avoid using trailers entirely, to avoid the
> notion that they are necessary to create the duplicate situation. In a
> normal repository, the most obvious one is just asking about both
> authors and committers:
>
>   git shortlog --group=format:%cn --group=format:%an

Yeah, that's fair. I was worried enough about whether or not this was
going to cause significant test fallout, but it ended up being extremely
straightforward and simplified the tests nicely. Thanks!

Thanks,
Taylor