On Mon, Oct 10, 2022 at 09:48:47PM -0400, Taylor Blau wrote:
> > > diff --git a/builtin/shortlog.c b/builtin/shortlog.c
> > > index f708d96558..aac8c7afa4 100644
> > > --- a/builtin/shortlog.c
> > > +++ b/builtin/shortlog.c
> > > @@ -245,15 +245,6 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
> > > }
> > > oneline_str = oneline.len ? oneline.buf : "<none>";
> > >
> > > - if (log->groups & SHORTLOG_GROUP_AUTHOR) {
> > > - strbuf_reset(&ident);
> > > - format_commit_message(commit,
> > > - log->email ? "%aN <%aE>" : "%aN",
> > > - &ident, &ctx);
> > > - if (!HAS_MULTI_BITS(log->groups) ||
> > > - strset_add(&dups, ident.buf))
> > > - insert_one_record(log, ident.buf, oneline_str);
> > > - }
> >
> > This loses the HAS_MULTI_BITS() optimization. The idea there is that if
> > you have a single group-by that can't produce multiple outputs, then
> > there's no need to do duplicate detection.
> >
> > The equivalent in an all-formats world is something like:
> >
> > log.format.nr > 1 && !log.trailers.nr
> >
> > (because trailers are special in that one trailer key can produce
> > multiple idents for a single commit).
Hmm. Shouldn't that "&& !log.trailers.nr" be an "|| log.trailers.nr"? It
doesn't seem to make sense to say "there are things that could produce
multiple outputs" if there's more than one format _and_ no trailers.
The logic should read "there are things that could produce multiple
outputs if there is more than one format *or* at least one trailer".
So I think the right change would be:
--- >8 ---
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 95ceab7649..7e1b56e2aa 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -216,7 +216,8 @@ static void insert_records_from_format(struct shortlog *log,
format_commit_message(commit, item->string, &buf, ctx);
- if (strset_add(dups, buf.buf))
+ if (!(log->format.nr > 1 || log->trailers.nr) ||
+ strset_add(dups, buf.buf))
insert_one_record(log, buf.buf, oneline);
}
--- 8< ---
Yeah?
Thanks,
Taylor
