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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Feb 05 2019, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> +static int set_ident_internal(const char *var, const char *value,
>> +			    struct strbuf *sb, const int flag)
>> +{
>> +	if (!value)
>> +		return config_error_nonbool(var);
>> +	strbuf_reset(sb);
>> +	strbuf_addstr(sb, value);
>> +	author_ident_explicitly_given |= flag;
>> +	ident_config_given |= flag;
>> +	return 0;
>> +}
>> +
>> +static int set_ident(const char *var, const char *value)
>> +{
>> +	if (!strcmp(var, "author.name"))
>> +		return set_ident_internal(var, value, &git_author_name,
>> +					  IDENT_NAME_GIVEN);
>> +	else if (!strcmp(var, "author.email"))
>> +		return set_ident_internal(var, value, &git_author_email,
>> +					  IDENT_MAIL_GIVEN);
>> +	else if (!strcmp(var, "committer.name"))
>> +		return set_ident_internal(var, value, &git_committer_name,
>> +					  IDENT_NAME_GIVEN);
>> +	else if (!strcmp(var, "committer.email"))
>> +		return set_ident_internal(var, value, &git_committer_email,
>> +					  IDENT_MAIL_GIVEN);
>> +	else if (!strcmp(var, "user.name"))
>> +		return set_ident_internal(var, value, &git_default_name,
>> +					  IDENT_NAME_GIVEN);
>> +	else if (!strcmp(var, "user.email"))
>> +		return set_ident_internal(var, value, &git_default_email,
>> +					  IDENT_MAIL_GIVEN);
>> +	return 0;
>> +}
>
> In the v5 patch from William, author_ident_explicitly_given and
> committer_ident_explicitly_given were set separately depending on
> what variable was given (e.g. user.name marked both, author.name
> marked only author but not committer_ident_explicitly_given).  In
> the original before the addition of this feature with v6, giving
> user.name would have set both, as we can see below.
>
> Is this change intended?
>
> Or did you find that committer_ident_explicitly_given is no longer
> useful and the variable is not used anymore?

No, that's a mistake of mine when porting this over, but also clearly a
blindspot in our tests since they all pass with this.

I haven't dug (don't have time right now) to check what the effect of
that is. William?

>>  int git_ident_config(const char *var, const char *value, void *data)
>>  {
>>  	if (!strcmp(var, "user.useconfigonly")) {
>> @@ -480,29 +551,7 @@ int git_ident_config(const char *var, const char *value, void *data)
>>  		return 0;
>>  	}
>>
>> -	if (!strcmp(var, "user.name")) {
>> -		if (!value)
>> -			return config_error_nonbool(var);
>> -		strbuf_reset(&git_default_name);
>> -		strbuf_addstr(&git_default_name, value);
>> -		committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
>> -		author_ident_explicitly_given |= IDENT_NAME_GIVEN;
>> -		ident_config_given |= IDENT_NAME_GIVEN;
>> -		return 0;
>> -	}
>> -
>> -	if (!strcmp(var, "user.email")) {
>> -		if (!value)
>> -			return config_error_nonbool(var);
>> -		strbuf_reset(&git_default_email);
>> -		strbuf_addstr(&git_default_email, value);
>> -		committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
>> -		author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
>> -		ident_config_given |= IDENT_MAIL_GIVEN;
>> -		return 0;
>> -	}
>> -
>> -	return 0;
>> +	return set_ident(var, value);
>>  }