diff mbox series

[v2] help: interpret boolean string values for help.autocorrect

Message ID pull.1869.v2.git.git.1736419777235.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series [v2] help: interpret boolean string values for help.autocorrect | expand

Commit Message

Scott Chacon Jan. 9, 2025, 10:49 a.m. UTC 
From: Scott Chacon <schacon@gmail.com>

A help.autocorrect value of 1 is currently interpreted as "wait 1
decisecond", which can be confusing to users who believe they are setting a
boolean value to turn the autocorrect feature on.

Interpret the value of help.autocorrect as either one of the accepted list
of special values ("never", "immediate", ...), a boolean or an integer. If
the value is 1, it is no longer interpreted as a decisecond value of 0.1s
but as a true boolean, the equivalent of "immediate". If the value is 2 or
more, continue treating it as a decisecond wait time.

False boolean string values ("off", "false", "no") are now equivalent to 0,
meaning that guessed values are still shown but nothing is executed (as
opposed to "never", which does not show the guesses). True boolean string
values are interpreted as "immediate".

Signed-off-by: Scott Chacon <schacon@gmail.com>
---
    help: interpret help.autocorrect=1 as "immediate" rather than 0.1s
    
    Took Junio's suggestion to include all boolean values as valid, though
    I'm not interpreting "false" as "never", but instead as 0, as they're
    subtly different. 0 will show the guessed commands and exit, "never"
    will not guess the commands.
    
    Changes since v1:
    
     * Include all boolean values rather than special casing "1"
     * Update the help.txt documentation

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1869%2Fschacon%2Fmaster-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1869/schacon/master-v2
Pull-Request: https://github.com/git/git/pull/1869

Range-diff vs v1:

 1:  dbda79cd4fc < -:  ----------- help: interpret help.autocorrect=1 as "immediate" rather than 0.1s
 -:  ----------- > 1:  07b47b70ded help: interpret boolean string values for help.autocorrect


 Documentation/config/help.txt |  5 +++--
 help.c                        | 18 +++++++++++++++---
 2 files changed, 18 insertions(+), 5 deletions(-)


base-commit: 14650065b76b28d3cfa9453356ac5669b19e706e

Comments

Junio C Hamano Jan. 9, 2025, 4:32 p.m. UTC | #1 
"Scott Chacon via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/Documentation/config/help.txt b/Documentation/config/help.txt
> index 610701f9a37..6d9c2e06908 100644
> --- a/Documentation/config/help.txt
> +++ b/Documentation/config/help.txt
> @@ -11,8 +11,9 @@ help.autoCorrect::
>  	If git detects typos and can identify exactly one valid command similar
>  	to the error, git will try to suggest the correct command or even
>  	run the suggestion automatically. Possible config values are:
> -	 - 0 (default): show the suggested command.
> -	 - positive number: run the suggested command after specified
> +	 - 0, false boolean string: show the suggested command (default).
> +	 - 1, true boolean string: run the suggested command immediately.
> +	 - positive number > 1: run the suggested command after specified
>  deciseconds (0.1 sec).
>  	 - "immediate": run the suggested command immediately.
>  	 - "prompt": show the suggestion and prompt for confirmation to run

Not a problem this patch introduces, but it looed funny to see the
second line abut to the left edge of the page there.

In any case, the updated semantics look quite sensible.

> diff --git a/help.c b/help.c
> index 5483ea8fd29..9e0f66c26dc 100644
> --- a/help.c
> +++ b/help.c
> @@ -573,9 +573,21 @@ static int git_unknown_cmd_config(const char *var, const char *value,
>  		} else if (!strcmp(value, "prompt")) {
>  			cfg->autocorrect = AUTOCORRECT_PROMPT;
>  		} else {
> -			int v = git_config_int(var, value, ctx->kvi);
> -			cfg->autocorrect = (v < 0)
> -				? AUTOCORRECT_IMMEDIATELY : v;
> +			int is_bool;
> +			int v = git_config_bool_or_int(var, value, ctx->kvi, &is_bool);
> +			if (is_bool) {
> +				if (v == 0) {
> +					cfg->autocorrect = 0;
> +				} else {
> +					cfg->autocorrect = AUTOCORRECT_IMMEDIATELY;
> +				}
> +			} else {
> +				if (v < 0 || v == 1) {
> +					cfg->autocorrect = AUTOCORRECT_IMMEDIATELY;
> +				} else {
> +					cfg->autocorrect = v;
> +				}
> +			}
>  		}
>  	}

The flow looks nice, but the pre-context of this hunk starts like
this:

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

IOW, the new code added at the end of the if/else if/ cascade is way
too late.  

	"[help] autocorrect"

that specifies "true" has already been rejected as an error, with a
now-stale error message saying that the variable is not a Boolean.

We may probably want to use git_parse_maybe_bool_text() upfront,
like

	static int parse_autocorrect(const char *value)
	{
		switch (git_parse_maybe_bool_text(value)) {
	        case 1:
			return AUTOCORRECT_IMMEDIATELY;
		case 0:
			return AUTOCORRECT_NEVER;
		default: /* other random text */
			break;
		}
                if (!strcmp(value, "prompt"))
			return AUTOCORRECT_PROMPT;
		...
		if (!strcmp(value, "prompt"))
			return AUTOCORRECT_PROMPT;

                return 0;
	}

and then in git_unknown_cmd_config(), do something like

	if (!strcmp(var, "help.autocorrect")) {
		int v = parse_autocorrect(value);

                if (!v) {
                	v = git_config_int(var, value, ctx->kvi);
                        if (v < 0)
				v = AUTOCORRECT_IMMEDIATELY;
                }
                cfg->autocorrect = v;
	}

perhaps?
diff mbox series

Patch

diff --git a/Documentation/config/help.txt b/Documentation/config/help.txt
index 610701f9a37..6d9c2e06908 100644
--- a/Documentation/config/help.txt
+++ b/Documentation/config/help.txt
@@ -11,8 +11,9 @@  help.autoCorrect::
 	If git detects typos and can identify exactly one valid command similar
 	to the error, git will try to suggest the correct command or even
 	run the suggestion automatically. Possible config values are:
-	 - 0 (default): show the suggested command.
-	 - positive number: run the suggested command after specified
+	 - 0, false boolean string: show the suggested command (default).
+	 - 1, true boolean string: run the suggested command immediately.
+	 - positive number > 1: run the suggested command after specified
 deciseconds (0.1 sec).
 	 - "immediate": run the suggested command immediately.
 	 - "prompt": show the suggestion and prompt for confirmation to run
diff --git a/help.c b/help.c
index 5483ea8fd29..9e0f66c26dc 100644
--- a/help.c
+++ b/help.c
@@ -573,9 +573,21 @@  static int git_unknown_cmd_config(const char *var, const char *value,
 		} else if (!strcmp(value, "prompt")) {
 			cfg->autocorrect = AUTOCORRECT_PROMPT;
 		} else {
-			int v = git_config_int(var, value, ctx->kvi);
-			cfg->autocorrect = (v < 0)
-				? AUTOCORRECT_IMMEDIATELY : v;
+			int is_bool;
+			int v = git_config_bool_or_int(var, value, ctx->kvi, &is_bool);
+			if (is_bool) {
+				if (v == 0) {
+					cfg->autocorrect = 0;
+				} else {
+					cfg->autocorrect = AUTOCORRECT_IMMEDIATELY;
+				}
+			} else {
+				if (v < 0 || v == 1) {
+					cfg->autocorrect = AUTOCORRECT_IMMEDIATELY;
+				} else {
+					cfg->autocorrect = v;
+				}
+			}
 		}
 	}
 	/* Also use aliases for command lookup */