"Scott Chacon via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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.
