| Message ID | f0044682be70f876f6f92455f1fa46dd2e8aa09f.1666320509.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | shortlog: introduce `--group=<format>` | expand |
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
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]); diff --git a/builtin/shortlog.c b/builtin/shortlog.c index 0358397a7b..fceb84f12f 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -249,15 +249,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); - } if (log->groups & SHORTLOG_GROUP_COMMITTER) { strbuf_reset(&ident); format_commit_message(commit, @@ -377,6 +368,15 @@ void shortlog_init(struct shortlog *log) log->format.strdup_strings = 1; } +void shortlog_finish_setup(struct shortlog *log) +{ + if (log->groups & SHORTLOG_GROUP_AUTHOR) + string_list_append(&log->format, + log->email ? "%aN <%aE>" : "%aN"); + + string_list_sort(&log->trailers); +} + int cmd_shortlog(int argc, const char **argv, const char *prefix) { struct shortlog log = { STRING_LIST_INIT_NODUP }; @@ -446,7 +446,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix) if (!log.groups) log.groups = SHORTLOG_GROUP_AUTHOR; - string_list_sort(&log.trailers); + shortlog_finish_setup(&log); /* assume HEAD if from a tty */ if (!nongit && !rev.pending.nr && isatty(0)) diff --git a/shortlog.h b/shortlog.h index 4850a8c30f..28d04f951a 100644 --- a/shortlog.h +++ b/shortlog.h @@ -33,6 +33,7 @@ struct shortlog { }; void shortlog_init(struct shortlog *log); +void shortlog_finish_setup(struct shortlog *log); void shortlog_add_commit(struct shortlog *log, struct commit *commit);
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. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- builtin/log.c | 1 + builtin/shortlog.c | 20 ++++++++++---------- shortlog.h | 1 + 3 files changed, 12 insertions(+), 10 deletions(-)