Re: [PATCH v6 2/2] config: allow giving separate author and committer idents

From: Derrick Stolee <stolee@gmail.com>
Date: Apr 15, 2019, 14:24 UTC
Message-ID: <15f17152-fcec-ee9a-acd3-f36cf2c98030@gmail.com>
In-Reply-To: <20190205195212.25550-3-avarab@gmail.com>

On 2/5/2019 2:52 PM, Ævar Arnfjörð Bjarmason wrote:

Show 23 quoted lines

> From: William Hubbs <williamh@gentoo.org>
> -const char *fmt_name(const char *name, const char *email)
> +const char *fmt_name(enum want_ident whose_ident)
>  {
> -	return fmt_ident(name, email, NULL, IDENT_STRICT | IDENT_NO_DATE);
> +	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,
> +			IDENT_STRICT | IDENT_NO_DATE);
>  }

William and Ævar,

The "WANT_AUTHOR_IDENT" block of this switch statement does not appear to be hit by any tests, despite the tests included in this patch. My guess is that it is ignored because we have the following code in builtin/commit.c:

static void determine_author_info(struct strbuf *author_ident)
{
	char *name, *email, *date;
	struct ident_split author;

	name = xstrdup_or_null(getenv("GIT_AUTHOR_NAME"));
	email = xstrdup_or_null(getenv("GIT_AUTHOR_EMAIL"));
	date = xstrdup_or_null(getenv("GIT_AUTHOR_DATE"));
...

This is likely overriding the need to use fmt_name. Should we de-duplicate this use of the environment variable by using your new method at this spot in builtin/commit.c?

Thanks, -Stolee