{"thread":{"id":"62770","subject":"[PATCH] help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s","startedAt":"2025-01-08T19:31:50Z","lastAt":"2025-02-04T13:38:10Z","messageCount":24,"participants":["Scott Chacon via GitGitGadget","Kristoffer Haugsbakk","Johannes Schindelin","Junio C Hamano","Yongmin","Scott Chacon","Jeff King","Taylor Blau","David Aguilar"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"510201","messageId":"pull.1869.git.git.1736364707068.gitgitgadget@gmail.com","threadId":"62770","inReplyTo":null,"subject":"[PATCH] help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s","fromName":"Scott Chacon via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-08T19:31:46Z","receivedAt":"2025-01-08T19:31:50Z","isPatch":true,"sender":{"key":"schacon@gmail.com","avatar":"https://gravatar.com/avatar/9b13a8a078e1dcf8588c4eea9554445d51ebed6c41b51f56f4d96738130b05c6?d=mp&s=160"},"body":"From: Scott Chacon \n\nMany people confusingly set the \"help.autocorrect\" setting to 1 believing it\nto be a boolean that turns on the autocorrect feature rather than an integer\nvalue of deciseconds wait time. Since it's impossible for a human being to\nreact this quickly, the help message stating that it's waiting for 0.1s\nbefore continuing becomes confusingly comical.\n\nThis patch simply interprets a \"1\" value as the same as the \"immedate\"\nautocorrect setting, which makes it skip the 0.1s and simply say that it's\nrunning the command, which is almost certainly what everyone setting it to\nthat value is actually trying to do.\n\nSigned-off-by: Scott Chacon \n---\n help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1869%2Fschacon%2Fmaster-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1869/schacon/master-v1\nPull-Request: https://github.com/git/git/pull/1869\n\n help.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/help.c b/help.c\nindex 5483ea8fd29..e6576644b99 100644\n--- a/help.c\n+++ b/help.c\n@@ -568,7 +568,7 @@ static int git_unknown_cmd_config(const char *var, const char *value,\n \t\t\treturn config_error_nonbool(var);\n \t\tif (!strcmp(value, \"never\")) {\n \t\t\tcfg->autocorrect = AUTOCORRECT_NEVER;\n-\t\t} else if (!strcmp(value, \"immediate\")) {\n+\t\t} else if (!strcmp(value, \"immediate\") || !strcmp(value, \"1\")) {\n \t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n \t\t} else if (!strcmp(value, \"prompt\")) {\n \t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n\nbase-commit: 14650065b76b28d3cfa9453356ac5669b19e706e\n-- \ngitgitgadget\n"},{"id":"510203","messageId":"52e414a5-e22f-4046-a321-ae9357ab43c3@app.fastmail.com","threadId":"62770","inReplyTo":"pull.1869.git.git.1736364707068.gitgitgadget@gmail.com","subject":"Re: [PATCH] help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-01-08T21:42:07Z","receivedAt":"2025-01-08T21:42:30Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Wed, Jan 8, 2025, at 20:31, Scott Chacon via GitGitGadget wrote:\n> From: Scott Chacon \n>\n> Many people confusingly set the \"help.autocorrect\" setting to 1 believing it\n> to be a boolean that turns on the autocorrect feature rather than an integer\n> value of deciseconds wait time. Since it's impossible for a human being to\n> react this quickly, the help message stating that it's waiting for 0.1s\n> before continuing becomes confusingly comical.\n>\n> This patch simply interprets a \"1\" value as the same as the \"immedate\"\n> autocorrect setting, which makes it skip the 0.1s and simply say that it's\n\nMaybe: s/This patch simply interprets a/Interpret a \"1\"/\n\nFrom “imperative-mood” section in SubmittingPatches.\n\nOr: Interpret \"1\" as \"immediate\"\n\nSince the sentence is getting a bit complex with “as the same as the”.\n\n> running the command, which is almost certainly what everyone setting it to\n> that value is actually trying to do.\n\nThe section in `man git config` should get an update I think.\n\n>\n> Signed-off-by: Scott Chacon \n> ---\n> help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s\n>\n> Published-As:\n> https://github.com/gitgitgadget/git/releases/tag/pr-git-1869%2Fschacon%2Fmaster-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git\n> pr-git-1869/schacon/master-v1\n> Pull-Request: https://github.com/git/git/pull/1869\n>\n> help.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/help.c b/help.c\n> index 5483ea8fd29..e6576644b99 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -568,7 +568,7 @@ static int git_unknown_cmd_config(const char *var,\n> const char *value,\n> \t\t\treturn config_error_nonbool(var);\n> \t\tif (!strcmp(value, \"never\")) {\n> \t\t\tcfg->autocorrect = AUTOCORRECT_NEVER;\n> -\t\t} else if (!strcmp(value, \"immediate\")) {\n> +\t\t} else if (!strcmp(value, \"immediate\") || !strcmp(value, \"1\")) {\n> \t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n> \t\t} else if (!strcmp(value, \"prompt\")) {\n> \t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n>\n> base-commit: 14650065b76b28d3cfa9453356ac5669b19e706e\n> --\n> gitgitgadget\n"},{"id":"510210","messageId":"7553dc2b-7b91-31c9-e86e-aa24d3bb4235@gmx.de","threadId":"62770","inReplyTo":"pull.1869.git.git.1736364707068.gitgitgadget@gmail.com","subject":"Re: [PATCH] help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-01-09T00:18:15Z","receivedAt":"2025-01-09T00:18:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Scott,\n\nOn Wed, 8 Jan 2025, Scott Chacon via GitGitGadget wrote:\n\n> From: Scott Chacon \n>\n> Many people confusingly set the \"help.autocorrect\" setting to 1 believing it\n> to be a boolean that turns on the autocorrect feature rather than an integer\n> value of deciseconds wait time. Since it's impossible for a human being to\n> react this quickly, the help message stating that it's waiting for 0.1s\n> before continuing becomes confusingly comical.\n>\n> This patch simply interprets a \"1\" value as the same as the \"immedate\"\n> autocorrect setting, which makes it skip the 0.1s and simply say that it's\n> running the command, which is almost certainly what everyone setting it to\n> that value is actually trying to do.\n\nNot trying to brag but I had no problems understanding this commit\nmessage as-is.\n\n> Signed-off-by: Scott Chacon \n> ---\n> help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1869%2Fschacon%2Fmaster-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1869/schacon/master-v1\n> Pull-Request: https://github.com/git/git/pull/1869\n>\n> help.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/help.c b/help.c\n> index 5483ea8fd29..e6576644b99 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -568,7 +568,7 @@ static int git_unknown_cmd_config(const char *var, const char *value,\n> \t\t\treturn config_error_nonbool(var);\n> \t\tif (!strcmp(value, \"never\")) {\n> \t\t\tcfg->autocorrect = AUTOCORRECT_NEVER;\n> -\t\t} else if (!strcmp(value, \"immediate\")) {\n> +\t\t} else if (!strcmp(value, \"immediate\") || !strcmp(value, \"1\")) {\n\nMakes sense to me!\n\nFor the record, I do think it was a mistake to treat number values as\n\"deciseconds\" here, it is inconsistent with pretty much any other config\nsetting. But I also don't see any way to remediate this design mistake at\nthis stage.\n\nThank you for working on this and making the feature at least a little bit\nmore usable.\n\nCiao,\nJohannes\n\n> \t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n> \t\t} else if (!strcmp(value, \"prompt\")) {\n> \t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n>\n> base-commit: 14650065b76b28d3cfa9453356ac5669b19e706e\n> --\n> gitgitgadget\n>\n>\n"},{"id":"510211","messageId":"xmqq34hsn62t.fsf@gitster.g","threadId":"62770","inReplyTo":"pull.1869.git.git.1736364707068.gitgitgadget@gmail.com","subject":"Re: [PATCH] help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-09T01:12:26Z","receivedAt":"2025-01-09T01:12:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Scott Chacon via GitGitGadget\" writes:\n\n> This patch simply interprets a \"1\" value as the same as the \"immedate\"\n> autocorrect setting, which makes it skip the 0.1s and simply say that it's\n> running the command, which is almost certainly what everyone setting it to\n> that value is actually trying to do.\n\nIt is a cute hack, but special casing a string that is a single\nletter \"1\" in a value that can take a number smells somewhat bad to\nme X-<. If we were redoing this from the start, we would probably\npick a better name for the variable (with \"delay\" somewhere in the\nname), but that is water under the bridge.\n\nI however wonder if we should allow people to have their cake and\neat it too. It currently says it is *not* a boolean, and manually\ninterpret \"never\" and other things, ...\n\n\t\tif (!value)\n> \t\t\treturn config_error_nonbool(var);\n> \t\tif (!strcmp(value, \"never\")) {\n> \t\t\tcfg->autocorrect = AUTOCORRECT_NEVER;\n> -\t\t} else if (!strcmp(value, \"immediate\")) {\n> +\t\t} else if (!strcmp(value, \"immediate\") || !strcmp(value, \"1\")) {\n> \t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n> \t\t} else if (!strcmp(value, \"prompt\")) {\n> \t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n\n... but would it be simpler if we made it an extended boolean, i.e.\n\n true, yes, on, 1 -> same as \"immediate\"\n false, no, off, 0 -> same as \"never\"\n immediate -> same as what we currently do\n never -> same as what we currently do\n prompt -> same as what we currently do\n number -> same as what we currently do\n\nIt would kill many birds with a stone (e.g., help.autocorrect=no\ndoes not work in the current system as anybody would expect, but it\nwould with the \"this is an extended boolean\" approach).\n\nI dunno.\n\nThanks.\n\n"},{"id":"510215","messageId":"09e516e7-37a5-4489-a30b-f26dd2462fc3@revi.email","threadId":"62770","inReplyTo":"pull.1869.git.git.1736364707068.gitgitgadget@gmail.com","subject":"Re: [PATCH] help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s","fromName":"Yongmin","fromEmail":"yewon@revi.email","sentAt":"2025-01-09T07:05:29Z","receivedAt":"2025-01-09T07:05:33Z","isPatch":true,"sender":{"key":"yewon@revi.email","avatar":"https://gravatar.com/avatar/04f6a4ed8ee7252f2a7bc6ba6b752422c734a80a39520a69e88b44d8bcd42c67?d=mp&s=160"},"body":"On 2025-01-09 (Thu) 04:31:46+09:00, Scott Chacon via GitGitGadget \n wrote:\n> From: Scott Chacon \n>\n> [snip]\n>\n> This patch simply interprets a \"1\" value as the same as the \"immedate\"\n> autocorrect setting, which makes it skip the 0.1s and simply say that it's\n> running the command, which is almost certainly what everyone setting it to\n> that value is actually trying to do.\n\nI think Kristoffer somewhat mentioned this but…\n\ns/immedate/immediate/\n\n-- \n----\nrevi | 레비 (IPA: lɛbi)\n- 홍용민\n- https://revi.xyz\n- he/him \n- What time is it in my timezone? \n- OpenPGP \n- In this Korean name , the family name is Hong \n,\n which makes my name HONG Yongmin.\n- I reply when my time permits. Don't feel pressured to reply ASAP;\n take your time and respond at your schedule.\n"},{"id":"510242","messageId":"pull.1869.v2.git.git.1736419777235.gitgitgadget@gmail.com","threadId":"62770","inReplyTo":"pull.1869.git.git.1736364707068.gitgitgadget@gmail.com","subject":"[PATCH v2] help: interpret boolean string values for help.autocorrect","fromName":"Scott Chacon via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-09T10:49:36Z","receivedAt":"2025-01-09T10:49:40Z","isPatch":true,"sender":{"key":"schacon@gmail.com","avatar":"https://gravatar.com/avatar/9b13a8a078e1dcf8588c4eea9554445d51ebed6c41b51f56f4d96738130b05c6?d=mp&s=160"},"body":"From: Scott Chacon \n\nA help.autocorrect value of 1 is currently interpreted as \"wait 1\ndecisecond\", which can be confusing to users who believe they are setting a\nboolean value to turn the autocorrect feature on.\n\nInterpret the value of help.autocorrect as either one of the accepted list\nof special values (\"never\", \"immediate\", ...), a boolean or an integer. If\nthe value is 1, it is no longer interpreted as a decisecond value of 0.1s\nbut as a true boolean, the equivalent of \"immediate\". If the value is 2 or\nmore, continue treating it as a decisecond wait time.\n\nFalse boolean string values (\"off\", \"false\", \"no\") are now equivalent to 0,\nmeaning that guessed values are still shown but nothing is executed (as\nopposed to \"never\", which does not show the guesses). True boolean string\nvalues are interpreted as \"immediate\".\n\nSigned-off-by: Scott Chacon \n---\n help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s\n \n Took Junio's suggestion to include all boolean values as valid, though\n I'm not interpreting \"false\" as \"never\", but instead as 0, as they're\n subtly different. 0 will show the guessed commands and exit, \"never\"\n will not guess the commands.\n \n Changes since v1:\n \n * Include all boolean values rather than special casing \"1\"\n * Update the help.txt documentation\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1869%2Fschacon%2Fmaster-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1869/schacon/master-v2\nPull-Request: https://github.com/git/git/pull/1869\n\nRange-diff vs v1:\n\n 1: dbda79cd4fc < -: ----------- help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s\n -: ----------- > 1: 07b47b70ded help: interpret boolean string values for help.autocorrect\n\n\n Documentation/config/help.txt | 5 +++--\n help.c | 18 +++++++++++++++---\n 2 files changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/help.txt b/Documentation/config/help.txt\nindex 610701f9a37..6d9c2e06908 100644\n--- a/Documentation/config/help.txt\n+++ b/Documentation/config/help.txt\n@@ -11,8 +11,9 @@ help.autoCorrect::\n \tIf git detects typos and can identify exactly one valid command similar\n \tto the error, git will try to suggest the correct command or even\n \trun the suggestion automatically. Possible config values are:\n-\t - 0 (default): show the suggested command.\n-\t - positive number: run the suggested command after specified\n+\t - 0, false boolean string: show the suggested command (default).\n+\t - 1, true boolean string: run the suggested command immediately.\n+\t - positive number > 1: run the suggested command after specified\n deciseconds (0.1 sec).\n \t - \"immediate\": run the suggested command immediately.\n \t - \"prompt\": show the suggestion and prompt for confirmation to run\ndiff --git a/help.c b/help.c\nindex 5483ea8fd29..9e0f66c26dc 100644\n--- a/help.c\n+++ b/help.c\n@@ -573,9 +573,21 @@ static int git_unknown_cmd_config(const char *var, const char *value,\n \t\t} else if (!strcmp(value, \"prompt\")) {\n \t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n \t\t} else {\n-\t\t\tint v = git_config_int(var, value, ctx->kvi);\n-\t\t\tcfg->autocorrect = (v < 0)\n-\t\t\t\t? AUTOCORRECT_IMMEDIATELY : v;\n+\t\t\tint is_bool;\n+\t\t\tint v = git_config_bool_or_int(var, value, ctx->kvi, &is_bool);\n+\t\t\tif (is_bool) {\n+\t\t\t\tif (v == 0) {\n+\t\t\t\t\tcfg->autocorrect = 0;\n+\t\t\t\t} else {\n+\t\t\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\tif (v < 0 || v == 1) {\n+\t\t\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n+\t\t\t\t} else {\n+\t\t\t\t\tcfg->autocorrect = v;\n+\t\t\t\t}\n+\t\t\t}\n \t\t}\n \t}\n \t/* Also use aliases for command lookup */\n\nbase-commit: 14650065b76b28d3cfa9453356ac5669b19e706e\n-- \ngitgitgadget\n"},{"id":"510262","messageId":"xmqq5xmoj6cz.fsf@gitster.g","threadId":"62770","inReplyTo":"pull.1869.v2.git.git.1736419777235.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] help: interpret boolean string values for help.autocorrect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-09T16:32:12Z","receivedAt":"2025-01-09T16:32:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Scott Chacon via GitGitGadget\" writes:\n\n> diff --git a/Documentation/config/help.txt b/Documentation/config/help.txt\n> index 610701f9a37..6d9c2e06908 100644\n> --- a/Documentation/config/help.txt\n> +++ b/Documentation/config/help.txt\n> @@ -11,8 +11,9 @@ help.autoCorrect::\n> \tIf git detects typos and can identify exactly one valid command similar\n> \tto the error, git will try to suggest the correct command or even\n> \trun the suggestion automatically. Possible config values are:\n> -\t - 0 (default): show the suggested command.\n> -\t - positive number: run the suggested command after specified\n> +\t - 0, false boolean string: show the suggested command (default).\n> +\t - 1, true boolean string: run the suggested command immediately.\n> +\t - positive number > 1: run the suggested command after specified\n> deciseconds (0.1 sec).\n> \t - \"immediate\": run the suggested command immediately.\n> \t - \"prompt\": show the suggestion and prompt for confirmation to run\n\nNot a problem this patch introduces, but it looed funny to see the\nsecond line abut to the left edge of the page there.\n\nIn any case, the updated semantics look quite sensible.\n\n> diff --git a/help.c b/help.c\n> index 5483ea8fd29..9e0f66c26dc 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -573,9 +573,21 @@ static int git_unknown_cmd_config(const char *var, const char *value,\n> \t\t} else if (!strcmp(value, \"prompt\")) {\n> \t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n> \t\t} else {\n> -\t\t\tint v = git_config_int(var, value, ctx->kvi);\n> -\t\t\tcfg->autocorrect = (v < 0)\n> -\t\t\t\t? AUTOCORRECT_IMMEDIATELY : v;\n> +\t\t\tint is_bool;\n> +\t\t\tint v = git_config_bool_or_int(var, value, ctx->kvi, &is_bool);\n> +\t\t\tif (is_bool) {\n> +\t\t\t\tif (v == 0) {\n> +\t\t\t\t\tcfg->autocorrect = 0;\n> +\t\t\t\t} else {\n> +\t\t\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n> +\t\t\t\t}\n> +\t\t\t} else {\n> +\t\t\t\tif (v < 0 || v == 1) {\n> +\t\t\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n> +\t\t\t\t} else {\n> +\t\t\t\t\tcfg->autocorrect = v;\n> +\t\t\t\t}\n> +\t\t\t}\n> \t\t}\n> \t}\n\nThe flow looks nice, but the pre-context of this hunk starts like\nthis:\n\n\t\tif (!value)\n\t\t\treturn config_error_nonbool(var);\n\t\tif (!strcmp(value, \"never\")) {\n\t\t\tcfg->autocorrect = AUTOCORRECT_NEVER;\n\t\t} else if (!strcmp(value, \"immediate\")) {\n\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n\t\t} else if (!strcmp(value, \"prompt\")) {\n\nIOW, the new code added at the end of the if/else if/ cascade is way\ntoo late. \n\n\t\"[help] autocorrect\"\n\nthat specifies \"true\" has already been rejected as an error, with a\nnow-stale error message saying that the variable is not a Boolean.\n\nWe may probably want to use git_parse_maybe_bool_text() upfront,\nlike\n\n\tstatic int parse_autocorrect(const char *value)\n\t{\n\t\tswitch (git_parse_maybe_bool_text(value)) {\n\t case 1:\n\t\t\treturn AUTOCORRECT_IMMEDIATELY;\n\t\tcase 0:\n\t\t\treturn AUTOCORRECT_NEVER;\n\t\tdefault: /* other random text */\n\t\t\tbreak;\n\t\t}\n if (!strcmp(value, \"prompt\"))\n\t\t\treturn AUTOCORRECT_PROMPT;\n\t\t...\n\t\tif (!strcmp(value, \"prompt\"))\n\t\t\treturn AUTOCORRECT_PROMPT;\n\n return 0;\n\t}\n\nand then in git_unknown_cmd_config(), do something like\n\n\tif (!strcmp(var, \"help.autocorrect\")) {\n\t\tint v = parse_autocorrect(value);\n\n if (!v) {\n \tv = git_config_int(var, value, ctx->kvi);\n if (v < 0)\n\t\t\t\tv = AUTOCORRECT_IMMEDIATELY;\n }\n cfg->autocorrect = v;\n\t}\n\nperhaps?\n"},{"id":"510279","messageId":"CAP2yMaJzHM+AyDwjfVNJrDeJt0iip5h_ZxgNJL3V-gVSOjoBWQ@mail.gmail.com","threadId":"62770","inReplyTo":"xmqq5xmoj6cz.fsf@gitster.g","subject":"Re: [PATCH v2] help: interpret boolean string values for help.autocorrect","fromName":"Scott Chacon","fromEmail":"schacon@gmail.com","sentAt":"2025-01-10T07:43:17Z","receivedAt":"2025-01-10T07:43:30Z","isPatch":true,"sender":{"key":"schacon@gmail.com","avatar":"https://gravatar.com/avatar/9b13a8a078e1dcf8588c4eea9554445d51ebed6c41b51f56f4d96738130b05c6?d=mp&s=160"},"body":"Hey,\n\nOn Thu, Jan 9, 2025 at 5:32 PM Junio C Hamano wrote:\n> The flow looks nice, but the pre-context of this hunk starts like\n> this:\n>\n> if (!value)\n> return config_error_nonbool(var);\n> if (!strcmp(value, \"never\")) {\n> cfg->autocorrect = AUTOCORRECT_NEVER;\n> } else if (!strcmp(value, \"immediate\")) {\n> cfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n> } else if (!strcmp(value, \"prompt\")) {\n>\n> IOW, the new code added at the end of the if/else if/ cascade is way\n> too late.\n>\n> \"[help] autocorrect\"\n>\n> that specifies \"true\" has already been rejected as an error, with a\n> now-stale error message saying that the variable is not a Boolean.\n\nI'm not super familiar with this codebase, honestly, but ifaict this\nis not what this does. That top block makes sure that value isn't\nnull, which I can't figure out how it would ever be - I've tried a\nbunch of different config values, but I'm not sure it's possible to do\n- and if so it just prints \"missing value for help.autocorrect\" (the\nnonbool part of that function is something of a misnomer, it appears).\nBut again, I can't see how those two lines aren't essentially a no-op.\n\n> We may probably want to use git_parse_maybe_bool_text() upfront,\n> like\n>\n> static int parse_autocorrect(const char *value)\n> {\n> switch (git_parse_maybe_bool_text(value)) {\n> case 1:\n> return AUTOCORRECT_IMMEDIATELY;\n> case 0:\n> return AUTOCORRECT_NEVER;\n> default: /* other random text */\n> break;\n> }\n> if (!strcmp(value, \"prompt\"))\n> return AUTOCORRECT_PROMPT;\n> ...\n> if (!strcmp(value, \"prompt\"))\n> return AUTOCORRECT_PROMPT;\n>\n> return 0;\n> }\n>\n> and then in git_unknown_cmd_config(), do something like\n>\n> if (!strcmp(var, \"help.autocorrect\")) {\n> int v = parse_autocorrect(value);\n>\n> if (!v) {\n> v = git_config_int(var, value, ctx->kvi);\n> if (v < 0)\n> v = AUTOCORRECT_IMMEDIATELY;\n> }\n> cfg->autocorrect = v;\n> }\n\nI _can_ do this, but it seems somewhat more complicated and I believe\nit would have the same end result, no?\n\nAlso, in thinking about this a bit more, while I updated the patch\nwith the suggestion to make it accept all boolean text values rather\nthan the \"1\" hack, it should be kept in mind that if someone does do\nthis, that config setting will be backwards incompatible with previous\nGit versions in a way that will have a fatal error if it encounters a\nstring boolean value when a command is mistyped. Maybe that's not\nsuper horrible, but I'm honestly not sure that accepting more boolean\nstring values is helpful - it's been 17 years of this feature and I\ndoubt that many people have tried to set it to 'on' or probably would\nin the future.\n\nAnyhow, I'm happy to redo this patch in the manner suggested, but\npersonally I think the first simple DWIM hack is a realistically\nbetter solution.\n\nScott\n"},{"id":"510282","messageId":"CAP2yMa+5ca22tNMc4qu=yBVd9t74uNnLFbKE3_=EcA5_goM6zw@mail.gmail.com","threadId":"62770","inReplyTo":"CAP2yMaJzHM+AyDwjfVNJrDeJt0iip5h_ZxgNJL3V-gVSOjoBWQ@mail.gmail.com","subject":"Re: [PATCH v2] help: interpret boolean string values for help.autocorrect","fromName":"Scott Chacon","fromEmail":"schacon@gmail.com","sentAt":"2025-01-10T09:30:12Z","receivedAt":"2025-01-10T09:30:25Z","isPatch":true,"sender":{"key":"schacon@gmail.com","avatar":"https://gravatar.com/avatar/9b13a8a078e1dcf8588c4eea9554445d51ebed6c41b51f56f4d96738130b05c6?d=mp&s=160"},"body":"On Fri, Jan 10, 2025 at 8:43 AM Scott Chacon wrote:\n>\n> Hey,\n>\n> On Thu, Jan 9, 2025 at 5:32 PM Junio C Hamano wrote:\n> > The flow looks nice, but the pre-context of this hunk starts like\n> > this:\n> >\n> > if (!value)\n> > return config_error_nonbool(var);\n> > if (!strcmp(value, \"never\")) {\n> > cfg->autocorrect = AUTOCORRECT_NEVER;\n> > } else if (!strcmp(value, \"immediate\")) {\n> > cfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n> > } else if (!strcmp(value, \"prompt\")) {\n> >\n> > IOW, the new code added at the end of the if/else if/ cascade is way\n> > too late.\n> >\n> > \"[help] autocorrect\"\n> >\n> > that specifies \"true\" has already been rejected as an error, with a\n> > now-stale error message saying that the variable is not a Boolean.\n>\n> I'm not super familiar with this codebase, honestly, but ifaict this\n> is not what this does. That top block makes sure that value isn't\n> null, which I can't figure out how it would ever be - I've tried a\n> bunch of different config values, but I'm not sure it's possible to do\n> - and if so it just prints \"missing value for help.autocorrect\" (the\n> nonbool part of that function is something of a misnomer, it appears).\n> But again, I can't see how those two lines aren't essentially a no-op.\n\nAh, I see. You can leave off the `=` and that will trigger this error.\nThough it seems to simultaneously be seen as a configuration error.\n\n ❯ ./git test\n error: missing value for 'help.autocorrect'\n fatal: bad config line 19 in file .git/config\n\nBut if that's the only way it seems to trigger this code path, to\nessentially have a corrupted config file, does it matter?\n\nScott\n"},{"id":"510312","messageId":"20250110121100.GE1014503@coredump.intra.peff.net","threadId":"62770","inReplyTo":"CAP2yMa+5ca22tNMc4qu=yBVd9t74uNnLFbKE3_=EcA5_goM6zw@mail.gmail.com","subject":"Re: [PATCH v2] help: interpret boolean string values for help.autocorrect","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-10T12:11:00Z","receivedAt":"2025-01-10T12:11:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 10, 2025 at 10:30:12AM +0100, Scott Chacon wrote:\n\n> > On Thu, Jan 9, 2025 at 5:32 PM Junio C Hamano wrote:\n> > > The flow looks nice, but the pre-context of this hunk starts like\n> > > this:\n> > >\n> > > if (!value)\n> > > return config_error_nonbool(var);\n> > > if (!strcmp(value, \"never\")) {\n> > > cfg->autocorrect = AUTOCORRECT_NEVER;\n> > > } else if (!strcmp(value, \"immediate\")) {\n> > > cfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n> > > } else if (!strcmp(value, \"prompt\")) {\n> > >\n> > > IOW, the new code added at the end of the if/else if/ cascade is way\n> > > too late.\n> > >\n> > > \"[help] autocorrect\"\n> > >\n> > > that specifies \"true\" has already been rejected as an error, with a\n> > > now-stale error message saying that the variable is not a Boolean.\n> >\n> > I'm not super familiar with this codebase, honestly, but ifaict this\n> > is not what this does. That top block makes sure that value isn't\n> > null, which I can't figure out how it would ever be - I've tried a\n> > bunch of different config values, but I'm not sure it's possible to do\n> > - and if so it just prints \"missing value for help.autocorrect\" (the\n> > nonbool part of that function is something of a misnomer, it appears).\n> > But again, I can't see how those two lines aren't essentially a no-op.\n> \n> Ah, I see. You can leave off the `=` and that will trigger this error.\n> Though it seems to simultaneously be seen as a configuration error.\n> \n> ❯ ./git test\n> error: missing value for 'help.autocorrect'\n> fatal: bad config line 19 in file .git/config\n> \n> But if that's the only way it seems to trigger this code path, to\n> essentially have a corrupted config file, does it matter?\n\nIt's not corrupted; that syntax is allowed for boolean variables[1]. The\n\"bad config line\" is due to the early \"return config_error_nonbool(var)\"\nquoted above. It is passing the error back to the general config code,\nwhich then just prints the \"bad config\" line.\n\nI think what Junio is saying is that if we are going to turn this into\nan option which accepts bool values, it should accept this special\nsyntax, too. And that first \"if (!value)\" has to either go away (and get\nreplace by a maybe_bool() call, as mentioned earlier) or has to set\nAUTOCORRECT_IMMEDIATELY itself.\n\n-Peff\n\n[1] There's a similar syntax for the \"-c\" option, which can make testing\n easier:\n\n git -c help.autocorrect foo\n"},{"id":"510327","messageId":"xmqq1pxaaf0n.fsf@gitster.g","threadId":"62770","inReplyTo":"20250110121100.GE1014503@coredump.intra.peff.net","subject":"Re: [PATCH v2] help: interpret boolean string values for help.autocorrect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-10T15:02:16Z","receivedAt":"2025-01-10T15:02:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> It's not corrupted; that syntax is allowed for boolean variables[1]. The\n> \"bad config line\" is due to the early \"return config_error_nonbool(var)\"\n> quoted above. It is passing the error back to the general config code,\n> which then just prints the \"bad config\" line.\n>\n> I think what Junio is saying is that if we are going to turn this into\n> an option which accepts bool values, it should accept this special\n> syntax, too. And that first \"if (!value)\" has to either go away (and get\n> replace by a maybe_bool() call, as mentioned earlier) or has to set\n> AUTOCORRECT_IMMEDIATELY itself.\n\nExactly.\n\nThanks for filling the blank in for me while I was away from the\nkeyboard ;-)\n"},{"id":"510369","messageId":"pull.1869.v3.git.git.1736594839527.gitgitgadget@gmail.com","threadId":"62770","inReplyTo":"pull.1869.v2.git.git.1736419777235.gitgitgadget@gmail.com","subject":"[PATCH v3] help: interpret boolean string values for help.autocorrect","fromName":"Scott Chacon via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-11T11:27:19Z","receivedAt":"2025-01-11T11:27:23Z","isPatch":true,"sender":{"key":"schacon@gmail.com","avatar":"https://gravatar.com/avatar/9b13a8a078e1dcf8588c4eea9554445d51ebed6c41b51f56f4d96738130b05c6?d=mp&s=160"},"body":"From: Scott Chacon \n\nA help.autocorrect value of 1 is currently interpreted as \"wait 1\ndecisecond\", which can be confusing to users who believe they are setting a\nboolean value to turn the autocorrect feature on.\n\nInterpret the value of help.autocorrect as either one of the accepted list\nof special values (\"never\", \"immediate\", ...), a boolean or an integer. If\nthe value is 1, it is no longer interpreted as a decisecond value of 0.1s\nbut as a true boolean, the equivalent of \"immediate\". If the value is 2 or\nmore, continue treating it as a decisecond wait time.\n\nFalse boolean string values (\"off\", \"false\", \"no\") are now equivalent to\n\"never\", meaning that guessed values are still shown but nothing is\nexecuted. True boolean string values are interpreted as \"immediate\".\n\nSigned-off-by: Scott Chacon \n---\n help: interpret boolean string values for help.autocorrect\n \n Basically just using Junio's suggested code from the ML, splitting most\n of the logic out into a parse_autocorrect method and then special casing\n a 1 integer as \"immediate\". I reverted to interpreting false boolean\n values as NEVER rather than 0, which means they no longer show guesses,\n which the last patch did.\n \n Changes since v2:\n \n * split out most logic into parse_autocorrect\n * interpret false boolean values as NEVER rather than 0\n * Update the help.txt documentation\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1869%2Fschacon%2Fmaster-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1869/schacon/master-v3\nPull-Request: https://github.com/git/git/pull/1869\n\nRange-diff vs v2:\n\n 1: 07b47b70ded ! 1: 4ce7652d19e help: interpret boolean string values for help.autocorrect\n @@ Commit message\n but as a true boolean, the equivalent of \"immediate\". If the value is 2 or\n more, continue treating it as a decisecond wait time.\n \n - False boolean string values (\"off\", \"false\", \"no\") are now equivalent to 0,\n - meaning that guessed values are still shown but nothing is executed (as\n - opposed to \"never\", which does not show the guesses). True boolean string\n - values are interpreted as \"immediate\".\n + False boolean string values (\"off\", \"false\", \"no\") are now equivalent to\n + \"never\", meaning that guessed values are still shown but nothing is\n + executed. True boolean string values are interpreted as \"immediate\".\n \n Signed-off-by: Scott Chacon \n \n @@ Documentation/config/help.txt: help.autoCorrect::\n \trun the suggestion automatically. Possible config values are:\n -\t - 0 (default): show the suggested command.\n -\t - positive number: run the suggested command after specified\n -+\t - 0, false boolean string: show the suggested command (default).\n -+\t - 1, true boolean string: run the suggested command immediately.\n ++\t - 0: show the suggested command (default).\n ++\t - 1, \"true\", \"on\", \"yes\": run the suggested command immediately.\n +\t - positive number > 1: run the suggested command after specified\n deciseconds (0.1 sec).\n \t - \"immediate\": run the suggested command immediately.\n \t - \"prompt\": show the suggestion and prompt for confirmation to run\n + the command.\n +-\t - \"never\": don't run or show any suggested command.\n ++\t - \"false\", \"off\", \"no\", \"never\": don't run or show any suggested command.\n + \n + help.htmlPath::\n + \tSpecify the path where the HTML documentation resides. File system paths\n \n ## help.c ##\n +@@ help.c: struct help_unknown_cmd_config {\n + #define AUTOCORRECT_NEVER (-2)\n + #define AUTOCORRECT_IMMEDIATELY (-1)\n + \n ++static int parse_autocorrect(const char *value)\n ++{\n ++\tswitch (git_parse_maybe_bool_text(value)) {\n ++\t\tcase 1:\n ++\t\t\treturn AUTOCORRECT_IMMEDIATELY;\n ++\t\tcase 0:\n ++\t\t\treturn AUTOCORRECT_NEVER;\n ++\t\tdefault: /* other random text */\n ++\t\t\tbreak;\n ++\t}\n ++\n ++\tif (!strcmp(value, \"prompt\"))\n ++\t\treturn AUTOCORRECT_PROMPT;\n ++\tif (!strcmp(value, \"never\"))\n ++\t\treturn AUTOCORRECT_NEVER;\n ++\tif (!strcmp(value, \"immediate\"))\n ++\t\treturn AUTOCORRECT_IMMEDIATELY;\n ++\n ++\treturn 0;\n ++}\n ++\n + static int git_unknown_cmd_config(const char *var, const char *value,\n + \t\t\t\t const struct config_context *ctx,\n + \t\t\t\t void *cb)\n @@ help.c: static int git_unknown_cmd_config(const char *var, const char *value,\n - \t\t} else if (!strcmp(value, \"prompt\")) {\n - \t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n - \t\t} else {\n + \tconst char *p;\n + \n + \tif (!strcmp(var, \"help.autocorrect\")) {\n +-\t\tif (!value)\n +-\t\t\treturn config_error_nonbool(var);\n +-\t\tif (!strcmp(value, \"never\")) {\n +-\t\t\tcfg->autocorrect = AUTOCORRECT_NEVER;\n +-\t\t} else if (!strcmp(value, \"immediate\")) {\n +-\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n +-\t\t} else if (!strcmp(value, \"prompt\")) {\n +-\t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n +-\t\t} else {\n -\t\t\tint v = git_config_int(var, value, ctx->kvi);\n -\t\t\tcfg->autocorrect = (v < 0)\n -\t\t\t\t? AUTOCORRECT_IMMEDIATELY : v;\n -+\t\t\tint is_bool;\n -+\t\t\tint v = git_config_bool_or_int(var, value, ctx->kvi, &is_bool);\n -+\t\t\tif (is_bool) {\n -+\t\t\t\tif (v == 0) {\n -+\t\t\t\t\tcfg->autocorrect = 0;\n -+\t\t\t\t} else {\n -+\t\t\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n -+\t\t\t\t}\n -+\t\t\t} else {\n -+\t\t\t\tif (v < 0 || v == 1) {\n -+\t\t\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n -+\t\t\t\t} else {\n -+\t\t\t\t\tcfg->autocorrect = v;\n -+\t\t\t\t}\n -+\t\t\t}\n ++\t\tint v = parse_autocorrect(value);\n ++\n ++\t\tif (!v) {\n ++\t\t\tv = git_config_int(var, value, ctx->kvi);\n ++\t\t\tif (v < 0 || v == 1)\n ++\t\t\t\tv = AUTOCORRECT_IMMEDIATELY;\n \t\t}\n ++\n ++\t\tcfg->autocorrect = v;\n \t}\n ++\n \t/* Also use aliases for command lookup */\n + \tif (skip_prefix(var, \"alias.\", &p))\n + \t\tadd_cmdname(&cfg->aliases, p, strlen(p));\n\n\n Documentation/config/help.txt | 7 +++---\n help.c | 42 +++++++++++++++++++++++++----------\n 2 files changed, 34 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/config/help.txt b/Documentation/config/help.txt\nindex 610701f9a37..16b124b1c17 100644\n--- a/Documentation/config/help.txt\n+++ b/Documentation/config/help.txt\n@@ -11,13 +11,14 @@ help.autoCorrect::\n \tIf git detects typos and can identify exactly one valid command similar\n \tto the error, git will try to suggest the correct command or even\n \trun the suggestion automatically. Possible config values are:\n-\t - 0 (default): show the suggested command.\n-\t - positive number: run the suggested command after specified\n+\t - 0: show the suggested command (default).\n+\t - 1, \"true\", \"on\", \"yes\": run the suggested command immediately.\n+\t - positive number > 1: run the suggested command after specified\n deciseconds (0.1 sec).\n \t - \"immediate\": run the suggested command immediately.\n \t - \"prompt\": show the suggestion and prompt for confirmation to run\n the command.\n-\t - \"never\": don't run or show any suggested command.\n+\t - \"false\", \"off\", \"no\", \"never\": don't run or show any suggested command.\n \n help.htmlPath::\n \tSpecify the path where the HTML documentation resides. File system paths\ndiff --git a/help.c b/help.c\nindex 5483ea8fd29..7148963e468 100644\n--- a/help.c\n+++ b/help.c\n@@ -556,6 +556,27 @@ struct help_unknown_cmd_config {\n #define AUTOCORRECT_NEVER (-2)\n #define AUTOCORRECT_IMMEDIATELY (-1)\n \n+static int parse_autocorrect(const char *value)\n+{\n+\tswitch (git_parse_maybe_bool_text(value)) {\n+\t\tcase 1:\n+\t\t\treturn AUTOCORRECT_IMMEDIATELY;\n+\t\tcase 0:\n+\t\t\treturn AUTOCORRECT_NEVER;\n+\t\tdefault: /* other random text */\n+\t\t\tbreak;\n+\t}\n+\n+\tif (!strcmp(value, \"prompt\"))\n+\t\treturn AUTOCORRECT_PROMPT;\n+\tif (!strcmp(value, \"never\"))\n+\t\treturn AUTOCORRECT_NEVER;\n+\tif (!strcmp(value, \"immediate\"))\n+\t\treturn AUTOCORRECT_IMMEDIATELY;\n+\n+\treturn 0;\n+}\n+\n static int git_unknown_cmd_config(const char *var, const char *value,\n \t\t\t\t const struct config_context *ctx,\n \t\t\t\t void *cb)\n@@ -564,20 +585,17 @@ static int git_unknown_cmd_config(const char *var, const char *value,\n \tconst char *p;\n \n \tif (!strcmp(var, \"help.autocorrect\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tif (!strcmp(value, \"never\")) {\n-\t\t\tcfg->autocorrect = AUTOCORRECT_NEVER;\n-\t\t} else if (!strcmp(value, \"immediate\")) {\n-\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n-\t\t} else if (!strcmp(value, \"prompt\")) {\n-\t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n-\t\t} else {\n-\t\t\tint v = git_config_int(var, value, ctx->kvi);\n-\t\t\tcfg->autocorrect = (v < 0)\n-\t\t\t\t? AUTOCORRECT_IMMEDIATELY : v;\n+\t\tint v = parse_autocorrect(value);\n+\n+\t\tif (!v) {\n+\t\t\tv = git_config_int(var, value, ctx->kvi);\n+\t\t\tif (v < 0 || v == 1)\n+\t\t\t\tv = AUTOCORRECT_IMMEDIATELY;\n \t\t}\n+\n+\t\tcfg->autocorrect = v;\n \t}\n+\n \t/* Also use aliases for command lookup */\n \tif (skip_prefix(var, \"alias.\", &p))\n \t\tadd_cmdname(&cfg->aliases, p, strlen(p));\n\nbase-commit: fbe8d3079d4a96aeb4e4529cc93cc0043b759a05\n-- \ngitgitgadget\n"},{"id":"510386","messageId":"20250113054355.GB767856@coredump.intra.peff.net","threadId":"62770","inReplyTo":"pull.1869.v3.git.git.1736594839527.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] help: interpret boolean string values for help.autocorrect","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-13T05:43:55Z","receivedAt":"2025-01-13T05:43:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 11, 2025 at 11:27:19AM +0000, Scott Chacon via GitGitGadget wrote:\n\n> Interpret the value of help.autocorrect as either one of the accepted list\n> of special values (\"never\", \"immediate\", ...), a boolean or an integer. If\n> the value is 1, it is no longer interpreted as a decisecond value of 0.1s\n> but as a true boolean, the equivalent of \"immediate\". If the value is 2 or\n> more, continue treating it as a decisecond wait time.\n\nThis mostly looks good to me, though this part gave me a little pause:\n\n> False boolean string values (\"off\", \"false\", \"no\") are now equivalent to\n> \"never\", meaning that guessed values are still shown but nothing is\n> executed. True boolean string values are interpreted as \"immediate\".\n\nI think false boolean values end up as \"never\", which shows _nothing_.\nAs opposed to \"0\", which continues to be \"show but do not execute\" (and\nwhich we can't change if we want to retain historical compatibility).\n\nThat's probably OK, though it is a little unlike other bools in that \"0\"\nis usually a strict synonym for \"false\". So we could go the other way,\nwith \"0, false, off, no\" meaning \"show but don't run\" and leaving\n\"never\" by itself to mean \"do nothing\".\n\n> diff --git a/Documentation/config/help.txt b/Documentation/config/help.txt\n> index 610701f9a37..16b124b1c17 100644\n> --- a/Documentation/config/help.txt\n> +++ b/Documentation/config/help.txt\n> @@ -11,13 +11,14 @@ help.autoCorrect::\n> \tIf git detects typos and can identify exactly one valid command similar\n> \tto the error, git will try to suggest the correct command or even\n> \trun the suggestion automatically. Possible config values are:\n> -\t - 0 (default): show the suggested command.\n> -\t - positive number: run the suggested command after specified\n> +\t - 0: show the suggested command (default).\n> +\t - 1, \"true\", \"on\", \"yes\": run the suggested command immediately.\n> +\t - positive number > 1: run the suggested command after specified\n> deciseconds (0.1 sec).\n> \t - \"immediate\": run the suggested command immediately.\n> \t - \"prompt\": show the suggestion and prompt for confirmation to run\n> the command.\n> -\t - \"never\": don't run or show any suggested command.\n> +\t - \"false\", \"off\", \"no\", \"never\": don't run or show any suggested command.\n\n\"never\" gets folded into the list of other false booleans. But\n\"immediate\" still gets its own bullet point. Should it be folded into\nthe \"true\" line?\n\n> diff --git a/help.c b/help.c\n> index 5483ea8fd29..7148963e468 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -556,6 +556,27 @@ struct help_unknown_cmd_config {\n> #define AUTOCORRECT_NEVER (-2)\n> #define AUTOCORRECT_IMMEDIATELY (-1)\n> \n> +static int parse_autocorrect(const char *value)\n> +{\n> +\tswitch (git_parse_maybe_bool_text(value)) {\n> +\t\tcase 1:\n> +\t\t\treturn AUTOCORRECT_IMMEDIATELY;\n> +\t\tcase 0:\n> +\t\t\treturn AUTOCORRECT_NEVER;\n> +\t\tdefault: /* other random text */\n> +\t\t\tbreak;\n> +\t}\n\nOne of the reasons I looked so closely at the \"0\" behavior above is that\nI thought the maybe_bool() parser might eat your \"0\" before you get a\nchance to act on it. But because you use maybe_bool_text(), it doesn't\ndo any integer interpretation at all. Good.\n\n> +\tif (!strcmp(value, \"prompt\"))\n> +\t\treturn AUTOCORRECT_PROMPT;\n> +\tif (!strcmp(value, \"never\"))\n> +\t\treturn AUTOCORRECT_NEVER;\n> +\tif (!strcmp(value, \"immediate\"))\n> +\t\treturn AUTOCORRECT_IMMEDIATELY;\n> +\n> +\treturn 0;\n> +}\n\nAnd these all make sense. I wondered if we might ever mistake this 0\nreturn for AUTOCORRECT_*, but they are all defined with non-zero values\n(which makes sense, since we store them in the same variable that might\nhold a \"0\" or positive value).\n\nAnd in fact it would make my bool alternative suggestion above trickier,\nsince this function could return a true \"0\" to mean \"show but don't\nrun\".\n\n> @@ -564,20 +585,17 @@ static int git_unknown_cmd_config(const char *var, const char *value,\n> \tconst char *p;\n> \n> \tif (!strcmp(var, \"help.autocorrect\")) {\n> -\t\tif (!value)\n> -\t\t\treturn config_error_nonbool(var);\n> -\t\tif (!strcmp(value, \"never\")) {\n> -\t\t\tcfg->autocorrect = AUTOCORRECT_NEVER;\n> -\t\t} else if (!strcmp(value, \"immediate\")) {\n> -\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n> -\t\t} else if (!strcmp(value, \"prompt\")) {\n> -\t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n> -\t\t} else {\n> -\t\t\tint v = git_config_int(var, value, ctx->kvi);\n> -\t\t\tcfg->autocorrect = (v < 0)\n> -\t\t\t\t? AUTOCORRECT_IMMEDIATELY : v;\n> +\t\tint v = parse_autocorrect(value);\n> +\n> +\t\tif (!v) {\n> +\t\t\tv = git_config_int(var, value, ctx->kvi);\n> +\t\t\tif (v < 0 || v == 1)\n> +\t\t\t\tv = AUTOCORRECT_IMMEDIATELY;\n> \t\t}\n> +\n> +\t\tcfg->autocorrect = v;\n> \t}\n> +\n\nOK, so parse_autocorrect() handles all of the non-numeric values. And\nthen we fall back on the integer values. Makes sense.\n\nSo assuming we are OK with the \"0\" vs \"false\" split, the whole patch\nlooks good to me, modulo the nit about folding the \"immediate\" line in\nthe documentation.\n\n-Peff\n"},{"id":"510407","messageId":"CAP2yMa+Upc0i688H5+no3sYbz5X-Ma=KBuaVALpPBdNDHuG8QA@mail.gmail.com","threadId":"62770","inReplyTo":"20250113054355.GB767856@coredump.intra.peff.net","subject":"Re: [PATCH v3] help: interpret boolean string values for help.autocorrect","fromName":"Scott Chacon","fromEmail":"schacon@gmail.com","sentAt":"2025-01-13T09:31:02Z","receivedAt":"2025-01-13T09:31:15Z","isPatch":true,"sender":{"key":"schacon@gmail.com","avatar":"https://gravatar.com/avatar/9b13a8a078e1dcf8588c4eea9554445d51ebed6c41b51f56f4d96738130b05c6?d=mp&s=160"},"body":"On Mon, Jan 13, 2025 at 6:43 AM Jeff King wrote:\n>\n> On Sat, Jan 11, 2025 at 11:27:19AM +0000, Scott Chacon via GitGitGadget wrote:\n>\n> > Interpret the value of help.autocorrect as either one of the accepted list\n> > of special values (\"never\", \"immediate\", ...), a boolean or an integer. If\n> > the value is 1, it is no longer interpreted as a decisecond value of 0.1s\n> > but as a true boolean, the equivalent of \"immediate\". If the value is 2 or\n> > more, continue treating it as a decisecond wait time.\n>\n> This mostly looks good to me, though this part gave me a little pause:\n>\n> > False boolean string values (\"off\", \"false\", \"no\") are now equivalent to\n> > \"never\", meaning that guessed values are still shown but nothing is\n> > executed. True boolean string values are interpreted as \"immediate\".\n>\n> I think false boolean values end up as \"never\", which shows _nothing_.\n> As opposed to \"0\", which continues to be \"show but do not execute\" (and\n> which we can't change if we want to retain historical compatibility).\n>\n> That's probably OK, though it is a little unlike other bools in that \"0\"\n> is usually a strict synonym for \"false\". So we could go the other way,\n> with \"0, false, off, no\" meaning \"show but don't run\" and leaving\n> \"never\" by itself to mean \"do nothing\".\n\nYeah, the first patch I sent that interpreted booleans actually did do\n0 rather than \"never\", but Junio continued to suggest that this\nreturns \"never\" so I did it this way in this latest patch.\n\nI looked for a hot second into how to do this, but the problem is that\n`parse_autocorrect` can't return 0 because that's the failure mode, so\nthen we would need another constant or some other way to return\nsomething that means \"don't run, but also show the matches\", which\nright now is only this special value 0.\n\nHonestly, I think \"never\" is a fine option if someone is actually\nexplicitly setting this to a false string, even if it's _slightly_\ninconsistent with the 0 value.\n\n> OK, so parse_autocorrect() handles all of the non-numeric values. And\n> then we fall back on the integer values. Makes sense.\n>\n> So assuming we are OK with the \"0\" vs \"false\" split, the whole patch\n> looks good to me, modulo the nit about folding the \"immediate\" line in\n> the documentation.\n\nOK, sending v4 now with just the docs change.\n\nThanks,\nScott\n"},{"id":"510408","messageId":"pull.1869.v4.git.git.1736760824201.gitgitgadget@gmail.com","threadId":"62770","inReplyTo":"pull.1869.v3.git.git.1736594839527.gitgitgadget@gmail.com","subject":"[PATCH v4] help: interpret boolean string values for help.autocorrect","fromName":"Scott Chacon via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-13T09:33:44Z","receivedAt":"2025-01-13T09:33:47Z","isPatch":true,"sender":{"key":"schacon@gmail.com","avatar":"https://gravatar.com/avatar/9b13a8a078e1dcf8588c4eea9554445d51ebed6c41b51f56f4d96738130b05c6?d=mp&s=160"},"body":"From: Scott Chacon \n\nA help.autocorrect value of 1 is currently interpreted as \"wait 1\ndecisecond\", which can be confusing to users who believe they are setting a\nboolean value to turn the autocorrect feature on.\n\nInterpret the value of help.autocorrect as either one of the accepted list\nof special values (\"never\", \"immediate\", ...), a boolean or an integer. If\nthe value is 1, it is no longer interpreted as a decisecond value of 0.1s\nbut as a true boolean, the equivalent of \"immediate\". If the value is 2 or\nmore, continue treating it as a decisecond wait time.\n\nFalse boolean string values (\"off\", \"false\", \"no\") are now equivalent to\n\"never\", meaning that guessed values are still shown but nothing is\nexecuted. True boolean string values are interpreted as \"immediate\".\n\nSigned-off-by: Scott Chacon \n---\n help: interpret boolean string values for help.autocorrect\n \n Just updating the docs with Peff's suggestion.\n \n Changes since v3:\n \n * docs update to group \"immediate\" in with the true bools\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1869%2Fschacon%2Fmaster-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1869/schacon/master-v4\nPull-Request: https://github.com/git/git/pull/1869\n\nRange-diff vs v3:\n\n 1: 4ce7652d19e ! 1: 64482b5249b help: interpret boolean string values for help.autocorrect\n @@ Documentation/config/help.txt: help.autoCorrect::\n -\t - 0 (default): show the suggested command.\n -\t - positive number: run the suggested command after specified\n +\t - 0: show the suggested command (default).\n -+\t - 1, \"true\", \"on\", \"yes\": run the suggested command immediately.\n ++\t - 1, \"true\", \"on\", \"yes\", \"immediate\": run the suggested command\n ++immediately.\n +\t - positive number > 1: run the suggested command after specified\n deciseconds (0.1 sec).\n - \t - \"immediate\": run the suggested command immediately.\n +-\t - \"immediate\": run the suggested command immediately.\n ++\t - \"false\", \"off\", \"no\", \"never\": don't run or show any suggested command.\n \t - \"prompt\": show the suggestion and prompt for confirmation to run\n the command.\n -\t - \"never\": don't run or show any suggested command.\n -+\t - \"false\", \"off\", \"no\", \"never\": don't run or show any suggested command.\n \n help.htmlPath::\n \tSpecify the path where the HTML documentation resides. File system paths\n\n\n Documentation/config/help.txt | 9 ++++----\n help.c | 42 +++++++++++++++++++++++++----------\n 2 files changed, 35 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/config/help.txt b/Documentation/config/help.txt\nindex 610701f9a37..a4c6079af81 100644\n--- a/Documentation/config/help.txt\n+++ b/Documentation/config/help.txt\n@@ -11,13 +11,14 @@ help.autoCorrect::\n \tIf git detects typos and can identify exactly one valid command similar\n \tto the error, git will try to suggest the correct command or even\n \trun the suggestion automatically. Possible config values are:\n-\t - 0 (default): show the suggested command.\n-\t - positive number: run the suggested command after specified\n+\t - 0: show the suggested command (default).\n+\t - 1, \"true\", \"on\", \"yes\", \"immediate\": run the suggested command\n+immediately.\n+\t - positive number > 1: run the suggested command after specified\n deciseconds (0.1 sec).\n-\t - \"immediate\": run the suggested command immediately.\n+\t - \"false\", \"off\", \"no\", \"never\": don't run or show any suggested command.\n \t - \"prompt\": show the suggestion and prompt for confirmation to run\n the command.\n-\t - \"never\": don't run or show any suggested command.\n \n help.htmlPath::\n \tSpecify the path where the HTML documentation resides. File system paths\ndiff --git a/help.c b/help.c\nindex 5483ea8fd29..7148963e468 100644\n--- a/help.c\n+++ b/help.c\n@@ -556,6 +556,27 @@ struct help_unknown_cmd_config {\n #define AUTOCORRECT_NEVER (-2)\n #define AUTOCORRECT_IMMEDIATELY (-1)\n \n+static int parse_autocorrect(const char *value)\n+{\n+\tswitch (git_parse_maybe_bool_text(value)) {\n+\t\tcase 1:\n+\t\t\treturn AUTOCORRECT_IMMEDIATELY;\n+\t\tcase 0:\n+\t\t\treturn AUTOCORRECT_NEVER;\n+\t\tdefault: /* other random text */\n+\t\t\tbreak;\n+\t}\n+\n+\tif (!strcmp(value, \"prompt\"))\n+\t\treturn AUTOCORRECT_PROMPT;\n+\tif (!strcmp(value, \"never\"))\n+\t\treturn AUTOCORRECT_NEVER;\n+\tif (!strcmp(value, \"immediate\"))\n+\t\treturn AUTOCORRECT_IMMEDIATELY;\n+\n+\treturn 0;\n+}\n+\n static int git_unknown_cmd_config(const char *var, const char *value,\n \t\t\t\t const struct config_context *ctx,\n \t\t\t\t void *cb)\n@@ -564,20 +585,17 @@ static int git_unknown_cmd_config(const char *var, const char *value,\n \tconst char *p;\n \n \tif (!strcmp(var, \"help.autocorrect\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tif (!strcmp(value, \"never\")) {\n-\t\t\tcfg->autocorrect = AUTOCORRECT_NEVER;\n-\t\t} else if (!strcmp(value, \"immediate\")) {\n-\t\t\tcfg->autocorrect = AUTOCORRECT_IMMEDIATELY;\n-\t\t} else if (!strcmp(value, \"prompt\")) {\n-\t\t\tcfg->autocorrect = AUTOCORRECT_PROMPT;\n-\t\t} else {\n-\t\t\tint v = git_config_int(var, value, ctx->kvi);\n-\t\t\tcfg->autocorrect = (v < 0)\n-\t\t\t\t? AUTOCORRECT_IMMEDIATELY : v;\n+\t\tint v = parse_autocorrect(value);\n+\n+\t\tif (!v) {\n+\t\t\tv = git_config_int(var, value, ctx->kvi);\n+\t\t\tif (v < 0 || v == 1)\n+\t\t\t\tv = AUTOCORRECT_IMMEDIATELY;\n \t\t}\n+\n+\t\tcfg->autocorrect = v;\n \t}\n+\n \t/* Also use aliases for command lookup */\n \tif (skip_prefix(var, \"alias.\", &p))\n \t\tadd_cmdname(&cfg->aliases, p, strlen(p));\n\nbase-commit: fbe8d3079d4a96aeb4e4529cc93cc0043b759a05\n-- \ngitgitgadget\n"},{"id":"510426","messageId":"xmqqikqibsbp.fsf@gitster.g","threadId":"62770","inReplyTo":"20250113054355.GB767856@coredump.intra.peff.net","subject":"Re: [PATCH v3] help: interpret boolean string values for help.autocorrect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-13T16:18:34Z","receivedAt":"2025-01-13T16:18:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> That's probably OK, though it is a little unlike other bools in that \"0\"\n> is usually a strict synonym for \"false\". So we could go the other way,\n> with \"0, false, off, no\" meaning \"show but don't run\" and leaving\n> \"never\" by itself to mean \"do nothing\".\n\nThat's my fault. Your version makes perfect sense.\n\nThanks for being extra careful (well, more careful than myself, that\nis).\n"},{"id":"510463","messageId":"Z4Wi3K8Hd41VWWmG@nand.local","threadId":"62770","inReplyTo":"7553dc2b-7b91-31c9-e86e-aa24d3bb4235@gmx.de","subject":"Re: [PATCH] help: interpret help.autocorrect=1 as \"immediate\" rather than 0.1s","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-01-13T23:33:48Z","receivedAt":"2025-01-13T23:33:51Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Jan 09, 2025 at 01:18:15AM +0100, Johannes Schindelin wrote:\n> For the record, I do think it was a mistake to treat number values as\n> \"deciseconds\" here, it is inconsistent with pretty much any other config\n> setting. But I also don't see any way to remediate this design mistake at\n> this stage.\n\nI almost made this same mistake when working on pseudo-merge bitmaps, in\nparticular with the non-integral configuration options like:\n\n - bitampPseudoMerge..decay\n - bitampPseudoMerge..sampleRate\n\nIf memory serves, I think this mostly had to do with the lack of a\ndouble parser in the config system. I ended up adding one in 5831f8ac41\n(config: introduce `git_config_double()`, 2024-05-23), and made those\nconfiguration options take values like '0.1', etc.\n\nI think it may be worth considering what \"starting from scratch\" would\nlook like, as Junio suggested above. To be clear, I think that that\nshould happen outside of the current patch and not hold it up, as what\nScott is proposing is a strict improvement.\n\nBut it may be worth thinking about what a different interface might look\nlike. If we settle on something we like, perhaps we could start nudging\nusers towards it and \"deprecate\" the existing syntax.\n\n> Thank you for working on this and making the feature at least a little bit\n> more usable.\n\nI concur.\n\nThanks,\nTaylor\n"},{"id":"510861","messageId":"xmqqed1053ht.fsf@gitster.g","threadId":"62770","inReplyTo":"xmqqikqibsbp.fsf@gitster.g","subject":"Re: [PATCH v3] help: interpret boolean string values for help.autocorrect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-18T01:12:46Z","receivedAt":"2025-01-18T01:12:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> That's probably OK, though it is a little unlike other bools in that \"0\"\n>> is usually a strict synonym for \"false\". So we could go the other way,\n>> with \"0, false, off, no\" meaning \"show but don't run\" and leaving\n>> \"never\" by itself to mean \"do nothing\".\n>\n> That's my fault. Your version makes perfect sense.\n>\n> Thanks for being extra careful (well, more careful than myself, that\n> is).\n\nNow this left the patch in a stuck state, with the latest round v4\nstill having the \"0 and false mean different things\" caused by\nmistake. We should do the \"0 and usual 'false' all show but don't\nrun, say 'never' if you want it to be absolute no-op\", which is more\nnatural.\n\nIf I find time, I can update further based on v4, but no promises.\n\nThanks.\n"},{"id":"511646","messageId":"20250201213319.153109-1-davvid@gmail.com","threadId":"62770","inReplyTo":"pull.1869.v4.git.git.1736760824201.gitgitgadget@gmail.com","subject":"[PATCH 1/2] help: show the suggested command when help.autocorrect is false","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2025-02-01T21:33:18Z","receivedAt":"2025-02-01T21:33:22Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Make the handling of false boolean values for help.autocorrect\nconsistent with the handling of value 0 by showing the suggested\ncommands but not running them.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: David Aguilar \n---\nThis is based on the sc/help-autocorrect-one patches from this thread\nand is in response to the open question from \"What's coooking in git.git\":\n\n> On Fri, 31 Jan 2025 18:51:33 -0800 Junio C Hamano wrote:\n> Looking good except for \"should 0 and false be 'tell it without doing it'?\".\n\nsource: \n\nThis is what it would look like if the answer were to be, \"yes\".\n\n Documentation/config/help.txt | 4 ++--\n help.c | 6 ++++--\n t/t9003-help-autocorrect.sh | 17 ++++++++++-------\n 3 files changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/config/help.txt b/Documentation/config/help.txt\nindex a4c6079af8..676ba3a55f 100644\n--- a/Documentation/config/help.txt\n+++ b/Documentation/config/help.txt\n@@ -11,12 +11,12 @@ help.autoCorrect::\n \tIf git detects typos and can identify exactly one valid command similar\n \tto the error, git will try to suggest the correct command or even\n \trun the suggestion automatically. Possible config values are:\n-\t - 0: show the suggested command (default).\n+\t - 0, \"false\", \"off\", \"no\": show the suggested command (default).\n \t - 1, \"true\", \"on\", \"yes\", \"immediate\": run the suggested command\n immediately.\n \t - positive number > 1: run the suggested command after specified\n deciseconds (0.1 sec).\n-\t - \"false\", \"off\", \"no\", \"never\": don't run or show any suggested command.\n+\t - \"never\": don't run or show any suggested command.\n \t - \"prompt\": show the suggestion and prompt for confirmation to run\n the command.\n \ndiff --git a/help.c b/help.c\nindex 7148963e46..55425c0d97 100644\n--- a/help.c\n+++ b/help.c\n@@ -552,6 +552,7 @@ struct help_unknown_cmd_config {\n \tstruct cmdnames aliases;\n };\n \n+#define AUTOCORRECT_SHOW (-4)\n #define AUTOCORRECT_PROMPT (-3)\n #define AUTOCORRECT_NEVER (-2)\n #define AUTOCORRECT_IMMEDIATELY (-1)\n@@ -562,7 +563,7 @@ static int parse_autocorrect(const char *value)\n \t\tcase 1:\n \t\t\treturn AUTOCORRECT_IMMEDIATELY;\n \t\tcase 0:\n-\t\t\treturn AUTOCORRECT_NEVER;\n+\t\t\treturn AUTOCORRECT_SHOW;\n \t\tdefault: /* other random text */\n \t\t\tbreak;\n \t}\n@@ -713,7 +714,8 @@ char *help_unknown_cmd(const char *cmd)\n \t\t n++)\n \t\t\t; /* still counting */\n \t}\n-\tif (cfg.autocorrect && n == 1 && SIMILAR_ENOUGH(best_similarity)) {\n+\tif (cfg.autocorrect && cfg.autocorrect != AUTOCORRECT_SHOW && n == 1 &&\n+\t SIMILAR_ENOUGH(best_similarity)) {\n \t\tchar *assumed = xstrdup(main_cmds.names[0]->name);\n \n \t\tfprintf_ln(stderr,\ndiff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh\nindex 85a5074b5e..5ebfc00f52 100755\n--- a/t/t9003-help-autocorrect.sh\n+++ b/t/t9003-help-autocorrect.sh\n@@ -28,15 +28,18 @@ test_expect_success 'setup' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'autocorrect showing candidates' '\n-\tgit config help.autocorrect 0 &&\n+for show in false no off 0\n+do\n+\ttest_expect_success 'autocorrect showing candidates' '\n+\t\tgit config help.autocorrect $show &&\n \n-\ttest_must_fail git lfg 2>actual &&\n-\tgrep \"^\tlgf\" actual &&\n+\t\ttest_must_fail git lfg 2>actual &&\n+\t\tgrep \"^\tlgf\" actual &&\n \n-\ttest_must_fail git distimdist 2>actual &&\n-\tgrep \"^\tdistimdistim\" actual\n-'\n+\t\ttest_must_fail git distimdist 2>actual &&\n+\t\tgrep \"^\tdistimdistim\" actual\n+\t'\n+done\n \n for immediate in -1 immediate\n do\n-- \n2.48.0.rc2.34.gefa3f50b25\n\n"},{"id":"511647","messageId":"20250201213319.153109-2-davvid@gmail.com","threadId":"62770","inReplyTo":"20250201213319.153109-1-davvid@gmail.com","subject":"[PATCH 2/2] help: add \"show\" as a valid configuration value","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2025-02-01T21:33:19Z","receivedAt":"2025-02-01T21:33:23Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Add a literal value for showing the suggested autocorrection\nfor consistency with the rest of the help.autocorrect options.\n\nSigned-off-by: David Aguilar \n---\nThis is just for consistency with the other config values and\ncan be dropped if it's not useful.\n\n Documentation/config/help.txt | 2 +-\n help.c | 2 ++\n t/t9003-help-autocorrect.sh | 2 +-\n 3 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/help.txt b/Documentation/config/help.txt\nindex 676ba3a55f..b369589cec 100644\n--- a/Documentation/config/help.txt\n+++ b/Documentation/config/help.txt\n@@ -11,7 +11,7 @@ help.autoCorrect::\n \tIf git detects typos and can identify exactly one valid command similar\n \tto the error, git will try to suggest the correct command or even\n \trun the suggestion automatically. Possible config values are:\n-\t - 0, \"false\", \"off\", \"no\": show the suggested command (default).\n+\t - 0, \"false\", \"off\", \"no\", \"show\": show the suggested command (default).\n \t - 1, \"true\", \"on\", \"yes\", \"immediate\": run the suggested command\n immediately.\n \t - positive number > 1: run the suggested command after specified\ndiff --git a/help.c b/help.c\nindex 55425c0d97..8d91afe851 100644\n--- a/help.c\n+++ b/help.c\n@@ -574,6 +574,8 @@ static int parse_autocorrect(const char *value)\n \t\treturn AUTOCORRECT_NEVER;\n \tif (!strcmp(value, \"immediate\"))\n \t\treturn AUTOCORRECT_IMMEDIATELY;\n+\tif (!strcmp(value, \"show\"))\n+\t\treturn AUTOCORRECT_SHOW;\n \n \treturn 0;\n }\ndiff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh\nindex 5ebfc00f52..8da318d2b5 100755\n--- a/t/t9003-help-autocorrect.sh\n+++ b/t/t9003-help-autocorrect.sh\n@@ -28,7 +28,7 @@ test_expect_success 'setup' '\n \ttest_cmp expect actual\n '\n \n-for show in false no off 0\n+for show in false no off 0 show\n do\n \ttest_expect_success 'autocorrect showing candidates' '\n \t\tgit config help.autocorrect $show &&\n-- \n2.48.0.rc2.34.gefa3f50b25\n\n"},{"id":"511764","messageId":"xmqqpljyzlmb.fsf@gitster.g","threadId":"62770","inReplyTo":"20250201213319.153109-1-davvid@gmail.com","subject":"Re: [PATCH 1/2] help: show the suggested command when help.autocorrect is false","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-03T22:53:00Z","receivedAt":"2025-02-03T22:53:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> Make the handling of false boolean values for help.autocorrect\n> consistent with the handling of value 0 by showing the suggested\n> commands but not running them.\n>\n> Suggested-by: Junio C Hamano \n> Signed-off-by: David Aguilar \n> ---\n> This is based on the sc/help-autocorrect-one patches from this thread\n> and is in response to the open question from \"What's coooking in git.git\":\n>\n>> On Fri, 31 Jan 2025 18:51:33 -0800 Junio C Hamano wrote:\n>> Looking good except for \"should 0 and false be 'tell it without doing it'?\".\n>\n> source: \n>\n> This is what it would look like if the answer were to be, \"yes\".\n\nI obviously like the updated semantics myself.\nThanks for updating it.\n\nLet's see what others think.\n\nThanks.\n"},{"id":"511765","messageId":"xmqqldumzlle.fsf@gitster.g","threadId":"62770","inReplyTo":"20250201213319.153109-2-davvid@gmail.com","subject":"Re: [PATCH 2/2] help: add \"show\" as a valid configuration value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-03T22:53:33Z","receivedAt":"2025-02-03T22:53:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> Add a literal value for showing the suggested autocorrection\n> for consistency with the rest of the help.autocorrect options.\n>\n> Signed-off-by: David Aguilar \n> ---\n> This is just for consistency with the other config values and\n> can be dropped if it's not useful.\n>\n> Documentation/config/help.txt | 2 +-\n> help.c | 2 ++\n> t/t9003-help-autocorrect.sh | 2 +-\n> 3 files changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/config/help.txt b/Documentation/config/help.txt\n> index 676ba3a55f..b369589cec 100644\n> --- a/Documentation/config/help.txt\n> +++ b/Documentation/config/help.txt\n> @@ -11,7 +11,7 @@ help.autoCorrect::\n> \tIf git detects typos and can identify exactly one valid command similar\n> \tto the error, git will try to suggest the correct command or even\n> \trun the suggestion automatically. Possible config values are:\n> -\t - 0, \"false\", \"off\", \"no\": show the suggested command (default).\n> +\t - 0, \"false\", \"off\", \"no\", \"show\": show the suggested command (default).\n\nMakes sense.\n\n\n> \t - 1, \"true\", \"on\", \"yes\", \"immediate\": run the suggested command\n> immediately.\n> \t - positive number > 1: run the suggested command after specified\n> diff --git a/help.c b/help.c\n> index 55425c0d97..8d91afe851 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -574,6 +574,8 @@ static int parse_autocorrect(const char *value)\n> \t\treturn AUTOCORRECT_NEVER;\n> \tif (!strcmp(value, \"immediate\"))\n> \t\treturn AUTOCORRECT_IMMEDIATELY;\n> +\tif (!strcmp(value, \"show\"))\n> +\t\treturn AUTOCORRECT_SHOW;\n> \n> \treturn 0;\n> }\n> diff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh\n> index 5ebfc00f52..8da318d2b5 100755\n> --- a/t/t9003-help-autocorrect.sh\n> +++ b/t/t9003-help-autocorrect.sh\n> @@ -28,7 +28,7 @@ test_expect_success 'setup' '\n> \ttest_cmp expect actual\n> '\n> \n> -for show in false no off 0\n> +for show in false no off 0 show\n> do\n> \ttest_expect_success 'autocorrect showing candidates' '\n> \t\tgit config help.autocorrect $show &&\n"},{"id":"511781","messageId":"20250204030501.GD23954@coredump.intra.peff.net","threadId":"62770","inReplyTo":"xmqqpljyzlmb.fsf@gitster.g","subject":"Re: [PATCH 1/2] help: show the suggested command when help.autocorrect is false","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-02-04T03:05:01Z","receivedAt":"2025-02-04T03:05:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 03, 2025 at 02:53:00PM -0800, Junio C Hamano wrote:\n\n> David Aguilar writes:\n> \n> > Make the handling of false boolean values for help.autocorrect\n> > consistent with the handling of value 0 by showing the suggested\n> > commands but not running them.\n> >\n> > Suggested-by: Junio C Hamano \n> > Signed-off-by: David Aguilar \n> > ---\n> > This is based on the sc/help-autocorrect-one patches from this thread\n> > and is in response to the open question from \"What's coooking in git.git\":\n> >\n> >> On Fri, 31 Jan 2025 18:51:33 -0800 Junio C Hamano wrote:\n> >> Looking good except for \"should 0 and false be 'tell it without doing it'?\".\n> >\n> > source: \n> >\n> > This is what it would look like if the answer were to be, \"yes\".\n> \n> I obviously like the updated semantics myself.\n> Thanks for updating it.\n> \n> Let's see what others think.\n\nI like it (including the new \"show\" which is even more descriptive).\n\n-Peff\n"},{"id":"511806","messageId":"xmqq34gtx22o.fsf@gitster.g","threadId":"62770","inReplyTo":"20250204030501.GD23954@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] help: show the suggested command when help.autocorrect is false","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-04T13:38:07Z","receivedAt":"2025-02-04T13:38:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Feb 03, 2025 at 02:53:00PM -0800, Junio C Hamano wrote:\n>\n>> David Aguilar writes:\n>> \n>> > Make the handling of false boolean values for help.autocorrect\n>> > consistent with the handling of value 0 by showing the suggested\n>> > commands but not running them.\n>> >\n>> > Suggested-by: Junio C Hamano \n>> > Signed-off-by: David Aguilar \n>> > ---\n>> > This is based on the sc/help-autocorrect-one patches from this thread\n>> > and is in response to the open question from \"What's coooking in git.git\":\n>> >\n>> >> On Fri, 31 Jan 2025 18:51:33 -0800 Junio C Hamano wrote:\n>> >> Looking good except for \"should 0 and false be 'tell it without doing it'?\".\n>> >\n>> > source: \n>> >\n>> > This is what it would look like if the answer were to be, \"yes\".\n>> \n>> I obviously like the updated semantics myself.\n>> Thanks for updating it.\n>> \n>> Let's see what others think.\n>\n> I like it (including the new \"show\" which is even more descriptive).\n\nThanks, really appreciate it.\n"}]}