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

[PATCH v2 2/8] shortlog: add grouping option

From
Jeff King <peff@peff.net>
Date
Sep 27, 2020, 08:39 UTC
Message-ID
<20200927083959.GB2465761@coredump.intra.peff.net>
In-Reply-To
<20200927083933.GA2222823@coredump.intra.peff.net>

In preparation for adding more grouping types, let's refactor the committer/author grouping code and add a user-facing option that binds them together. In particular:

  - the main option is now "--group", to make it clear
    that the various group types are mutually exclusive. The
    "--committer" option is an alias for "--group=committer".
  - we keep an enum rather than a binary flag, to prepare
    for more values
  - we prefer switch statements to ternary assignment, since
    other group types will need more custom code
Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/git-shortlog.txt |  9 +++++-
 builtin/shortlog.c             | 59 +++++++++++++++++++++++++++-------
 shortlog.h                     |  6 +++-
 t/t4201-shortlog.sh            |  5 +++
 4 files changed, 66 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index a72ea7f7ba..6496d313c1 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -47,9 +47,16 @@ OPTIONS
 
 	Each pretty-printed commit will be rewrapped before it is shown.
 
+--group=<type>::
+	Group commits based on `<type>`. If no `--group` option is
+	specified, the default is `author`. `<type>` is one of:
++
+ - `author`, commits are grouped by author
+ - `committer`, commits are grouped by committer (the same as `-c`)
+
 -c::
 --committer::
-	Collect and show committer identities instead of authors.
+	This is an alias for `--group=committer`.
 
 -w[<width>[,<indent1>[,<indent2>]]]::
 	Linewrap the output by wrapping each line at `width`.  The first
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index edcf2e0d54..880ce19304 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -129,7 +129,17 @@ static void read_from_stdin(struct shortlog *log)
 	static const char *committer_match[2] = { "Commit: ", "committer " };
 	const char **match;
 
-	match = log->committer ? committer_match : author_match;
+	switch (log->group) {
+	case SHORTLOG_GROUP_AUTHOR:
+		match = author_match;
+		break;
+	case SHORTLOG_GROUP_COMMITTER:
+		match = committer_match;
+		break;
+	default:
+		BUG("unhandled shortlog group");
+	}
+
 	while (strbuf_getline_lf(&ident, stdin) != EOF) {
 		const char *v;
 		if (!skip_prefix(ident.buf, match[0], &v) &&
@@ -158,27 +168,36 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
 	struct strbuf ident = STRBUF_INIT;
 	struct strbuf oneline = STRBUF_INIT;
 	struct pretty_print_context ctx = {0};
-	const char *fmt;
+	const char *oneline_str;
 
 	ctx.fmt = CMIT_FMT_USERFORMAT;
 	ctx.abbrev = log->abbrev;
 	ctx.print_email_subject = 1;
 	ctx.date_mode.type = DATE_NORMAL;
 	ctx.output_encoding = get_log_output_encoding();
 
-	fmt = log->committer ?
-		(log->email ? "%cN <%cE>" : "%cN") :
-		(log->email ? "%aN <%aE>" : "%aN");
-
-	format_commit_message(commit, fmt, &ident, &ctx);
 	if (!log->summary) {
 		if (log->user_format)
 			pretty_print_commit(&ctx, commit, &oneline);
 		else
 			format_commit_message(commit, "%s", &oneline, &ctx);
 	}
-
-	insert_one_record(log, ident.buf, oneline.len ? oneline.buf : "<none>");
+	oneline_str = oneline.len ? oneline.buf : "<none>";
+
+	switch (log->group) {
+	case SHORTLOG_GROUP_AUTHOR:
+		format_commit_message(commit,
+				      log->email ? "%aN <%aE>" : "%aN",
+				      &ident, &ctx);
+		insert_one_record(log, ident.buf, oneline_str);
+		break;
+	case SHORTLOG_GROUP_COMMITTER:
+		format_commit_message(commit,
+				      log->email ? "%cN <%cE>" : "%cN",
+				      &ident, &ctx);
+		insert_one_record(log, ident.buf, oneline_str);
+		break;
+	}
 
 	strbuf_release(&ident);
 	strbuf_release(&oneline);
@@ -241,6 +260,21 @@ static int parse_wrap_args(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int parse_group_option(const struct option *opt, const char *arg, int unset)
+{
+	struct shortlog *log = opt->value;
+
+	if (unset || !strcasecmp(arg, "author"))
+		log->group = SHORTLOG_GROUP_AUTHOR;
+	else if (!strcasecmp(arg, "committer"))
+		log->group = SHORTLOG_GROUP_COMMITTER;
+	else
+		return error(_("unknown group type: %s"), arg);
+
+	return 0;
+}
+
+
 void shortlog_init(struct shortlog *log)
 {
 	memset(log, 0, sizeof(*log));
@@ -260,8 +294,9 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 	int nongit = !startup_info->have_repository;
 
 	const struct option options[] = {
-		OPT_BOOL('c', "committer", &log.committer,
-			 N_("Group by committer rather than author")),
+		OPT_SET_INT('c', "committer", &log.group,
+			    N_("Group by committer rather than author"),
+			    SHORTLOG_GROUP_COMMITTER),
 		OPT_BOOL('n', "numbered", &log.sort_by_number,
 			 N_("sort output according to the number of commits per author")),
 		OPT_BOOL('s', "summary", &log.summary,
@@ -271,6 +306,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 		OPT_CALLBACK_F('w', NULL, &log, N_("<w>[,<i1>[,<i2>]]"),
 			N_("Linewrap output"), PARSE_OPT_OPTARG,
 			&parse_wrap_args),
+		OPT_CALLBACK(0, "group", &log, N_("field"),
+			N_("Group by field"), parse_group_option),
 		OPT_END(),
 	};
 
diff --git a/shortlog.h b/shortlog.h
index 2fa61c4294..876a52158d 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -15,7 +15,11 @@ struct shortlog {
 	int in2;
 	int user_format;
 	int abbrev;
-	int committer;
+
+	enum {
+		SHORTLOG_GROUP_AUTHOR = 0,
+		SHORTLOG_GROUP_COMMITTER,
+	} group;
 
 	char *common_repo_prefix;
 	int email;
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index d3a7ce6bbb..65e4468746 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -215,4 +215,9 @@ test_expect_success 'shortlog --committer (external)' '
 	test_cmp expect actual
 '
 
+test_expect_success '--group=committer is the same as --committer' '
+	git shortlog -ns --group=committer HEAD >actual &&
+	test_cmp expect actual
+'
+
 test_done

-- 
2.28.0.1127.ga65787d918
Previous: Junio C HamanoNext: Jeff King
Message 27 of 44 in “parsing trailers with shortlog”
  1. 0/8 parsing trailers with shortlogJeff King, Sep 25, 2020
  2. 1/8 shortlog: change "author" variables to "ident"Jeff King, Sep 25, 2020
  3. 2/8 shortlog: refactor committer/author groupingJeff King, Sep 25, 2020
  4. Eric SunshineSep 25, 2020
  5. Jeff KingSep 27, 2020
  6. Jeff KingSep 27, 2020
  7. Eric SunshineSep 27, 2020
  8. Martin ÅgrenSep 26, 2020
  9. Jeff KingSep 27, 2020
  10. 3/8 trailer: add interface for iterating over commit trailersJeff King, Sep 25, 2020
  11. Martin ÅgrenSep 26, 2020
  12. Jeff KingSep 27, 2020
  13. 4/8 shortlog: match commit trailers with --groupJeff King, Sep 25, 2020
  14. 5/8 shortlog: de-duplicate trailer valuesJeff King, Sep 25, 2020
  15. 6/8 shortlog: rename parse_stdin_ident()Jeff King, Sep 25, 2020
  16. 7/8 shortlog: parse trailer identsJeff King, Sep 25, 2020
  17. 8/8 shortlog: allow multiple groups to be specifiedJeff King, Sep 25, 2020
  18. Eric SunshineSep 25, 2020
  19. Jeff KingSep 27, 2020
  20. Martin ÅgrenSep 26, 2020
  21. Jeff KingSep 27, 2020
  22. Derrick StoleeSep 25, 2020
  23. Junio C HamanoSep 25, 2020
  24. 0/8 parsing trailers with shortlogJeff King, Sep 27, 2020
  25. 1/8 shortlog: change "author" variables to "ident"Jeff King, Sep 27, 2020
  26. Junio C HamanoSep 27, 2020
  27. 2/8 shortlog: add grouping optionJeff King, Sep 27, 2020
  28. 3/8 trailer: add interface for iterating over commit trailersJeff King, Sep 27, 2020
  29. 4/8 shortlog: match commit trailers with --groupJeff King, Sep 27, 2020
  30. Junio C HamanoSep 27, 2020
  31. Jeff KingSep 28, 2020
  32. Junio C HamanoSep 28, 2020
  33. 5/8 shortlog: de-duplicate trailer valuesJeff King, Sep 27, 2020
  34. Junio C HamanoSep 27, 2020
  35. Jeff KingSep 28, 2020
  36. 6/8 shortlog: rename parse_stdin_ident()Jeff King, Sep 27, 2020
  37. 7/8 shortlog: parse trailer identsJeff King, Sep 27, 2020
  38. Junio C HamanoSep 27, 2020
  39. 8/8 shortlog: allow multiple groups to be specifiedJeff King, Sep 27, 2020
  40. Junio C HamanoSep 27, 2020
  41. Jeff KingSep 28, 2020
  42. Junio C HamanoDec 28, 2020
  43. Junio C HamanoFeb 4, 2021
  44. Martin ÅgrenSep 27, 2020

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.