Re: [RFC PATCH v2] shortlog: add group-by options for year and month

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 7, 2022, 22:24 UTC
Message-ID: <Y0CnJBzTbNgRIqZ+@nand.local>
In-Reply-To: <Yz4EsT8noIoygk9b@coredump.intra.peff.net>

On Wed, Oct 05, 2022 at 06:26:57PM -0400, Jeff King wrote:

Show 27 quoted lines

> > +static void insert_record_from_pretty(struct shortlog *log,
> > +				      struct strset *dups,
> > +				      struct commit *commit,
> > +				      struct pretty_print_context *ctx,
> > +				      const char *oneline)
> > +{
> > +	struct strbuf ident = STRBUF_INIT;
> > +	size_t i;
> > +
> > +	for (i = 0; i < log->pretty.nr; i++) {
> > +		if (i)
> > +			strbuf_addch(&ident, ' ');
> > +
> > +		format_commit_message(commit, log->pretty.items[i].string,
> > +				      &ident, ctx);
> > +	}
>
> So here you're allowing multiple pretty options. But really, once we
> allow the user an arbitrary format, is there any reason for them to do:
>
>   git shortlog --group=%an --group=%ad
>
> versus just:
>
>   git shortlog --group='%an %ad'
>
> ?

I think that if you want to unify `--group=author` into the new format group implementation, you would have to allow multiple `--group` options, but each such option would generate its own shortlog identity instead of getting concatenated together.

Thanks, Taylor