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

Drew DeVault <sir@cmpwn.com> writes:

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.

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 ...

... 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.