git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/6] shortlog: introduce `--group=<format>`

From
Taylor Blau <me@ttaylorr.com>
Date
Oct 21, 2022, 03:11 UTC
Message-ID
<cover.1666320509.git.me@ttaylorr.com>
In-Reply-To
<cover.1665448437.git.me@ttaylorr.com>

Here is a reroll of my series to implement arbitrary pretty formats as shortlog `--group`'s, based on a suggestion from Jacob Stopak.

The changes are somewhat minimal, including a rebase onto the current tip of master. Less minimal, however, is dropping the reimplementation of `--group=trailer:<key>` in terms of the format group, since this ended up being more trouble than it was worth.

There are also a handful of small tweaks throughout based on feedback from Peff.

Thanks in advance for your review.
Jeff King (1):
  shortlog: accept `--date`-related options
Taylor Blau (5):
  shortlog: make trailer insertion a noop when appropriate
  shortlog: extract `--group` fragment for translation
  shortlog: support arbitrary commit format `--group`s
  shortlog: implement `--group=author` in terms of `--group=<format>`
  shortlog: implement `--group=committer` in terms of `--group=<format>`
 Documentation/git-shortlog.txt |  8 ++++
 builtin/log.c                  |  1 +
 builtin/shortlog.c             | 83 +++++++++++++++++++++++-----------
 shortlog.h                     |  5 ++
 t/t4201-shortlog.sh            | 51 +++++++++++++++++++++
 5 files changed, 121 insertions(+), 27 deletions(-)
Range-diff against v1:
1:  eaec59daa1 < -:  ---------- Documentation: extract date-options.txt
2:  b587b8ea4a ! 1:  58baccbaa8 shortlog: accept `--date`-related options
    @@ Metadata
      ## Commit message ##
         shortlog: accept `--date`-related options
     
    -    Prepare for the future patch which will introduce arbitrary pretty
    -    formats via the `--group` argument.
    +    Prepare for a future patch which will introduce arbitrary pretty formats
    +    via the `--group` argument.
     
         To allow additional customizability (for example, to support something
         like `git shortlog -s --group='%aD' --date='format:%Y-%m' ...` (which
         groups commits by the datestring 'YYYY-mm' according to author date), we
         must store off the `--date` parsed from calling `parse_revision_opt()`.
     
    +    Note that this also affects custom output `--format` strings in `git
    +    shortlog`. Though this is a behavior change, this is arguably fixing a
    +    long-standing bug (ie., that `--format` strings are not affected by
    +    `--date` specifiers as they should be).
    +
         Signed-off-by: Jeff King <peff@peff.net>
         Signed-off-by: Taylor Blau <me@ttaylorr.com>
     
      ## Documentation/git-shortlog.txt ##
    -@@ Documentation/git-shortlog.txt: options or the revision range, when confusion arises.
    - :git-shortlog: 1
    - include::rev-list-options.txt[]
    +@@ Documentation/git-shortlog.txt: OPTIONS
      
    -+include::date-options.txt[]
    + 	Each pretty-printed commit will be rewrapped before it is shown.
    + 
    ++--date=<format>::
    ++	With a `--group=format:<format>`, show dates formatted
    ++	according to the given date string. (See the `--date` options
    ++	in the "COMMIT FORMATTING" section of linkgit:git-log[1].)
     +
    - MAPPING AUTHORS
    - ---------------
    - 
    + --group=<type>::
    + 	Group commits based on `<type>`. If no `--group` option is
    + 	specified, the default is `author`. `<type>` is one of:
     
      ## builtin/shortlog.c ##
     @@ builtin/shortlog.c: void shortlog_add_commit(struct shortlog *log, struct commit *commit)
    @@ shortlog.h: struct shortlog {
      
      	enum {
      		SHORTLOG_GROUP_AUTHOR = (1 << 0),
    +
    + ## t/t4201-shortlog.sh ##
    +@@ t/t4201-shortlog.sh: test_expect_success 'pretty format' '
    + 	test_cmp expect log.predictable
    + '
    + 
    ++test_expect_success 'pretty format (with --date)' '
    ++	sed "s/SUBJECT/2005-04-07 OBJECT_NAME/" expect.template >expect &&
    ++	git shortlog --format="%ad %H" --date=short HEAD >log &&
    ++	fuzz log >log.predictable &&
    ++	test_cmp expect log.predictable
    ++'
    ++
    + test_expect_success '--abbrev' '
    + 	sed s/SUBJECT/OBJID/ expect.template >expect &&
    + 	git shortlog --format="%h" --abbrev=35 HEAD >log &&
-:  ---------- > 2:  7decccad7c shortlog: make trailer insertion a noop when appropriate
3:  c3f50465cb = 3:  3665488fc9 shortlog: extract `--group` fragment for translation
4:  6f38990cc2 ! 4:  4a36c0ca4e shortlog: support arbitrary commit format `--group`s
    @@ Documentation/git-shortlog.txt: OPTIONS
         example, if your project uses `Reviewed-by` trailers, you might want
         to see who has been reviewing with
         `git shortlog -ns --group=trailer:reviewed-by`.
    -+ - `<format>`, any string accepted by the `--format` option of 'git log'.
    -+   (See the "PRETTY FORMATS" section of linkgit:git-log[1].)
    ++ - `format:<format>`, any string accepted by the `--format` option of
    ++   'git log'. (See the "PRETTY FORMATS" section of
    ++   linkgit:git-log[1].)
      +
      Note that commits that do not include the trailer will not be counted.
      Likewise, commits with multiple trailers (e.g., multiple signoffs) may
    @@ builtin/shortlog.c: static void insert_records_from_trailers(struct shortlog *lo
     +
     +		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);
     +	}
     +
    @@ builtin/shortlog.c: static void insert_records_from_trailers(struct shortlog *lo
      {
      	struct strbuf ident = STRBUF_INIT;
     @@ builtin/shortlog.c: void shortlog_add_commit(struct shortlog *log, struct commit *commit)
    - 	if (log->groups & SHORTLOG_GROUP_TRAILER) {
    - 		insert_records_from_trailers(log, &dups, commit, &ctx, oneline_str);
    + 			insert_one_record(log, ident.buf, oneline_str);
      	}
    + 	insert_records_from_trailers(log, &dups, commit, &ctx, oneline_str);
     +	insert_records_from_format(log, &dups, commit, &ctx, oneline_str);
      
      	strset_clear(&dups);
    @@ builtin/shortlog.c: static int parse_group_option(const struct option *opt, cons
      		log->groups |= SHORTLOG_GROUP_TRAILER;
      		string_list_append(&log->trailers, field);
     -	} else
    -+	} else if (strchrnul(arg, '%')) {
    ++	} else if (skip_prefix(arg, "format:", &field)) {
    ++		log->groups |= SHORTLOG_GROUP_FORMAT;
    ++		string_list_append(&log->format, field);
    ++	} else if (strchr(arg, '%')) {
     +		log->groups |= SHORTLOG_GROUP_FORMAT;
     +		string_list_append(&log->format, arg);
     +	} else {
    @@ t/t4201-shortlog.sh: test_expect_success 'shortlog --group=trailer:signed-off-by
      	test_cmp expect actual
      '
      
    -+test_expect_success 'shortlog --group=<format>' '
    ++test_expect_success 'shortlog --group=format' '
    ++	git shortlog -s --date="format:%Y" --group="format:%cN (%cd)" \
    ++		HEAD >actual &&
    ++	cat >expect <<-\EOF &&
    ++	     4	C O Mitter (2005)
    ++	     1	Sin Nombre (2005)
    ++	EOF
    ++	test_cmp expect actual
    ++'
    ++
    ++test_expect_success 'shortlog --group=<format> DWIM' '
     +	git shortlog -s --date="format:%Y" --group="%cN (%cd)" HEAD >actual &&
    ++	test_cmp expect actual
    ++'
    ++
    ++test_expect_success 'shortlog multiple --group=format' '
    ++	git shortlog -s --date="format:%Y" --group="format:%cN (%cd)" \
    ++		HEAD >actual &&
     +	cat >expect <<-\EOF &&
     +	     4	C O Mitter (2005)
     +	     1	Sin Nombre (2005)
     +	EOF
     +	test_cmp expect actual
     +'
    ++
    ++test_expect_success 'shortlog bogus --group' '
    ++	test_must_fail git shortlog --group=bogus HEAD 2>err &&
    ++	grep "unknown group type" err
    ++'
     +
      test_expect_success 'trailer idents are split' '
      	cat >expect <<-\EOF &&
      	     2	C O Mitter
    +@@ t/t4201-shortlog.sh: test_expect_success 'shortlog can match multiple groups' '
    + 	test_cmp expect actual
    + '
    + 
    ++test_expect_success 'shortlog can match multiple format groups' '
    ++	cat >expect <<-\EOF &&
    ++	     2	User B <b@example.com>
    ++	     1	A U Thor <author@example.com>
    ++	     1	User A <a@example.com>
    ++	EOF
    ++	git shortlog -ns \
    ++		--group="%(trailers:valueonly,key=some-trailer)" \
    ++		--group="%(trailers:valueonly,key=another-trailer)" \
    ++		-2 HEAD >actual.raw &&
    ++	grep -v "^$" actual.raw >actual &&
    ++	test_cmp expect actual
    ++'
    ++
    + test_expect_success 'set up option selection tests' '
    + 	git commit --allow-empty -F - <<-\EOF
    + 	subject
5:  55a6ef7bc0 ! 5:  f0044682be shortlog: implement `--group=author` in terms of `--group=<format>`
    @@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_separ
      	log.in2 = 4;
      	log.file = rev->diffopt.file;
      	log.groups = SHORTLOG_GROUP_AUTHOR;
    -+	shortlog_init_group(&log);
    ++	shortlog_finish_setup(&log);
      	for (i = 0; i < nr; i++)
      		shortlog_add_commit(&log, list[i]);
      
    @@ builtin/shortlog.c: void shortlog_init(struct shortlog *log)
      	log->format.strdup_strings = 1;
      }
      
    -+void shortlog_init_group(struct shortlog *log)
    ++void shortlog_finish_setup(struct shortlog *log)
     +{
    -+	if (!log->groups)
    -+		log->groups = SHORTLOG_GROUP_AUTHOR;
    -+
     +	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 };
     @@ builtin/shortlog.c: int cmd_shortlog(int argc, const char **argv, const char *prefix)
    - 	log.file = rev.diffopt.file;
    - 	log.date_mode = rev.date_mode;
      
    --	if (!log.groups)
    --		log.groups = SHORTLOG_GROUP_AUTHOR;
    -+	shortlog_init_group(&log);
    -+
    - 	string_list_sort(&log.trailers);
    + 	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))
     
      ## shortlog.h ##
     @@ shortlog.h: struct shortlog {
      };
      
      void shortlog_init(struct shortlog *log);
    -+void shortlog_init_group(struct shortlog *log);
    ++void shortlog_finish_setup(struct shortlog *log);
      
      void shortlog_add_commit(struct shortlog *log, struct commit *commit);
      
6:  814ee4b8d8 ! 6:  6a9e204177 shortlog: implement `--group=committer` in terms of `--group=<format>`
    @@ builtin/shortlog.c: void shortlog_add_commit(struct shortlog *log, struct commit
     -		    strset_add(&dups, ident.buf))
     -			insert_one_record(log, ident.buf, oneline_str);
     -	}
    - 	if (log->groups & SHORTLOG_GROUP_TRAILER) {
    - 		insert_records_from_trailers(log, &dups, commit, &ctx, oneline_str);
    - 	}
    + 	insert_records_from_trailers(log, &dups, commit, &ctx, oneline_str);
      	insert_records_from_format(log, &dups, commit, &ctx, oneline_str);
      
      	strset_clear(&dups);
    @@ builtin/shortlog.c: void shortlog_add_commit(struct shortlog *log, struct commit
      	strbuf_release(&oneline);
      }
      
    -@@ builtin/shortlog.c: void shortlog_init_group(struct shortlog *log)
    +@@ builtin/shortlog.c: void shortlog_finish_setup(struct shortlog *log)
      	if (log->groups & SHORTLOG_GROUP_AUTHOR)
      		string_list_append(&log->format,
      				   log->email ? "%aN <%aE>" : "%aN");
     +	if (log->groups & SHORTLOG_GROUP_COMMITTER)
     +		string_list_append(&log->format,
     +				   log->email ? "%cN <%cE>" : "%cN");
    - }
      
    - int cmd_shortlog(int argc, const char **argv, const char *prefix)
    + 	string_list_sort(&log->trailers);
    + }
7:  02adc297e7 < -:  ---------- shortlog: implement `--group=trailer` in terms of `--group=<format>`
-- 
2.38.0.16.g393fd4c6db
Previous: Jeff KingNext: Taylor Blau
Message 36 of 72 in “shortlog: introduce `--group=<format>`”
  1. 0/7 shortlog: introduce `--group=<format>`Taylor Blau, Oct 11, 2022
  2. 1/7 Documentation: extract date-options.txtTaylor Blau, Oct 11, 2022
  3. Jeff KingOct 11, 2022
  4. Taylor BlauOct 11, 2022
  5. 2/7 shortlog: accept `--date`-related optionsTaylor Blau, Oct 11, 2022
  6. Jeff KingOct 11, 2022
  7. Taylor BlauOct 11, 2022
  8. 3/7 shortlog: extract `--group` fragment for translationTaylor Blau, Oct 11, 2022
  9. Ævar Arnfjörð BjarmasonOct 11, 2022
  10. Jeff KingOct 11, 2022
  11. 4/7 shortlog: support arbitrary commit format `--group`sTaylor Blau, Oct 11, 2022
  12. Jeff KingOct 11, 2022
  13. Taylor BlauOct 11, 2022
  14. Taylor BlauOct 11, 2022
  15. Jeff KingOct 11, 2022
  16. Jeff KingOct 11, 2022
  17. Taylor BlauOct 21, 2022
  18. Jeff KingOct 21, 2022
  19. Taylor BlauOct 21, 2022
  20. 5/7 shortlog: implement `--group=author` in terms of `--group=<format>`Taylor Blau, Oct 11, 2022
  21. Jeff KingOct 11, 2022
  22. Taylor BlauOct 11, 2022
  23. Jeff KingOct 11, 2022
  24. Taylor BlauOct 21, 2022
  25. Taylor BlauOct 21, 2022
  26. Jeff KingOct 21, 2022
  27. Taylor BlauOct 21, 2022
  28. Ævar Arnfjörð BjarmasonOct 11, 2022
  29. Ævar Arnfjörð BjarmasonOct 11, 2022
  30. 7/7 shortlog: implement `--group=trailer` in terms of `--group=<format>`Taylor Blau, Oct 11, 2022
  31. Jeff KingOct 11, 2022
  32. Jeff KingOct 11, 2022
  33. Taylor BlauOct 21, 2022
  34. 6/7 shortlog: implement `--group=committer` in terms of `--group=<format>`Taylor Blau, Oct 11, 2022
  35. Jeff KingOct 11, 2022
  36. 0/6 shortlog: introduce `--group=<format>`Taylor Blau, Oct 21, 2022
  37. 1/6 shortlog: accept `--date`-related optionsTaylor Blau, Oct 21, 2022
  38. Jeff KingOct 21, 2022
  39. Taylor BlauOct 21, 2022
  40. 2/6 shortlog: make trailer insertion a noop when appropriateTaylor Blau, Oct 21, 2022
  41. Jeff KingOct 21, 2022
  42. Taylor BlauOct 21, 2022
  43. 3/6 shortlog: extract `--group` fragment for translationTaylor Blau, Oct 21, 2022
  44. Jeff KingOct 21, 2022
  45. 4/6 shortlog: support arbitrary commit format `--group`sTaylor Blau, Oct 21, 2022
  46. Jeff KingOct 21, 2022
  47. Taylor BlauOct 21, 2022
  48. 5/6 shortlog: implement `--group=author` in terms of `--group=<format>`Taylor Blau, Oct 21, 2022
  49. Jeff KingOct 21, 2022
  50. 6/6 shortlog: implement `--group=committer` in terms of `--group=<format>`Taylor Blau, Oct 21, 2022
  51. Jeff KingOct 21, 2022
  52. Jeff KingOct 21, 2022
  53. 0/7 shortlog: introduce `--group=<format>`Taylor Blau, Oct 21, 2022
  54. 1/7 shortlog: accept `--date`-related optionsTaylor Blau, Oct 21, 2022
  55. 2/7 shortlog: make trailer insertion a noop when appropriateTaylor Blau, Oct 21, 2022
  56. 4/7 shortlog: support arbitrary commit format `--group`sTaylor Blau, Oct 21, 2022
  57. Jeff KingOct 22, 2022
  58. 5/7 shortlog: extract `shortlog_finish_setup()`Taylor Blau, Oct 21, 2022
  59. 6/7 shortlog: implement `--group=author` in terms of `--group=<format>`Taylor Blau, Oct 21, 2022
  60. 3/7 shortlog: extract `--group` fragment for translationTaylor Blau, Oct 21, 2022
  61. 7/7 shortlog: implement `--group=committer` in terms of `--group=<format>`Taylor Blau, Oct 21, 2022
  62. Jeff KingOct 22, 2022
  63. 0/7 shortlog: introduce `--group=<format>`Taylor Blau, Oct 24, 2022
  64. 7/7 shortlog: implement `--group=committer` in terms of `--group=<format>`Taylor Blau, Oct 24, 2022
  65. 5/7 shortlog: extract `shortlog_finish_setup()`Taylor Blau, Oct 24, 2022
  66. 4/7 shortlog: support arbitrary commit format `--group`sTaylor Blau, Oct 24, 2022
  67. 3/7 shortlog: extract `--group` fragment for translationTaylor Blau, Oct 24, 2022
  68. 1/7 shortlog: accept `--date`-related optionsTaylor Blau, Oct 24, 2022
  69. 6/7 shortlog: implement `--group=author` in terms of `--group=<format>`Taylor Blau, Oct 24, 2022
  70. 2/7 shortlog: make trailer insertion a noop when appropriateTaylor Blau, Oct 24, 2022
  71. Junio C HamanoOct 24, 2022
  72. Jeff KingOct 24, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.