shortlog: introduce --email-only, --group-filter options

These couple of patches introduce a pair of new options to git shortlog. They are:

- --group-filter, which restricts the output to group(s) of interest

- --email-only, which allows callers to splice emails out of groups, which is useful when grouping based on trailers that usually contain identities (e.g. "Co-authored-by"), where the search is name-insensitive and only cares about a strict match on the email.

Let me know what you think, and thanks in advance for your review.

Taylor Blau (2):
shortlog: introduce --group-filter to restrict output
shortlog: introduce --email-only to only show emails

 Documentation/git-shortlog.txt | 10 +++++
 builtin/shortlog.c             | 53 +++++++++++++++++-----
 shortlog.h                     |  3 ++
 t/t4201-shortlog.sh            | 82 ++++++++++++++++++++++++++++++++++
 4 files changed, 138 insertions(+), 10 deletions(-)

2.41.0.1.gf123b68cb8

Sometimes it is useful to ask, "how many commits have I authored or been mentioned in via the Co-authored-by trailer"? git shortlog is a reasonable tool for this, and the answer can easily be obtained by running:

    $ git shortlog -ns --group=author --group=trailer:Co-authored-by

and reading off the corresponding value from its output. But what if you want to know which commits contribute to the above count? You can drop the -s option and parse through the results, but you'll have to skip past everything you don't want (and stop reading after matching everything you do want).

That is a script-able task, but it is cumbersome, and potentially very slow, if there is a large amount of output that does not match the given query.

Instead, this patch introduces --group-filter in order to restrict the output of git shortlog to only matching group(s). Items match if they are in a group which is strictly equal to one of the specified filters.

This means that you could easily view the hashes of all commits you either wrote or co-authored with something like:

    $ git shortlog -n --group=author --group=trailer:Co-authored-by \
        --group-filter="$(git config user.name)"

When filtering just by trailers, it is tempting to want to introduce a new grep mode for matching a given trailer, like --author=<pattern> for matching the author header. But this would not be suitable for the above, since we want commits which match either the author or the Co-authored-by trailer, not ones which match both.

An alternative approach might be to implement trailer filtering as above in revision.c, and show commits matching either the --author value or some hypothetical --trailer filter. That would give shortlog fewer commits, which may improve its performance. But it would restrict the interpretation of these options to be an OR (i.e. show commits matching either the --author or --trailer field).

In fact, this is already not possible to do, since the --author and --committer options are documented as:

So introducing another option which changes the behavior of existing ones is a non-starter.

Instead, git shortlog will process more commits than necessary. But this is a marginal cost, since implementing the hypothetical revision options from above as an OR would mean that revision.c has to process every commit anyway.

---
 Documentation/git-shortlog.txt |  5 ++++
 builtin/shortlog.c             | 20 ++++++++++++-
 shortlog.h                     |  2 ++
 t/t4201-shortlog.sh            | 54 ++++++++++++++++++++++++++++++++++
 4 files changed, 80 insertions(+), 1 deletion(-)

index 7d0277d033..dab6d09648 100644
@@ -97,6 +97,11 @@ counts both authors and co-authors.
 If width is `0` (zero) then indent the lines of the output without wrapping
 them.
 
+--group-filter=<group>::
+	Only show output from the given group. If given more than once,
+	show output from any of the previously specified groups. May be
+	cleared with `--no-group-filter`.
+
 <revision-range>::
 	Show only commits in the specified revision range.  When no
 	<revision-range> is specified, it defaults to `HEAD` (i.e. the

index 46f4e0832a..679db22c57 100644
@@ -365,6 +365,7 @@ void shortlog_init(struct shortlog *log)
 	log->trailers.strdup_strings = 1;
 	log->trailers.cmp = strcasecmp;
 	log->format.strdup_strings = 1;
+	log->group_filter.strdup_strings = 1;
 }
 
 void shortlog_finish_setup(struct shortlog *log)

@@ -377,6 +378,7 @@ void shortlog_finish_setup(struct shortlog *log)
 				   log->email ? "%cN <%cE>" : "%cN");
 
 	string_list_sort(&log->trailers);
+	string_list_sort(&log->group_filter);
 }
 
 int cmd_shortlog(int argc, const char **argv, const char *prefix)

@@ -400,6 +402,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 			&parse_wrap_args),
 		OPT_CALLBACK(0, "group", &log, N_("field"),
 			N_("group by field"), parse_group_option),
+		OPT_STRING_LIST(0, "group-filter", &log.group_filter,
+				N_("group"), N_("only show matching groups")),
 		OPT_END(),
 	};
 

@@ -476,6 +480,13 @@ static void add_wrapped_shortlog_msg(struct strbuf *sb, const char *s,
 	strbuf_addch(sb, '\n');
 }
 
+static int want_shortlog_group(struct shortlog *log, const char *group)
+{
+	if (!log->group_filter.nr)
+		return 1;
+	return string_list_has_string(&log->group_filter, group);
+}
+
 void shortlog_output(struct shortlog *log)
 {
 	size_t i, j;

@@ -486,6 +497,9 @@ void shortlog_output(struct shortlog *log)
 		      log->summary ? compare_by_counter : compare_by_list);
 	for (i = 0; i < log->list.nr; i++) {
 		const struct string_list_item *item = &log->list.items[i];
+		if (!want_shortlog_group(log, item->string))
+			goto next;
+
 		if (log->summary) {
 			fprintf(log->file, "%6d\t%s\n",
 				(int)UTIL_TO_INT(item), item->string);

@@ -505,11 +519,15 @@ void shortlog_output(struct shortlog *log)
 					fprintf(log->file, "      %s\n", msg);
 			}
 			putc('\n', log->file);
+		}
+
+next:
+		if (!log->summary) {
+			struct string_list *onelines = item->util;
 			onelines->strdup_strings = 1;
 			string_list_clear(onelines, 0);
 			free(onelines);
 		}
-
 		log->list.items[i].util = NULL;
 	}
 

index 28d04f951a..8ebee0e2d6 100644
@@ -18,6 +18,8 @@ struct shortlog {
 	int abbrev;
 	struct date_mode date_mode;
 
+	struct string_list group_filter;
+
 	enum {
 		SHORTLOG_GROUP_AUTHOR = (1 << 0),
 		SHORTLOG_GROUP_COMMITTER = (1 << 1),

2.41.0.1.gf123b68cb8

When a shortlog caller wants to group output by, say, author email, they can easily express this with:

    $ git shortlog --group=format:%ae

and restrict output to specific email(s) with the new --group-filter option introduced by the previous commit.

But they are not able to apply the same treatment to identities that appear in trailers. Doing:

    $ git shortlog -e --group=format:%ae --group=trailer:Co-authored-by

will produce funky results, interspersing proper emails with full "Name <email>" identities from the Co-authored-by trailer (or anything else that might appear there), like:

461 [email protected]
11 Taylor Blau <[email protected]>

So if the caller wants to restrict output to a set of matching email addresses (say, "[email protected]"), they cannot do it with a --group-filter, since it would discard the group "Taylor Blau <[email protected]>".

Introduce a new --email-only option, which extracts the email component of an identity from all shortlog groups, including trailers. It behaves similarly to the -e option, but replaces its output with just the email component, instead of adding it on to the end.

Now, shortlog callers can perform:

    $ git shortlog -s --group=author --group=trailer:Co-authored-by \
        --email-only --group-filter="<[email protected]>"
       472  <[email protected]>

to obtain the output they want.

---
 Documentation/git-shortlog.txt |  5 +++++
 builtin/shortlog.c             | 33 ++++++++++++++++++++++++---------
 shortlog.h                     |  1 +
 t/t4201-shortlog.sh            | 28 ++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 9 deletions(-)

index dab6d09648..160c11aead 100644
@@ -39,6 +39,11 @@ OPTIONS
 --email::
 	Show the email address of each author.
 
+--email-only::
+	Show only the email address of each author, or committer. If
+	using a different kind of group (e.g. trailers, custom format,
+	etc.), only values which contain name/email-pairs will be shown.
+
 --format[=<format>]::
 	Instead of the commit subject, use some other information to
 	describe each commit.  '<format>' can be any string accepted

index 679db22c57..bbe01a376f 100644
@@ -108,9 +108,13 @@ static int parse_ident(struct shortlog *log,
 	maillen = ident.mail_end - ident.mail_begin;
 
 	map_user(&log->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
-	strbuf_add(out, namebuf, namelen);
-	if (log->email)
-		strbuf_addf(out, " <%.*s>", (int)maillen, mailbuf);
+	if (log->email_only) {
+		strbuf_addf(out, "<%.*s>", (int)maillen, mailbuf);
+	} else {
+		strbuf_add(out, namebuf, namelen);
+		if (log->email)
+			strbuf_addf(out, " <%.*s>", (int)maillen, mailbuf);
+	}
 
 	return 0;
 }

@@ -198,6 +202,8 @@ static void insert_records_from_trailers(struct shortlog *log,
 		strbuf_reset(&ident);
 		if (!parse_ident(log, &ident, value))
 			value = ident.buf;
+		else if (log->email_only)
+			continue;
 
 		if (!strset_add(dups, value))
 			continue;

@@ -370,12 +376,19 @@ void shortlog_init(struct shortlog *log)
 
 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");
+	if (log->email_only) {
+		if (log->groups & SHORTLOG_GROUP_AUTHOR)
+			string_list_append(&log->format, "<%aE>");
+		if (log->groups & SHORTLOG_GROUP_COMMITTER)
+			string_list_append(&log->format, "<%cE>");
+	} else {
+		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");
+	}
 
 	string_list_sort(&log->trailers);
 	string_list_sort(&log->group_filter);

@@ -397,6 +410,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 			 N_("suppress commit descriptions, only provides commit count")),
 		OPT_BOOL('e', "email", &log.email,
 			 N_("show the email address of each author")),
+		OPT_BOOL(0, "email-only", &log.email_only,
+			 N_("only show the email address of each author")),
 		OPT_CALLBACK_F('w', NULL, &log, N_("<w>[,<i1>[,<i2>]]"),
 			N_("linewrap output"), PARSE_OPT_OPTARG,
 			&parse_wrap_args),

index 8ebee0e2d6..3fb28639c3 100644
@@ -30,6 +30,7 @@ struct shortlog {
 	struct string_list format;
 
 	int email;
+	int email_only;
 	struct string_list mailmap;
 	FILE *file;
 };

index 0695c42ca8..d747a402ff 100755
@@ -400,6 +400,34 @@ test_expect_success '--no-group-filter reset group filters' '
 	test_cmp expect actual
 '
 
+test_expect_success '--email-only shows emails from author' '
+	cat >expect <<-\EOF &&
+	     2	<[email protected]>
+	EOF
+	git shortlog -ns --group=author --email-only -2 HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--email-only shows emails from committer' '
+	cat >expect <<-\EOF &&
+	     2	<[email protected]>
+	EOF
+	git shortlog -ns --group=committer --email-only -2 HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--email-only shows emails from trailers with idents' '
+	cat >expect <<-\EOF &&
+	     1	<[email protected]>
+	     1	<[email protected]>
+	EOF
+	# at this point, HEAD~3 has a trailer "Repeated-trailer: Foo",
+	# which is not shown here since it cannot be parsed as an ident
+	git shortlog -ns --group=trailer:some-trailer --email-only -3 \
+		HEAD >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'shortlog can match multiple format groups' '
 	GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME" \
 		git commit --allow-empty -m "identical names" &&

2.41.0.1.gf123b68cb8

Am 08.06.23 um 01:02 schrieb Taylor Blau:

A note from the peanut gallery: "--email-only" sounds like an option that affects the output of the command. But it does not (IIUC), there is no hint that it affects grouping and filtering. It is named too generically, IMHO. Can we not have the desired effect by specifying some token to one of the --group* options?

-- Hannes

One thing that is not immediately obvious from reading the patch, but becomes clearer in patch 2, is that your --group-filter is an exact string match. This differs from the --author filter in 'git log' and similar, which is actually a case-insensitive substring match.

{
	struct string_list_item *item;
	if (!log->group_filter.nr)
		return 1;

	for_each_string_list_item(item, &log->group_filter) {
		if (strcasestr(group, item->string))
			return 1;
	}

	return 0;
}

Yeah, funnily enough I originally thought about adding a --trailer flag to the revision machinery, but realized that it was a non-starter since --author is documented as only showing commits matching that author.

So that doesn't solve for the case of finding a commit which has the identity you want buried in some trailer, but is written by a different author. shortlog needs to see all commits along the traversal, so I went with the filtering approach instead.

Thanks for the pointer on beefing up the documentation to indicate that this is an exact search.

Yeah, thanks for raising it. I wonder if there are other semantics that don't incorrectly return substring matches. You could conceivably extend the --group-filter argument to take an extended regular expression, and then just match on whatever's inside the last "<>".

That approach was one that I considered, but feels a little heavyweight for what we're trying to do here. One potential approach is to keep things as-is, and then consider extending --group-filter to mean "extended regular expression" in the future. Although I'm not sure that would work, either, since fixed-string matches would suddenly match anywhere in the string, breaking backwards compatibility.

You could add another option that specifies the behavior of --group-filter, or add some extra bit of information there like --group-filter=egrep:<pattern> or something.

At least there's no shortage of options ;-).

Thanks,
Taylor

I had thought about the possibility of something like this since we (a) already have lots of %(trailer)-related pretty formatting options, and
(b) support grouping by pretty formatted-commits with the --group=format:<format> option I introduced via c112d8d9c2 (Merge branch 'tb/shortlog-group', 2022-10-30).

Thanks,
Taylor

Good observation. I had exactly the same reaction.