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

[Derrick Stolee <stolee@gmail.com>]

On 2/5/2019 2:52 PM, Ævar Arnfjörð Bjarmason wrote:
> 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