Re: [PATCH 4/3] am, sequencer: stop parsing our own committer ident

From

Jeff King <peff@peff.net>

Date

Oct 23, 2020, 07:45 UTC

Message-ID

<20201023074510.GB2918369@coredump.intra.peff.net>

In-Reply-To

<20201023072630.GA2918369@coredump.intra.peff.net>

On Fri, Oct 23, 2020 at 03:26:30AM -0400, Jeff King wrote:

> By the way, I wondered why we needed to do this parsing at all. The
> patch below does this in a much simpler way. It's a little bit ugly, I
> think, because we have to call getenv() ourselves. But that's the way
> fmt_ident() has always worked. We could probably improve that now that
> it takes a whose_ident flag (before that, it had no idea if we wanted
> author or committer ident).

---
 builtin/am.c |  4 +--
 ident.c      | 47 ++++++++++++++++-------------------
 sequencer.c  |  4 +--
 3 files changed, 23 insertions(+), 32 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 52206bc56b..9f0d1b4859 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1580,9 +1580,7 @@ static void do_commit(const struct am_state *state)
 			IDENT_STRICT);
 
 	if (state->committer_date_is_author_date)
-		committer = fmt_ident(getenv("GIT_COMMITTER_NAME"),
-				      getenv("GIT_COMMITTER_EMAIL"),
-				      WANT_COMMITTER_IDENT,
+		committer = fmt_ident(NULL, NULL, WANT_COMMITTER_IDENT,
 				      state->ignore_date ? NULL
 							 : state->author_date,
 				      IDENT_STRICT);
diff --git a/ident.c b/ident.c
index 6aba4b5cb6..7743c1ed05 100644
--- a/ident.c
+++ b/ident.c
@@ -384,6 +384,12 @@ const char *fmt_ident(const char *name, const char *email,
 	struct strbuf *ident = &ident_pool[index];
 	index = (index + 1) % ARRAY_SIZE(ident_pool);
 
+	if (!email) {
+		if (whose_ident == WANT_AUTHOR_IDENT)
+			email = getenv("GIT_AUTHOR_EMAIL");
+		else if (whose_ident == WANT_COMMITTER_IDENT)
+			email = getenv("GIT_COMMITTER_EMAIL");
+	}
 	if (!email) {
 		if (whose_ident == WANT_AUTHOR_IDENT && git_author_email.len)
 			email = git_author_email.buf;
@@ -405,6 +411,12 @@ const char *fmt_ident(const char *name, const char *email,
 
 	if (want_name) {
 		int using_default = 0;
+		if (!name) {
+			if (whose_ident == WANT_AUTHOR_IDENT)
+				name = getenv("GIT_AUTHOR_NAME");
+			else if (whose_ident == WANT_COMMITTER_IDENT)
+				name = getenv("GIT_COMMITTER_NAME");
+		}
 		if (!name) {
 			if (whose_ident == WANT_AUTHOR_IDENT && git_author_name.len)
 				name = git_author_name.buf;
@@ -449,6 +461,12 @@ const char *fmt_ident(const char *name, const char *email,
 		strbuf_addch(ident, '>');
 	if (want_date) {
 		strbuf_addch(ident, ' ');
+		if (!date_str) {
+			if (whose_ident == WANT_AUTHOR_IDENT)
+				date_str = getenv("GIT_AUTHOR_DATE");
+			else if (whose_ident == WANT_COMMITTER_IDENT)
+				date_str = getenv("GIT_COMMITTER_DATE");
+		}
 		if (date_str && date_str[0]) {
 			if (parse_date(date_str, ident) < 0)
 				die(_("invalid date format: %s"), date_str);
@@ -462,22 +480,7 @@ const char *fmt_ident(const char *name, const char *email,
 
 const char *fmt_name(enum want_ident whose_ident)
 {
-	char *name = NULL;
-	char *email = NULL;
-
-	switch (whose_ident) {
-	case WANT_BLANK_IDENT:
-		break;
-	case WANT_AUTHOR_IDENT:
-		name = getenv("GIT_AUTHOR_NAME");
-		email = getenv("GIT_AUTHOR_EMAIL");
-		break;
-	case WANT_COMMITTER_IDENT:
-		name = getenv("GIT_COMMITTER_NAME");
-		email = getenv("GIT_COMMITTER_EMAIL");
-		break;
-	}
-	return fmt_ident(name, email, whose_ident, NULL,
+	return fmt_ident(NULL, NULL, whose_ident, NULL,
 			IDENT_STRICT | IDENT_NO_DATE);
 }
 
@@ -487,11 +490,7 @@ const char *git_author_info(int flag)
 		author_ident_explicitly_given |= IDENT_NAME_GIVEN;
 	if (getenv("GIT_AUTHOR_EMAIL"))
 		author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-	return fmt_ident(getenv("GIT_AUTHOR_NAME"),
-			 getenv("GIT_AUTHOR_EMAIL"),
-			 WANT_AUTHOR_IDENT,
-			 getenv("GIT_AUTHOR_DATE"),
-			 flag);
+	return fmt_ident(NULL, NULL, WANT_AUTHOR_IDENT, NULL, flag);
 }
 
 const char *git_committer_info(int flag)
@@ -500,11 +499,7 @@ const char *git_committer_info(int flag)
 		committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
 	if (getenv("GIT_COMMITTER_EMAIL"))
 		committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-	return fmt_ident(getenv("GIT_COMMITTER_NAME"),
-			 getenv("GIT_COMMITTER_EMAIL"),
-			 WANT_COMMITTER_IDENT,
-			 getenv("GIT_COMMITTER_DATE"),
-			 flag);
+	return fmt_ident(NULL, NULL, WANT_COMMITTER_IDENT, NULL, flag);
 }
 
 static int ident_is_sufficient(int user_ident_explicitly_given)
diff --git a/sequencer.c b/sequencer.c
index 07321a7d95..6a8a5c077b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1458,9 +1458,7 @@ static int try_to_commit(struct repository *r,
 		} else {
 			reset_ident_date();
 		}
-		committer = fmt_ident(getenv("GIT_COMMITTER_NAME"),
-				      getenv("GIT_COMMITTER_EMAIL"),
-				      WANT_COMMITTER_IDENT,
+		committer = fmt_ident(NULL, NULL, WANT_COMMITTER_IDENT,
 				      opts->ignore_date ? NULL : date.buf,
 				      IDENT_STRICT);
 		strbuf_release(&date);