shortlog: allow multiple groups to be specified
Now that shortlog supports reading from trailers, it can be useful to combine counts from multiple trailers, or between trailers and authors. This can be done manually by post-processing the output from multiple runs, but it's non-trivial to make sure that each name/commit pair is counted only once. This patch teaches shortlog to accept multiple --group options on the command line, and pull data from all of them. That makes it possible to run: git shortlog -ns --group=author --group=trailer:co-authored-by to get a shortlog that counts authors and co-authors equally. The implementation is mostly straightforward. The "group" enum becomes a bitfield, and the trailer key becomes a list. I didn't bother implementing the multi-group semantics for reading from stdin. It would be possible to do, but the existing matching code makes it awkward, and I doubt anybody cares. The duplicate suppression we used for trailers now covers authors and committers as well (though in non-trailer single-group mode we can skip the hash insertion and lookup, since we only see one value per commit). There is one subtlety: we now care about the case when no group bit is set (in which case we default to showing the author). The caller in builtin/log.c needs to be adapted to ask explicitly for authors, rather than relying on shortlog_init(). It would be possible with some gymnastics to make this keep working as-is, but it's not worth it for a single caller. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Sep 27, 2020 at 04:40 UTC
63d24fa0b055d3f0386ee3686c07428450add708
5 files changed
+127
-29
Documentation/git-shortlog.txt
+7
@@ -51,6 +51,7 @@ OPTIONS
51
Group commits based on `<type>`. If no `--group` option is
52
specified, the default is `author`. `<type>` is one of:
53
+
54
+--
55
- `author`, commits are grouped by author
56
- `committer`, commits are grouped by committer (the same as `-c`)
57
- `trailer:<field>`, the `<field>` is interpreted as a case-insensitive
@@ -68,6 +69,12 @@ Shortlog will attempt to parse each trailer value as a `name <email>`
69
identity. If successful, the mailmap is applied and the email is omitted
70
unless the `--email` option is specified. If the value cannot be parsed
71
as an identity, it will be taken literally and completely.
72
+--
73
++
74
+If `--group` is specified multiple times, commits are counted under each
75
+value (but again, only once per unique value in that commit). For
76
+example, `git shortlog --group=author --group=trailer:co-authored-by`
77
+counts both authors and co-authors.
78
79
-c::
80
--committer::
builtin/log.c
+1
@@ -1197,6 +1197,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
1197
log.in1 = 2;
1198
log.in2 = 4;
1199
log.file = rev->diffopt.file;
1200
+ log.groups = SHORTLOG_GROUP_AUTHOR;
1201
for (i = 0; i < nr; i++)
1202
shortlog_add_commit(&log, list[i]);
1203
builtin/shortlog.c
+40
-24
@@ -130,7 +130,10 @@ static void read_from_stdin(struct shortlog *log)
130
static const char *committer_match[2] = { "Commit: ", "committer " };
131
const char **match;
132
133
- switch (log->group) {
133
+ if (HAS_MULTI_BITS(log->groups))
134
+ die(_("using multiple --group options with stdin is not supported"));
135
+
136
+ switch (log->groups) {
137
case SHORTLOG_GROUP_AUTHOR:
138
match = author_match;
139
break;
@@ -221,13 +224,13 @@ static void strset_clear(struct strset *ss)
224
}
225
226
static void insert_records_from_trailers(struct shortlog *log,
227
+ struct strset *dups,
228
struct commit *commit,
229
struct pretty_print_context *ctx,
230
const char *oneline)
231
{
232
struct trailer_iterator iter;
233
const char *commit_buffer, *body;
230
- struct strset dups = STRSET_INIT;
234
struct strbuf ident = STRBUF_INIT;
235
236
/*
@@ -243,21 +246,20 @@ static void insert_records_from_trailers(struct shortlog *log,
246
while (trailer_iterator_advance(&iter)) {
247
const char *value = iter.val.buf;
248
246
- if (strcasecmp(iter.key.buf, log->trailer))
249
+ if (!string_list_has_string(&log->trailers, iter.key.buf))
250
continue;
251
252
strbuf_reset(&ident);
253
if (!parse_ident(log, &ident, value))
254
value = ident.buf;
255
253
- if (strset_check_and_add(&dups, value))
256
+ if (strset_check_and_add(dups, value))
257
continue;
258
insert_one_record(log, value, oneline);
259
}
260
trailer_iterator_release(&iter);
261
262
strbuf_release(&ident);
260
- strset_clear(&dups);
263
unuse_commit_buffer(commit, commit_buffer);
264
}
265
@@ -265,6 +267,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
267
{
268
struct strbuf ident = STRBUF_INIT;
269
struct strbuf oneline = STRBUF_INIT;
270
+ struct strset dups = STRSET_INIT;
271
struct pretty_print_context ctx = {0};
272
const char *oneline_str;
273
@@ -282,24 +285,29 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
285
}
286
oneline_str = oneline.len ? oneline.buf : "<none>";
287
285
- switch (log->group) {
286
- case SHORTLOG_GROUP_AUTHOR:
288
+ if (log->groups & SHORTLOG_GROUP_AUTHOR) {
289
+ strbuf_reset(&ident);
290
format_commit_message(commit,
291
log->email ? "%aN <%aE>" : "%aN",
292
&ident, &ctx);
290
- insert_one_record(log, ident.buf, oneline_str);
291
- break;
292
- case SHORTLOG_GROUP_COMMITTER:
293
+ if (!HAS_MULTI_BITS(log->groups) ||
294
+ !strset_check_and_add(&dups, ident.buf))
295
+ insert_one_record(log, ident.buf, oneline_str);
296
+ }
297
+ if (log->groups & SHORTLOG_GROUP_COMMITTER) {
298
+ strbuf_reset(&ident);
299
format_commit_message(commit,
300
log->email ? "%cN <%cE>" : "%cN",
301
&ident, &ctx);
296
- insert_one_record(log, ident.buf, oneline_str);
297
- break;
298
- case SHORTLOG_GROUP_TRAILER:
299
- insert_records_from_trailers(log, commit, &ctx, oneline_str);
300
- break;
302
+ if (!HAS_MULTI_BITS(log->groups) ||
303
+ !strset_check_and_add(&dups, ident.buf))
304
+ insert_one_record(log, ident.buf, oneline_str);
305
+ }
306
+ if (log->groups & SHORTLOG_GROUP_TRAILER) {
307
+ insert_records_from_trailers(log, &dups, commit, &ctx, oneline_str);
308
}
309
310
+ strset_clear(&dups);
311
strbuf_release(&ident);
312
strbuf_release(&oneline);
313
}
@@ -366,14 +374,16 @@ static int parse_group_option(const struct option *opt, const char *arg, int uns
374
struct shortlog *log = opt->value;
375
const char *field;
376
369
- if (unset || !strcasecmp(arg, "author"))
370
- log->group = SHORTLOG_GROUP_AUTHOR;
377
+ if (unset) {
378
+ log->groups = 0;
379
+ string_list_clear(&log->trailers, 0);
380
+ } else if (!strcasecmp(arg, "author"))
381
+ log->groups |= SHORTLOG_GROUP_AUTHOR;
382
else if (!strcasecmp(arg, "committer"))
372
- log->group = SHORTLOG_GROUP_COMMITTER;
383
+ log->groups |= SHORTLOG_GROUP_COMMITTER;
384
else if (skip_prefix(arg, "trailer:", &field)) {
374
- log->group = SHORTLOG_GROUP_TRAILER;
375
- free(log->trailer);
376
- log->trailer = xstrdup(field);
385
+ log->groups |= SHORTLOG_GROUP_TRAILER;
386
+ string_list_append(&log->trailers, field);
387
} else
388
return error(_("unknown group type: %s"), arg);
389
@@ -391,6 +401,8 @@ void shortlog_init(struct shortlog *log)
401
log->wrap = DEFAULT_WRAPLEN;
402
log->in1 = DEFAULT_INDENT1;
403
log->in2 = DEFAULT_INDENT2;
404
+ log->trailers.strdup_strings = 1;
405
+ log->trailers.cmp = strcasecmp;
406
}
407
408
int cmd_shortlog(int argc, const char **argv, const char *prefix)
@@ -400,9 +412,9 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
412
int nongit = !startup_info->have_repository;
413
414
const struct option options[] = {
403
- OPT_SET_INT('c', "committer", &log.group,
404
- N_("Group by committer rather than author"),
405
- SHORTLOG_GROUP_COMMITTER),
415
+ OPT_BIT('c', "committer", &log.groups,
416
+ N_("Group by committer rather than author"),
417
+ SHORTLOG_GROUP_COMMITTER),
418
OPT_BOOL('n', "numbered", &log.sort_by_number,
419
N_("sort output according to the number of commits per author")),
420
OPT_BOOL('s', "summary", &log.summary,
@@ -454,6 +466,10 @@ parse_done:
466
log.abbrev = rev.abbrev;
467
log.file = rev.diffopt.file;
468
469
+ if (!log.groups)
470
+ log.groups = SHORTLOG_GROUP_AUTHOR;
471
+ string_list_sort(&log.trailers);
472
+
473
/* assume HEAD if from a tty */
474
if (!nongit && !rev.pending.nr && isatty(0))
475
add_head_to_pending(&rev);
shortlog.h
+5
-5
@@ -17,11 +17,11 @@ struct shortlog {
17
int abbrev;
18
19
enum {
20
- SHORTLOG_GROUP_AUTHOR = 0,
21
- SHORTLOG_GROUP_COMMITTER,
22
- SHORTLOG_GROUP_TRAILER,
23
- } group;
24
- char *trailer;
20
+ SHORTLOG_GROUP_AUTHOR = (1 << 0),
21
+ SHORTLOG_GROUP_COMMITTER = (1 << 1),
22
+ SHORTLOG_GROUP_TRAILER = (1 << 2),
23
+ } groups;
24
+ struct string_list trailers;
25
26
char *common_repo_prefix;
27
int email;
t/t4201-shortlog.sh
+74
@@ -282,4 +282,78 @@ test_expect_success 'shortlog de-duplicates trailers in a single commit' '
282
test_cmp expect actual
283
'
284
285
+test_expect_success 'shortlog can match multiple groups' '
286
+ git commit --allow-empty -F - <<-\EOF &&
287
+ subject one
288
+
289
+ this has two trailers that are distinct from the author; it will count
290
+ 3 times in the output
291
+
292
+ Some-trailer: User A <a@example.com>
293
+ Another-trailer: User B <b@example.com>
294
+ EOF
295
+
296
+ git commit --allow-empty -F - <<-\EOF &&
297
+ subject two
298
+
299
+ this one has two trailers, one of which is a duplicate with the author;
300
+ it will only be counted once for them
301
+
302
+ Another-trailer: A U Thor <author@example.com>
303
+ Some-trailer: User B <b@example.com>
304
+ EOF
305
+
306
+ cat >expect <<-\EOF &&
307
+ 2 A U Thor
308
+ 2 User B
309
+ 1 User A
310
+ EOF
311
+ git shortlog -ns \
312
+ --group=author \
313
+ --group=trailer:some-trailer \
314
+ --group=trailer:another-trailer \
315
+ -2 HEAD >actual &&
316
+ test_cmp expect actual
317
+'
318
+
319
+test_expect_success 'set up option selection tests' '
320
+ git commit --allow-empty -F - <<-\EOF
321
+ subject
322
+
323
+ body
324
+
325
+ Trailer-one: value-one
326
+ Trailer-two: value-two
327
+ EOF
328
+'
329
+
330
+test_expect_success '--no-group resets group list to author' '
331
+ cat >expect <<-\EOF &&
332
+ 1 A U Thor
333
+ EOF
334
+ git shortlog -ns \
335
+ --group=committer \
336
+ --group=trailer:trailer-one \
337
+ --no-group \
338
+ -1 HEAD >actual &&
339
+ test_cmp expect actual
340
+'
341
+
342
+test_expect_success '--no-group resets trailer list' '
343
+ cat >expect <<-\EOF &&
344
+ 1 value-two
345
+ EOF
346
+ git shortlog -ns \
347
+ --group=trailer:trailer-one \
348
+ --no-group \
349
+ --group=trailer:trailer-two \
350
+ -1 HEAD >actual &&
351
+ test_cmp expect actual
352
+'
353
+
354
+test_expect_success 'stdin with multiple groups reports error' '
355
+ git log >log &&
356
+ test_must_fail git shortlog --group=author --group=committer <log
357
+'
358
+
359
test_done