Re: [PATCH 1/2] ident: check for useConfigOnly before auto-detection of name/email

From: Jeff King <peff@peff.net>
Date: Mar 31, 2016, 14:40 UTC
Message-ID: <20160331144003.GE31116@sigill.intra.peff.net>
In-Reply-To: <1459366183-15451-1-git-send-email-redneb@gmx.com>

On Wed, Mar 30, 2016 at 10:29:42PM +0300, Marios Titas wrote:

> If user.useConfigOnly is set, it does not make sense to try to
> auto-detect the name and/or the email. So it's better to do the
> useConfigOnly checks first.

It might be nice to explain how it is better here. I'd guess it is because we may fail during xgetpwuid(), giving a message that is much less informative?

Show 16 quoted lines

> @@ -374,14 +374,14 @@ const char *fmt_ident(const char *name, const char *email,
>  	}
>  
>  	if (!email) {
> +		if (strict && ident_use_config_only
> +		    && !(ident_config_given & IDENT_MAIL_GIVEN))
> +			die("user.useConfigOnly set but no mail given");
>  		email = ident_default_email();
>  		if (strict && default_email_is_bogus) {
>  			fputs(env_hint, stderr);
>  			die("unable to auto-detect email address (got '%s')", email);
>  		}
> -		if (strict && ident_use_config_only
> -		    && !(ident_config_given & IDENT_MAIL_GIVEN))
> -			die("user.useConfigOnly set but no mail given");
>  	}

I wondered on this hunk whether ident_default_email() could ever set the IDENT_MAIL_GIVEN flag. It _does_ set it, but only for "explicitly_given", not for "config_given", which makes sense.

So I think this is doing the right thing.

-Peff