Re: [PATCH v4 1/4] Add log.mailmap as configurational option for mailmap location

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 5, 2009, 17:44 UTC
Message-ID: <7vljskeq21.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <565c86bdbc8d6303d7d468fa196fb54ff7bd2352.1233819451.git.marius@trolltech.com>

Marius Storm-Olsen <marius@trolltech.com> writes:

Show 25 quoted lines

> diff --git a/config.c b/config.c
> index 790405a..9ebcbbe 100644
> --- a/config.c
> +++ b/config.c
> @@ -565,6 +565,13 @@ static int git_default_branch_config(const char *var, const char *value)
>  	return 0;
>  }
>  
> +static int git_default_log_config(const char *var, const char *value)
> +{
> +	if (!strcmp(var, "log.mailmap"))
> +		return git_config_string(&git_log_mailmap, var, value);
> +	return 0;
> +}
> +
>  int git_default_config(const char *var, const char *value, void *dummy)
>  {
>  	if (!prefixcmp(var, "core."))
> @@ -579,6 +586,9 @@ int git_default_config(const char *var, const char *value, void *dummy)
>  	if (!prefixcmp(var, "branch."))
>  		return git_default_branch_config(var, value);
>  
> +	if (!prefixcmp(var, "log."))
> +		return git_default_log_config(var, value);
> +

The placement of this looked *really* wrong, as mailmap is not *that* important to most of the commands.

Initially I wondered if this should be better done inside existing git_log_config(). I suspect that the reason you didn't do so is because you would want to use this also in blame, which is not part of the log family, and does not use git_log_config() (nor it should).

Which probably means that the code can stay here (it is just two strcmp and assignment to a pointer variable), but also suggests that log.mailmap is perhaps misnamed.