Re: [PATCH v4] fetch.c: defer fetch.followRemoteHEAD validation

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

Colin Hinton <colinlewishinton@gmail.com> writes:

>  	if (!strcmp(k, "fetch.followremotehead")) {
> +		free(fetch_config->follow_remote_head_raw);
>  		if (!v)
> +			fetch_config->follow_remote_head_raw = xstrdup("");
>  		else
> +			fetch_config->follow_remote_head_raw = xstrdup(v);

Hmph, this means that the code cannot distinguish between

	[fetch] followremotehead

	[fetch] followremotehead = ""

It would be less code and more expressive if you lost the
conditional, i.e.,

	if (!strcmp(k, "fetch.followremotehead"))
		free(fetch_config->follow_remote_head_raw);
		fetch_config->follow_remote_head_raw = xstrdup_or_null(v);
	}

> +static enum follow_remote_head_settings get_follow_remote_head(const char *setting)
> +{
> +	if (!setting || !*setting)
> +		die(_("missing value for 'fetch.followRemoteHEAD'"));

Then you can differenciate

	if (!setting)
		... we got '[fetch] followRemoteHEAD' ...
		die() as before, complaining that the this is not a Bool.
	else if (!*setting)
		... we got '[fetch] followRemoteHEAD = ""' ...

if we wanted to.  It probably do not need to check for an empty
string as it will fall through the "else if" cascade below and
eventually end up with the warning + default.  

> +	else if (!strcmp(setting, "never"))
> +		return FOLLOW_REMOTE_NEVER;
> +	else if (!strcmp(setting, "create"))
> +		return FOLLOW_REMOTE_CREATE;
> +	else if (!strcmp(setting, "warn"))
> +		return FOLLOW_REMOTE_WARN;
> +	else if (!strcmp(setting, "always"))
> +		return FOLLOW_REMOTE_ALWAYS;
> +	warning(_("unrecognized fetch.followRemoteHEAD value '%s' ignored"), setting);
> +	return BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
> +}