[PATCH 3/3] commit: show interesting ident information in summary

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



There are a few cases of user identity information that we
consider interesting:

  1. When the author and committer identities do not match.

  2. When the committer identity was picked automatically
     from the username, hostname and GECOS information.

In these cases, we already show the information in the
commit message template. However, users do not always see
that template because they might use "-m" or "-F". With this
patch, we show these interesting cases after the commit,
along with the subject and change summary. The new output
looks like:

  $ git commit \
      -m "federalist papers" \
      --author='Publius <alexander@xxxxxxxxxxxx>'
  [master 3d226a7] federalist papers
   Author: Publius <alexander@xxxxxxxxxxxx>
   1 files changed, 1 insertions(+), 0 deletions(-)

for case (1), and:

  $ git config --global --unset user.name
  $ git config --global --unset user.email
  $ git commit -m foo
  [master 7c2a927] foo
   Committer: Jeff King <peff@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
   1 files changed, 1 insertions(+), 0 deletions(-)

for case (2).

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
Note that this has a slight semantic conflict with the jc/ident topic in
next. The user_ident_explicitly_given flag needs to be compared to
IDENT_ALL.

I hope the example output in the commit message is not too verbose. I
was recently reviewing somebody's series that made output changes, and
they didn't include sample output anywhere, which made reviewing a lot
more annoying.

Personally I don't care much about case (2) one way or the other, but it
is the one that triggered this thread. I think case (1) is very useful,
though.

I tested case (2) manually, but I didn't include anything in the test
suite; I feel funny testing output created from the hostname and GECOS
(can't it even barf if the user's system isn't set up very well? That
would produce a false negative for the test).

 builtin-commit.c  |   25 ++++++++++++++++++++++---
 t/t7501-commit.sh |    6 +++++-
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 073fe90..279145d 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1046,9 +1046,12 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 {
 	struct rev_info rev;
 	struct commit *commit;
-	static const char *format = "format:%h] %s";
+	struct strbuf format = STRBUF_INIT;
 	unsigned char junk_sha1[20];
 	const char *head = resolve_ref("HEAD", junk_sha1, 0, NULL);
+	struct pretty_print_context pctx = {0};
+	struct strbuf author_ident = STRBUF_INIT;
+	struct strbuf committer_ident = STRBUF_INIT;
 
 	commit = lookup_commit(sha1);
 	if (!commit)
@@ -1056,6 +1059,21 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 	if (!commit || parse_commit(commit))
 		die("could not parse newly created commit");
 
+	strbuf_addstr(&format, "format:%h] %s");
+
+	format_commit_message(commit, "%an <%ae>", &author_ident, &pctx);
+	format_commit_message(commit, "%cn <%ce>", &committer_ident, &pctx);
+	if (strbuf_cmp(&author_ident, &committer_ident)) {
+		strbuf_addstr(&format, "\n Author: ");
+		strbuf_percentquote_buf(&format, &author_ident);
+	}
+	if (!user_ident_explicitly_given) {
+		strbuf_addstr(&format, "\n Committer: ");
+		strbuf_percentquote_buf(&format, &committer_ident);
+	}
+	strbuf_release(&author_ident);
+	strbuf_release(&committer_ident);
+
 	init_revisions(&rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
 
@@ -1066,7 +1084,8 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 
 	rev.verbose_header = 1;
 	rev.show_root_diff = 1;
-	get_commit_format(format, &rev);
+	get_commit_format(format.buf, &rev);
+	strbuf_release(&format);
 	rev.always_show_header = 0;
 	rev.diffopt.detect_rename = 1;
 	rev.diffopt.rename_limit = 100;
@@ -1085,7 +1104,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 		struct pretty_print_context ctx = {0};
 		struct strbuf buf = STRBUF_INIT;
 		ctx.date_mode = DATE_NORMAL;
-		format_commit_message(commit, format + 7, &buf, &ctx);
+		format_commit_message(commit, format.buf + 7, &buf, &ctx);
 		printf("%s\n", buf.buf);
 		strbuf_release(&buf);
 	}
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index a529701..7940901 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -117,7 +117,11 @@ test_expect_success \
 test_expect_success \
 	"overriding author from command line" \
 	"echo 'gak' >file && \
-	 git commit -m 'author' --author 'Rubber Duck <rduck@xxxxxxxxxx>' -a"
+	 git commit -m 'author' --author 'Rubber Duck <rduck@xxxxxxxxxx>' -a >output 2>&1"
+
+test_expect_success \
+	"commit --author output mentions author" \
+	"grep Rubber.Duck output"
 
 test_expect_success PERL \
 	"interactive add" \
-- 
1.6.6.138.g309fc.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]