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

[Junio C Hamano <gitster@pobox.com>]

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

> 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.