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

[William Hubbs <williamh@gentoo.org>]

On Tue, Feb 05, 2019 at 10:14:37PM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> 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?

I attempted to save your patches to apply them, but didn't have any luck

Also, according to Junio's report, my patch is already merged to next,
so if you give me your patch based on that, I'll take a look, but it may
be a day or so.

William