Re: [PATCH v2] help.c: configurable suggestions

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

Drew DeVault <sir@cmpwn.com> writes:

> -	if (!strcmp(var, "help.autocorrect"))
> -		autocorrect = git_config_int(var,value);
> +	if (!strcasecmp(var, "help.autocorrect")) {

This is an unwarranted change.  The first part and the last part of
a configuration variable name are downcased before the config
callback functions are called, so strcmp() is sufficient.

> +		if (!value)
> +			return config_error_nonbool(var);

This is to protect us against

	[help]
		autocorrect

(i.e. "true" expressed without "= true"), which is not strictly
needed when the only thing we do with value is git_config_int()
because it knows how to handle value==NULL case.  But it starts
to matter when parsing variables with more elaborate shape, like
"int or some known token".

And because we want to keep the promise "if you write negative
value, it means immediate" we made to our users, we can instead make
this variable "number or 'never'".  To do so, keep that "barf if
NULL" check above, and then add something like:

		if (!strcmp(value, "never"))
			autocorrect = AUTOCORRECT_NEVER;
		else {
			int v = git_config_int(var, value);
                        autocorrect = (v < 0) 
				? AUTOCORRECT_IMMEDIATELY : v;
		}

The configuration the end user has may say '-2', but that is
different from 'never', so instead of leaving a negative value as
is, like this code below does ...

> +		autocorrect = git_config_int(var, value);

... we normalize any negative value the user gave us to
AUTOCORRECT_IMMEDIATELY.  That way, we can keep configuration the
user has working the same way as before, while allowing a new value
'never' to have a new meaning.

We might want to further make the variable "number, 'never' or
'immediate'" for consistency, with an eye on the possibility that we
might eventually want to deprecate the "negative means immediate".

To do so, we could do:

		if (!strcmp(value, "never")) {
			autocorrect = AUTOCORRECT_NEVER;
		} else if (!strcmp(value, "immediate")) {
			autocorrect = AUTOCORRECT_IMMEDIATELY;
		} else {
			int v = git_config_int(var, value);
                        autocorrect = (v < 0) 
				? AUTOCORRECT_IMMEDIATELY : v;
		}

instead.