Re: [PATCH v2 5/6] shortlog: implement `--group=author` in terms of `--group=<format>`

[Jeff King <peff@peff.net>]

On Thu, Oct 20, 2022 at 11:11:41PM -0400, Taylor Blau wrote:

> Instead of handling SHORTLOG_GROUP_AUTHOR separately, reimplement it as
> a special case of the new `--group=<format>` mode, where the author mode
> is a shorthand for `--group='%aN <%aE>'.
> 
> Note that we still need to keep the SHORTLOG_GROUP_AUTHOR enum since it
> has a different meaning in `read_from_stdin()`, where it is still used
> for a different purpose.

Yep, makes sense. The implementation all looks correct to me, although:

> diff --git a/builtin/log.c b/builtin/log.c
> index ee19dc5d45..b4d5420217 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1334,6 +1334,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
>  	log.in2 = 4;
>  	log.file = rev->diffopt.file;
>  	log.groups = SHORTLOG_GROUP_AUTHOR;
> +	shortlog_finish_setup(&log);
>  	for (i = 0; i < nr; i++)
>  		shortlog_add_commit(&log, list[i]);

I'd probably have pulled out this "finish_setup()" stuff into its own
patch. I can live with it either way, though. The advantage of having it
here is that it becomes obvious why this caller needs it (whereas if
done as a preparatory patch, it would just do the trailer sorting, which
make_cover_letter() obviously does not need).

-Peff