Re: [PATCH] help: interpret help.autocorrect=1 as "immediate" rather than 0.1s

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

"Scott Chacon via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This patch simply interprets a "1" value as the same as the "immedate"
> autocorrect setting, which makes it skip the 0.1s and simply say that it's
> running the command, which is almost certainly what everyone setting it to
> that value is actually trying to do.

It is a cute hack, but special casing a string that is a single
letter "1" in a value that can take a number smells somewhat bad to
me X-<.  If we were redoing this from the start, we would probably
pick a better name for the variable (with "delay" somewhere in the
name), but that is water under the bridge.

I however wonder if we should allow people to have their cake and
eat it too.  It currently says it is *not* a boolean, and manually
interpret "never" and other things, ...

		if (!value)
>  			return config_error_nonbool(var);
>  		if (!strcmp(value, "never")) {
>  			cfg->autocorrect = AUTOCORRECT_NEVER;
> -		} else if (!strcmp(value, "immediate")) {
> +		} else if (!strcmp(value, "immediate") || !strcmp(value, "1")) {
>  			cfg->autocorrect = AUTOCORRECT_IMMEDIATELY;
>  		} else if (!strcmp(value, "prompt")) {
>  			cfg->autocorrect = AUTOCORRECT_PROMPT;

... but would it be simpler if we made it an extended boolean, i.e.

    true, yes, on, 1  -> same as "immediate"
    false, no, off, 0 -> same as "never"
    immediate         -> same as what we currently do
    never             -> same as what we currently do
    prompt            -> same as what we currently do
    number            -> same as what we currently do

It would kill many birds with a stone (e.g., help.autocorrect=no
does not work in the current system as anybody would expect, but it
would with the "this is an extended boolean" approach).

I dunno.

Thanks.