diff mbox series

[2/2] shortlog: introduce `--email-only` to only show emails

Message ID 44179d28fa7676965a28734e20584d54b44e051b.1686178917.git.me@ttaylorr.com (mailing list archive)
State New, archived
Headers show
Series shortlog: introduce --email-only, --group-filter options | expand

Commit Message

Taylor Blau June 7, 2023, 11:02 p.m. UTC
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  me@ttaylorr.com
     11  Taylor Blau <me@ttaylorr.com>

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

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="<me@ttaylorr.com>"
       472  <me@ttaylorr.com>

to obtain the output they want.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/git-shortlog.txt |  5 +++++
 builtin/shortlog.c             | 33 ++++++++++++++++++++++++---------
 shortlog.h                     |  1 +
 t/t4201-shortlog.sh            | 28 ++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 9 deletions(-)

Comments

Johannes Sixt June 8, 2023, 7:35 a.m. UTC | #1
Am 08.06.23 um 01:02 schrieb Taylor Blau:
> 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  me@ttaylorr.com
>      11  Taylor Blau <me@ttaylorr.com>
> 
> So if the caller wants to restrict output to a set of matching email
> addresses (say, "me@ttaylorr.com"), they cannot do it with a
> `--group-filter`, since it would discard the group "Taylor Blau
> <me@ttaylorr.com>".
> 
> 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="<me@ttaylorr.com>"
>        472  <me@ttaylorr.com>
> 
> to obtain the output they want.

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
Taylor Blau June 8, 2023, 4:24 p.m. UTC | #2
On Thu, Jun 08, 2023 at 09:35:14AM +0200, Johannes Sixt wrote:
> 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?

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
Junio C Hamano June 12, 2023, 9:22 p.m. UTC | #3
Johannes Sixt <j6t@kdbg.org> writes:

> A note from the peanut gallery: "--email-only" sounds like an option
> that affects the output of the command.

Good observation.  I had exactly the same reaction.
diff mbox series

Patch

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index dab6d09648..160c11aead 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -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
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 679db22c57..bbe01a376f 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -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),
diff --git a/shortlog.h b/shortlog.h
index 8ebee0e2d6..3fb28639c3 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -30,6 +30,7 @@  struct shortlog {
 	struct string_list format;
 
 	int email;
+	int email_only;
 	struct string_list mailmap;
 	FILE *file;
 };
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 0695c42ca8..d747a402ff 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -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	<author@example.com>
+	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	<committer@example.com>
+	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	<a@example.com>
+	     1	<b@example.com>
+	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" &&