[PATCH 0/5] parseopt: add subcommand autocorrection

[PATCH 0/5] parseopt: add subcommand autocorrection

[Jiamu Sun]

Git currently provides auto-correction for builtins and aliases, but
lacks this functionality for subcommands parsed via the parse-options
API. Subcommands are also commands, and typos will occur, too. Like:

	git remote add-rul

So, this series introduces subcommand auto-correction.

Currently, builtins with mandatory subcommands enable autocorrection by
default. However, those using PARSE_OPT_SUBCOMMAND_OPTIONAL skip it to
avoid treating valid unknown arguments as mistyped subcommands.

This series adds PARSE_OPT_SUBCOMMAND_AUTOCORR, allowing commands with
optional subcommands to explicitly opt in to autocorrection. It then
enables this flag for git-remote and git-notes.

Additionally, it extracts the existing autocorrection logic from help.c
so subcommand handling can reuse the same config parsing and
prompt/delay logic.

Jiamu Sun (5):
  parseopt: extract subcommand handling from parse_options_step()
  help: refactor command autocorrection handling
  parseopt: autocorrect mistyped subcommands
  parseopt: enable subcommand autocorrect for remote and notes
  help: add tests for subcommand autocorrection

 Makefile                          |   1 +
 autocorrect.c                     |  92 +++++++++++++++++++
 autocorrect.h                     |  23 +++++
 builtin/notes.c                   |  10 +-
 builtin/remote.c                  |  12 +--
 help.c                            | 106 ++++-----------------
 parse-options.c                   | 147 +++++++++++++++++++++++-------
 parse-options.h                   |   1 +
 t/t9004-autocorrect-subcommand.sh |  49 ++++++++++
 9 files changed, 306 insertions(+), 135 deletions(-)
 create mode 100644 autocorrect.c
 create mode 100644 autocorrect.h
 create mode 100755 t/t9004-autocorrect-subcommand.sh


base-commit: 795c338de725e13bd361214c6b768019fc45a2c1
-- 
2.53.0

[PATCH 1/5] parseopt: extract subcommand handling from parse_options_step()

[Jiamu Sun]

Move the subcommand branch out of parse_options_step() into a new
handle_subcommand() helper. Also, make parse_subcommand() return a
simple success/failure status.

This removes the switch over impossible parse_opt_result values and
makes the non-option path easier to follow and maintain.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 parse-options.c | 75 ++++++++++++++++++++++++++-----------------------
 1 file changed, 40 insertions(+), 35 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index c9cafc21b903..6bb0c5697099 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -605,17 +605,43 @@ static enum parse_opt_result parse_nodash_opt(struct parse_opt_ctx_t *p,
 	return PARSE_OPT_ERROR;
 }
 
-static enum parse_opt_result parse_subcommand(const char *arg,
-					      const struct option *options)
+static int parse_subcommand(const char *arg, const struct option *options)
 {
-	for (; options->type != OPTION_END; options++)
-		if (options->type == OPTION_SUBCOMMAND &&
-		    !strcmp(options->long_name, arg)) {
-			*(parse_opt_subcommand_fn **)options->value = options->subcommand_fn;
-			return PARSE_OPT_SUBCOMMAND;
-		}
+	for (; options->type != OPTION_END; options++) {
+		if (options->type != OPTION_SUBCOMMAND ||
+		    strcmp(options->long_name, arg))
+			continue;
 
-	return PARSE_OPT_UNKNOWN;
+		parse_opt_subcommand_fn **opt_val = options->value;
+		*opt_val = options->subcommand_fn;
+
+		return 0;
+	}
+
+	return -1;
+}
+
+static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
+					       const char *arg,
+					       const struct option *options,
+					       const char * const usagestr[])
+{
+	int err = parse_subcommand(arg, options);
+
+	if (!err)
+		return PARSE_OPT_SUBCOMMAND;
+
+	/*
+	 * arg is neither a short or long option nor a subcommand.  Since this
+	 * command has a default operation mode, we have to treat this arg and
+	 * all remaining args as args meant to that default operation mode.
+	 * So we are done parsing.
+	 */
+	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
+		return PARSE_OPT_DONE;
+
+	error(_("unknown subcommand: `%s'"), arg);
+	usage_with_options(usagestr, options);
 }
 
 static void check_typos(const char *arg, const struct option *options)
@@ -990,37 +1016,16 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
 		if (*arg != '-' || !arg[1]) {
 			if (parse_nodash_opt(ctx, arg, options) == 0)
 				continue;
+
 			if (!ctx->has_subcommands) {
 				if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
 					return PARSE_OPT_NON_OPTION;
 				ctx->out[ctx->cpidx++] = ctx->argv[0];
 				continue;
-			}
-			switch (parse_subcommand(arg, options)) {
-			case PARSE_OPT_SUBCOMMAND:
-				return PARSE_OPT_SUBCOMMAND;
-			case PARSE_OPT_UNKNOWN:
-				if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
-					/*
-					 * arg is neither a short or long
-					 * option nor a subcommand.  Since
-					 * this command has a default
-					 * operation mode, we have to treat
-					 * this arg and all remaining args
-					 * as args meant to that default
-					 * operation mode.
-					 * So we are done parsing.
-					 */
-					return PARSE_OPT_DONE;
-				error(_("unknown subcommand: `%s'"), arg);
-				usage_with_options(usagestr, options);
-			case PARSE_OPT_COMPLETE:
-			case PARSE_OPT_HELP:
-			case PARSE_OPT_ERROR:
-			case PARSE_OPT_DONE:
-			case PARSE_OPT_NON_OPTION:
-				/* Impossible. */
-				BUG("parse_subcommand() cannot return these");
+
+			} else {
+				return handle_subcommand(ctx, arg,
+							 options, usagestr);
 			}
 		}
 
-- 
2.53.0

[PATCH 2/5] help: refactor command autocorrection handling

[Jiamu Sun]

Refactor autocorrect config into struct autocorr, with an explicit mode
enum and a separate delay field. Move config parsing, TTY validation,
and prompt handling into autocorrect.c.

This simplifies help.c and makes state handling for autocorrect explicit
and easier to maintain.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 Makefile      |   1 +
 autocorrect.c |  92 +++++++++++++++++++++++++++++++++++++++++++
 autocorrect.h |  23 +++++++++++
 help.c        | 106 ++++++++++----------------------------------------
 4 files changed, 136 insertions(+), 86 deletions(-)
 create mode 100644 autocorrect.c
 create mode 100644 autocorrect.h

diff --git a/Makefile b/Makefile
index f3264d0a37cc..6111631c2caa 100644
--- a/Makefile
+++ b/Makefile
@@ -1098,6 +1098,7 @@ LIB_OBJS += archive-tar.o
 LIB_OBJS += archive-zip.o
 LIB_OBJS += archive.o
 LIB_OBJS += attr.o
+LIB_OBJS += autocorrect.o
 LIB_OBJS += base85.o
 LIB_OBJS += bisect.o
 LIB_OBJS += blame.o
diff --git a/autocorrect.c b/autocorrect.c
new file mode 100644
index 000000000000..eaae01645910
--- /dev/null
+++ b/autocorrect.c
@@ -0,0 +1,92 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
+#include "git-compat-util.h"
+#include "autocorrect.h"
+#include "config.h"
+#include "parse.h"
+#include "strbuf.h"
+#include "prompt.h"
+#include "gettext.h"
+
+static enum autocorr_mode parse_autocorr(const char *value)
+{
+	switch (git_parse_maybe_bool_text(value)) {
+		case 1:
+			return AUTOCORR_IMMEDIATELY;
+		case 0:
+			return AUTOCORR_HINTONLY;
+		default: /* other random text */
+			break;
+	}
+
+	if (!strcmp(value, "prompt"))
+		return AUTOCORR_PROMPT;
+	else if (!strcmp(value, "never"))
+		return AUTOCORR_NEVER;
+	else if (!strcmp(value, "immediate"))
+		return AUTOCORR_IMMEDIATELY;
+	else if (!strcmp(value, "show"))
+		return AUTOCORR_HINTONLY;
+	else
+		return AUTOCORR_DELAY;
+}
+
+static int config_cb(const char *var, const char *value,
+		     const struct config_context *ctx, void *data)
+{
+	struct autocorr *conf = data;
+
+	if (strcmp(var, "help.autocorrect"))
+		return 0;
+
+	conf->mode = parse_autocorr(value);
+
+	/*
+	 * Disable autocorrection prompt in a non-interactive session
+	 */
+	if (conf->mode == AUTOCORR_PROMPT && (!isatty(0) || !isatty(2)))
+		conf->mode = AUTOCORR_NEVER;
+
+	if (conf->mode == AUTOCORR_DELAY) {
+		conf->delay = git_config_int(var, value, ctx->kvi);
+
+		if (conf->delay == 0)
+			conf->mode = AUTOCORR_HINTONLY;
+		else if (conf->delay <= 1)
+			conf->mode = AUTOCORR_IMMEDIATELY;
+	}
+
+	return 0;
+}
+
+void autocorr_resolve(struct autocorr *conf)
+{
+	read_early_config(the_repository, config_cb, conf);
+}
+
+void autocorr_prompt_or_delay(struct autocorr *conf, const char *assumed)
+{
+	if (conf->mode == AUTOCORR_IMMEDIATELY) {
+		fprintf_ln(stderr,
+			   _("Continuing under the assumption that you meant '%s'."),
+			   assumed);
+
+	} else if (conf->mode == AUTOCORR_PROMPT) {
+		char *answer;
+		struct strbuf msg = STRBUF_INIT;
+
+		strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
+		answer = git_prompt(msg.buf, PROMPT_ECHO);
+		strbuf_release(&msg);
+
+		if (!(starts_with(answer, "y") || starts_with(answer, "Y")))
+			exit(1);
+
+	} else if (conf->mode == AUTOCORR_DELAY) {
+		fprintf_ln(stderr,
+			   _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
+			   (float)conf->delay / 10.0, assumed);
+
+		sleep_millisec(conf->delay * 100);
+	}
+}
diff --git a/autocorrect.h b/autocorrect.h
new file mode 100644
index 000000000000..ea21811a43f2
--- /dev/null
+++ b/autocorrect.h
@@ -0,0 +1,23 @@
+#ifndef AUTOCORRECT_H
+#define AUTOCORRECT_H
+
+struct config_context;
+
+enum autocorr_mode {
+	AUTOCORR_HINTONLY,
+	AUTOCORR_NEVER,
+	AUTOCORR_PROMPT,
+	AUTOCORR_IMMEDIATELY,
+	AUTOCORR_DELAY,
+};
+
+struct autocorr {
+	enum autocorr_mode mode;
+	int delay;
+};
+
+void autocorr_resolve(struct autocorr *conf);
+
+void autocorr_prompt_or_delay(struct autocorr *conf, const char *assumed);
+
+#endif /* AUTOCORRECT_H */
diff --git a/help.c b/help.c
index 95f576c5c81d..5eec6a34de33 100644
--- a/help.c
+++ b/help.c
@@ -22,6 +22,7 @@
 #include "repository.h"
 #include "alias.h"
 #include "utf8.h"
+#include "autocorrect.h"
 
 #ifndef NO_CURL
 #include "git-curl-compat.h" /* For LIBCURL_VERSION only */
@@ -536,70 +537,23 @@ int is_in_cmdlist(struct cmdnames *c, const char *s)
 	return 0;
 }
 
-struct help_unknown_cmd_config {
-	int autocorrect;
-	struct cmdnames aliases;
-};
-
-#define AUTOCORRECT_SHOW (-4)
-#define AUTOCORRECT_PROMPT (-3)
-#define AUTOCORRECT_NEVER (-2)
-#define AUTOCORRECT_IMMEDIATELY (-1)
-
-static int parse_autocorrect(const char *value)
+static int resolve_aliases(const char *var, const char *value,
+			   const struct config_context *ctx, void *data)
 {
-	switch (git_parse_maybe_bool_text(value)) {
-		case 1:
-			return AUTOCORRECT_IMMEDIATELY;
-		case 0:
-			return AUTOCORRECT_SHOW;
-		default: /* other random text */
-			break;
-	}
-
-	if (!strcmp(value, "prompt"))
-		return AUTOCORRECT_PROMPT;
-	if (!strcmp(value, "never"))
-		return AUTOCORRECT_NEVER;
-	if (!strcmp(value, "immediate"))
-		return AUTOCORRECT_IMMEDIATELY;
-	if (!strcmp(value, "show"))
-		return AUTOCORRECT_SHOW;
-
-	return 0;
-}
-
-static int git_unknown_cmd_config(const char *var, const char *value,
-				  const struct config_context *ctx,
-				  void *cb)
-{
-	struct help_unknown_cmd_config *cfg = cb;
+	struct cmdnames *aliases = data;
 	const char *subsection, *key;
 	size_t subsection_len;
 
-	if (!strcmp(var, "help.autocorrect")) {
-		int v = parse_autocorrect(value);
-
-		if (!v) {
-			v = git_config_int(var, value, ctx->kvi);
-			if (v < 0 || v == 1)
-				v = AUTOCORRECT_IMMEDIATELY;
-		}
-
-		cfg->autocorrect = v;
-	}
-
-	/* Also use aliases for command lookup */
 	if (!parse_config_key(var, "alias", &subsection, &subsection_len,
 			      &key)) {
 		if (subsection) {
 			/* [alias "name"] command = value */
 			if (!strcmp(key, "command"))
-				add_cmdname(&cfg->aliases, subsection,
+				add_cmdname(aliases, subsection,
 					    subsection_len);
 		} else {
 			/* alias.name = value */
-			add_cmdname(&cfg->aliases, key, strlen(key));
+			add_cmdname(aliases, key, strlen(key));
 		}
 	}
 
@@ -636,28 +590,27 @@ static const char bad_interpreter_advice[] =
 
 char *help_unknown_cmd(const char *cmd)
 {
-	struct help_unknown_cmd_config cfg = { 0 };
+	struct cmdnames aliases = { 0 };
+	struct autocorr autocorr = { 0 };
+
 	int i, n, best_similarity = 0;
 	struct cmdnames main_cmds = { 0 };
 	struct cmdnames other_cmds = { 0 };
 	struct cmdname_help *common_cmds;
 
-	read_early_config(the_repository, git_unknown_cmd_config, &cfg);
+	autocorr_resolve(&autocorr);
 
-	/*
-	 * Disable autocorrection prompt in a non-interactive session
-	 */
-	if ((cfg.autocorrect == AUTOCORRECT_PROMPT) && (!isatty(0) || !isatty(2)))
-		cfg.autocorrect = AUTOCORRECT_NEVER;
+	/* Also use aliases for command lookup */
+	read_early_config(the_repository, resolve_aliases, &aliases);
 
-	if (cfg.autocorrect == AUTOCORRECT_NEVER) {
+	if (autocorr.mode == AUTOCORR_NEVER) {
 		fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
 		exit(1);
 	}
 
 	load_command_list("git-", &main_cmds, &other_cmds);
 
-	add_cmd_list(&main_cmds, &cfg.aliases);
+	add_cmd_list(&main_cmds, &aliases);
 	add_cmd_list(&main_cmds, &other_cmds);
 	QSORT(main_cmds.names, main_cmds.cnt, cmdname_compare);
 	uniq(&main_cmds);
@@ -716,37 +669,18 @@ char *help_unknown_cmd(const char *cmd)
 		     n++)
 			; /* still counting */
 	}
-	if (cfg.autocorrect && cfg.autocorrect != AUTOCORRECT_SHOW && n == 1 &&
+
+	if (autocorr.mode != AUTOCORR_HINTONLY && n == 1 &&
 	    SIMILAR_ENOUGH(best_similarity)) {
 		char *assumed = xstrdup(main_cmds.names[0]->name);
 
 		fprintf_ln(stderr,
-			   _("WARNING: You called a Git command named '%s', "
-			     "which does not exist."),
+			   _("WARNING: You called a Git command named '%s', which does not exist."),
 			   cmd);
-		if (cfg.autocorrect == AUTOCORRECT_IMMEDIATELY)
-			fprintf_ln(stderr,
-				   _("Continuing under the assumption that "
-				     "you meant '%s'."),
-				   assumed);
-		else if (cfg.autocorrect == AUTOCORRECT_PROMPT) {
-			char *answer;
-			struct strbuf msg = STRBUF_INIT;
-			strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
-			answer = git_prompt(msg.buf, PROMPT_ECHO);
-			strbuf_release(&msg);
-			if (!(starts_with(answer, "y") ||
-			      starts_with(answer, "Y")))
-				exit(1);
-		} else {
-			fprintf_ln(stderr,
-				   _("Continuing in %0.1f seconds, "
-				     "assuming that you meant '%s'."),
-				   (float)cfg.autocorrect/10.0, assumed);
-			sleep_millisec(cfg.autocorrect * 100);
-		}
 
-		cmdnames_release(&cfg.aliases);
+		autocorr_prompt_or_delay(&autocorr, assumed);
+
+		cmdnames_release(&aliases);
 		cmdnames_release(&main_cmds);
 		cmdnames_release(&other_cmds);
 		return assumed;
-- 
2.53.0

[PATCH 3/5] parseopt: autocorrect mistyped subcommands

[Jiamu Sun]

Enable autocorrection for mandatory subcommands parsed through
parse-options.

AUTOCORR_HINTONLY is equivalent to AUTOCORR_NEVER here, because builtins
have a limited number of subcommands. They won't become too large.
Displaying all subcommands via usage_with_options() is already good
enough. This keeps the autocorrection handling simple, too.

Also, use a dynamic threshold for similar_enough(), which can yield more
accurate typo correction results. Although subcommands are often short,
they can still vary across builtins. And a fixed threshold won't do
better on both short and long subcommands at the same time.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 parse-options.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 6bb0c5697099..cbbb04b9997a 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -6,6 +6,8 @@
 #include "strbuf.h"
 #include "string-list.h"
 #include "utf8.h"
+#include "autocorrect.h"
+#include "levenshtein.h"
 
 static int disallow_abbreviated_options;
 
@@ -621,6 +623,64 @@ static int parse_subcommand(const char *arg, const struct option *options)
 	return -1;
 }
 
+static void find_subcommands(struct string_list *list,
+			     const struct option *options)
+{
+	for (; options->type != OPTION_END; options++) {
+		if (options->type == OPTION_SUBCOMMAND)
+			string_list_append(list, options->long_name);
+	}
+}
+
+static int similar_enough(const char *cmd, unsigned int dist)
+{
+	size_t len = strlen(cmd);
+	unsigned int threshold = len < 3 ? 1 : len < 6 ? 3 : 6;
+
+	return dist < threshold;
+}
+
+static const char *autocorrect_subcmd(const char *cmd,
+				      struct string_list *cmds)
+{
+	struct autocorr autocorr = { 0 };
+
+	autocorr_resolve(&autocorr);
+
+	if (autocorr.mode == AUTOCORR_NEVER ||
+	    autocorr.mode == AUTOCORR_HINTONLY)
+		return NULL;
+
+	unsigned int min = -1;
+	unsigned int ties = 0;
+	struct string_list_item *cand;
+	struct string_list_item *best = NULL;
+
+	for_each_string_list_item(cand, cmds) {
+		unsigned int dist = levenshtein(cmd, cand->string, 0, 2, 1, 3);
+
+		if (dist < min) {
+			min = dist;
+			best = cand;
+			ties = 0;
+
+		} else if (dist == min) {
+			ties++;
+		}
+	}
+
+	if (ties == 0 && similar_enough(cmd, min)) {
+		fprintf_ln(stderr,
+			   _("WARNING: You called a subcommand named '%s', which does not exist."),
+			   cmd);
+
+		autocorr_prompt_or_delay(&autocorr, best->string);
+		return best->string;
+	}
+
+	return NULL;
+}
+
 static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 					       const char *arg,
 					       const struct option *options,
@@ -640,8 +700,20 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
 		return PARSE_OPT_DONE;
 
-	error(_("unknown subcommand: `%s'"), arg);
-	usage_with_options(usagestr, options);
+	struct string_list cmds = STRING_LIST_INIT_NODUP;
+
+	find_subcommands(&cmds, options);
+
+	const char *cmd = autocorrect_subcmd(arg, &cmds);
+
+	if (!cmd) {
+		error(_("unknown subcommand: `%s'"), arg);
+		usage_with_options(usagestr, options);
+	}
+
+	string_list_clear(&cmds, 0);
+	parse_subcommand(cmd, options);
+	return PARSE_OPT_SUBCOMMAND;
 }
 
 static void check_typos(const char *arg, const struct option *options)
-- 
2.53.0

[PATCH 4/5] parseopt: enable subcommand autocorrect for remote and notes

[Jiamu Sun]

Add PARSE_OPT_SUBCOMMAND_AUTOCORR to enable autocorrection for
subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL.

Use it for git-remote and git-notes, so mistyped subcommands can be
automatically corrected, and builtin entry points no longer need
to handle missing subcommand error path themselves.

This is safe for these two builtins, because they either resolve to a
single subcommand or take no subcommand at all. This means that if the
subcommand parser encounters an unknown argument, it must be a mistyped
subcommand.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 builtin/notes.c  | 10 +++-------
 builtin/remote.c | 12 ++++--------
 parse-options.c  | 16 +++++++++-------
 parse-options.h  |  1 +
 4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 9af602bdd7b4..087eb898a441 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -1149,14 +1149,10 @@ int cmd_notes(int argc,
 
 	repo_config(the_repository, git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, git_notes_usage,
-			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
-	if (!fn) {
-		if (argc) {
-			error(_("unknown subcommand: `%s'"), argv[0]);
-			usage_with_options(git_notes_usage, options);
-		}
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL |
+			     PARSE_OPT_SUBCOMMAND_AUTOCORR);
+	if (!fn)
 		fn = list;
-	}
 
 	if (override_notes_ref) {
 		struct strbuf sb = STRBUF_INIT;
diff --git a/builtin/remote.c b/builtin/remote.c
index ace390c671d6..d1d6244a662a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1949,15 +1949,11 @@ int cmd_remote(int argc,
 	};
 
 	argc = parse_options(argc, argv, prefix, options, builtin_remote_usage,
-			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL |
+			     PARSE_OPT_SUBCOMMAND_AUTOCORR);
 
-	if (fn) {
+	if (fn)
 		return !!fn(argc, argv, prefix, repo);
-	} else {
-		if (argc) {
-			error(_("unknown subcommand: `%s'"), argv[0]);
-			usage_with_options(builtin_remote_usage, options);
-		}
+	else
 		return !!show_all();
-	}
 }
diff --git a/parse-options.c b/parse-options.c
index cbbb04b9997a..b7c818e818fe 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -691,14 +691,16 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 	if (!err)
 		return PARSE_OPT_SUBCOMMAND;
 
-	/*
-	 * arg is neither a short or long option nor a subcommand.  Since this
-	 * command has a default operation mode, we have to treat this arg and
-	 * all remaining args as args meant to that default operation mode.
-	 * So we are done parsing.
-	 */
-	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
+	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL &&
+	    !(ctx->flags & PARSE_OPT_SUBCOMMAND_AUTOCORR)) {
+		/*
+		 * arg is neither a short or long option nor a subcommand.
+		 * Since this command has a default operation mode, we have to
+		 * treat this arg and all remaining args as args meant to that
+		 * default operation mode.  So we are done parsing.
+		 */
 		return PARSE_OPT_DONE;
+	}
 
 	struct string_list cmds = STRING_LIST_INIT_NODUP;
 
diff --git a/parse-options.h b/parse-options.h
index 706de9729f6b..f29ac337893c 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -40,6 +40,7 @@ enum parse_opt_flags {
 	PARSE_OPT_ONE_SHOT = 1 << 5,
 	PARSE_OPT_SHELL_EVAL = 1 << 6,
 	PARSE_OPT_SUBCOMMAND_OPTIONAL = 1 << 7,
+	PARSE_OPT_SUBCOMMAND_AUTOCORR = 1 << 8,
 };
 
 enum parse_opt_option_flags {
-- 
2.53.0

[PATCH 5/5] help: add tests for subcommand autocorrection

[Jiamu Sun]

These tests cover default behavior (help.autocorrect is unset), no
correction, immediate correction, delayed correction, and rejection
when the typo is too dissimilar.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 t/t9004-autocorrect-subcommand.sh | 49 +++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100755 t/t9004-autocorrect-subcommand.sh

diff --git a/t/t9004-autocorrect-subcommand.sh b/t/t9004-autocorrect-subcommand.sh
new file mode 100755
index 000000000000..760760c8851a
--- /dev/null
+++ b/t/t9004-autocorrect-subcommand.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='subcommand auto-correction test
+
+Test autocorrection for subcommands with different
+help.autocorrect mode.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' "
+	echo '^error: unknown subcommand: ' >grep_unknown
+"
+
+test_expect_success 'default is not to autocorrect' '
+	test_must_fail git worktree lsit 2>actual &&
+	head -n1 actual >first && test_grep -f grep_unknown first
+'
+
+for mode in false no off 0 show never; do
+	test_expect_success "'$mode' disables autocorrection" "
+		test_config help.autocorrect $mode &&
+
+		test_must_fail git worktree lsit 2>actual &&
+		head -n1 actual >first && test_grep -f grep_unknown first
+	"
+done
+
+for mode in -39 immediate 1; do
+	test_expect_success "autocorrect immediately with '$mode'" - <<-EOT
+		test_config help.autocorrect $mode &&
+
+		git worktree lsit 2>actual &&
+		test_grep "you meant 'list'\.$" actual
+	EOT
+done
+
+test_expect_success 'delay path is executed' - <<-\EOT
+	test_config help.autocorrect 2 &&
+
+	git worktree lsit 2>actual &&
+	test_grep '^Continuing in 0.2 seconds, ' actual
+EOT
+
+test_expect_success 'deny if too dissimilar' - <<-\EOT
+	test_must_fail git remote rensnr 2>actual &&
+	head -n1 actual >first && test_grep -f grep_unknown first
+EOT
+
+test_done
-- 
2.53.0

Re: [PATCH 0/5] parseopt: add subcommand autocorrection

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> Git currently provides auto-correction for builtins and aliases, but
> lacks this functionality for subcommands parsed via the parse-options
> API. Subcommands are also commands, and typos will occur, too. Like:
>
> 	git remote add-rul
>
> So, this series introduces subcommand auto-correction.
>
> Currently, builtins with mandatory subcommands enable autocorrection by
> default. However, those using PARSE_OPT_SUBCOMMAND_OPTIONAL skip it to
> avoid treating valid unknown arguments as mistyped subcommands.

This is a bit confusing as it describes the behavior introduced by
this series rather than the state of the codebase before these
patches.  Since subcommand autocorrection doesn't exist yet, it
would be clearer to phrase this as describing your implementation
choices. Perhaps:

    By default, this implementation enables autocorrection for
    builtins with mandatory subcommands. However, for those using
    PARSE_OPT_SUBCOMMAND_OPTIONAL, autocorrection is skipped to
    avoid misinterpreting legitimate unknown arguments as mistyped
    subcommands.

or something, perhaps?

Re: [PATCH 0/5] parseopt: add subcommand autocorrection

[Jiamu Sun]

On Sun, Mar 08, 2026 at 01:11:22PM -0700, Junio C Hamano wrote:
> Jiamu Sun <39@barroit.sh> writes:
> 
> > So, this series introduces subcommand auto-correction.
> >
> > Currently, builtins with mandatory subcommands enable autocorrection by
> > default. However, those using PARSE_OPT_SUBCOMMAND_OPTIONAL skip it to
> > avoid treating valid unknown arguments as mistyped subcommands.
> 
> This is a bit confusing as it describes the behavior introduced by
> this series rather than the state of the codebase before these
> patches.  Since subcommand autocorrection doesn't exist yet, it
> would be clearer to phrase this as describing your implementation
> choices. Perhaps:
> 
>     By default, this implementation enables autocorrection for
>     builtins with mandatory subcommands. However, for those using
>     PARSE_OPT_SUBCOMMAND_OPTIONAL, autocorrection is skipped to
>     avoid misinterpreting legitimate unknown arguments as mistyped
>     subcommands.
> 
> or something, perhaps?

Right, that's confusing. By "currently", I meant "in this patch".

Will reword the cover letter in v2 to use your phrasing. Thanks!

-- 
Jiamu Sun <39@barroit.sh>

[PATCH v2 0/5] parseopt: add subcommand autocorrection

[Jiamu Sun]

Git currently provides auto-correction for builtins and aliases, but
lacks this functionality for subcommands parsed via the parse-options
API. Subcommands are also commands, and typos will occur, too. Like:

	git remote add-rul

So, this series introduces subcommand auto-correction.

By default, this implementation enables autocorrection for builtins
with mandatory subcommands. However, for those using
PARSE_OPT_SUBCOMMAND_OPTIONAL, autocorrection is skipped to avoid
misinterpreting legitimate unknown arguments as mistyped subcommands.

To allow builtins with optional subcommands to explicitly opt in,
this series adds the PARSE_OPT_SUBCOMMAND_AUTOCORR flag. This flag
is subsequently enabled for git remote and git notes.

Additionally, the existing autocorrection logic is extracted from
help.c so subcommand handling can reuse the same config parsing and
prompt/delay logic.

Changes in v2:
  - Reworded the explanation of default autocorrection behavior.

Jiamu Sun (5):
  parseopt: extract subcommand handling from parse_options_step()
  help: refactor command autocorrection handling
  parseopt: autocorrect mistyped subcommands
  parseopt: enable subcommand autocorrect for remote and notes
  help: add tests for subcommand autocorrection

 Makefile                          |   1 +
 autocorrect.c                     |  92 +++++++++++++++++++
 autocorrect.h                     |  23 +++++
 builtin/notes.c                   |  10 +-
 builtin/remote.c                  |  12 +--
 help.c                            | 106 ++++-----------------
 parse-options.c                   | 147 +++++++++++++++++++++++-------
 parse-options.h                   |   1 +
 t/t9004-autocorrect-subcommand.sh |  49 ++++++++++
 9 files changed, 306 insertions(+), 135 deletions(-)
 create mode 100644 autocorrect.c
 create mode 100644 autocorrect.h
 create mode 100755 t/t9004-autocorrect-subcommand.sh


base-commit: 795c338de725e13bd361214c6b768019fc45a2c1
-- 
2.53.0

[PATCH v2 1/5] parseopt: extract subcommand handling from parse_options_step()

[Jiamu Sun]

Move the subcommand branch out of parse_options_step() into a new
handle_subcommand() helper. Also, make parse_subcommand() return a
simple success/failure status.

This removes the switch over impossible parse_opt_result values and
makes the non-option path easier to follow and maintain.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 parse-options.c | 75 ++++++++++++++++++++++++++-----------------------
 1 file changed, 40 insertions(+), 35 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index c9cafc21b903..6bb0c5697099 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -605,17 +605,43 @@ static enum parse_opt_result parse_nodash_opt(struct parse_opt_ctx_t *p,
 	return PARSE_OPT_ERROR;
 }
 
-static enum parse_opt_result parse_subcommand(const char *arg,
-					      const struct option *options)
+static int parse_subcommand(const char *arg, const struct option *options)
 {
-	for (; options->type != OPTION_END; options++)
-		if (options->type == OPTION_SUBCOMMAND &&
-		    !strcmp(options->long_name, arg)) {
-			*(parse_opt_subcommand_fn **)options->value = options->subcommand_fn;
-			return PARSE_OPT_SUBCOMMAND;
-		}
+	for (; options->type != OPTION_END; options++) {
+		if (options->type != OPTION_SUBCOMMAND ||
+		    strcmp(options->long_name, arg))
+			continue;
 
-	return PARSE_OPT_UNKNOWN;
+		parse_opt_subcommand_fn **opt_val = options->value;
+		*opt_val = options->subcommand_fn;
+
+		return 0;
+	}
+
+	return -1;
+}
+
+static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
+					       const char *arg,
+					       const struct option *options,
+					       const char * const usagestr[])
+{
+	int err = parse_subcommand(arg, options);
+
+	if (!err)
+		return PARSE_OPT_SUBCOMMAND;
+
+	/*
+	 * arg is neither a short or long option nor a subcommand.  Since this
+	 * command has a default operation mode, we have to treat this arg and
+	 * all remaining args as args meant to that default operation mode.
+	 * So we are done parsing.
+	 */
+	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
+		return PARSE_OPT_DONE;
+
+	error(_("unknown subcommand: `%s'"), arg);
+	usage_with_options(usagestr, options);
 }
 
 static void check_typos(const char *arg, const struct option *options)
@@ -990,37 +1016,16 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
 		if (*arg != '-' || !arg[1]) {
 			if (parse_nodash_opt(ctx, arg, options) == 0)
 				continue;
+
 			if (!ctx->has_subcommands) {
 				if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
 					return PARSE_OPT_NON_OPTION;
 				ctx->out[ctx->cpidx++] = ctx->argv[0];
 				continue;
-			}
-			switch (parse_subcommand(arg, options)) {
-			case PARSE_OPT_SUBCOMMAND:
-				return PARSE_OPT_SUBCOMMAND;
-			case PARSE_OPT_UNKNOWN:
-				if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
-					/*
-					 * arg is neither a short or long
-					 * option nor a subcommand.  Since
-					 * this command has a default
-					 * operation mode, we have to treat
-					 * this arg and all remaining args
-					 * as args meant to that default
-					 * operation mode.
-					 * So we are done parsing.
-					 */
-					return PARSE_OPT_DONE;
-				error(_("unknown subcommand: `%s'"), arg);
-				usage_with_options(usagestr, options);
-			case PARSE_OPT_COMPLETE:
-			case PARSE_OPT_HELP:
-			case PARSE_OPT_ERROR:
-			case PARSE_OPT_DONE:
-			case PARSE_OPT_NON_OPTION:
-				/* Impossible. */
-				BUG("parse_subcommand() cannot return these");
+
+			} else {
+				return handle_subcommand(ctx, arg,
+							 options, usagestr);
 			}
 		}
 
-- 
2.53.0

[PATCH v2 2/5] help: refactor command autocorrection handling

[Jiamu Sun]

Refactor autocorrect config into struct autocorr, with an explicit mode
enum and a separate delay field. Move config parsing, TTY validation,
and prompt handling into autocorrect.c.

This simplifies help.c and makes state handling for autocorrect explicit
and easier to maintain.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 Makefile      |   1 +
 autocorrect.c |  92 +++++++++++++++++++++++++++++++++++++++++++
 autocorrect.h |  23 +++++++++++
 help.c        | 106 ++++++++++----------------------------------------
 4 files changed, 136 insertions(+), 86 deletions(-)
 create mode 100644 autocorrect.c
 create mode 100644 autocorrect.h

diff --git a/Makefile b/Makefile
index f3264d0a37cc..6111631c2caa 100644
--- a/Makefile
+++ b/Makefile
@@ -1098,6 +1098,7 @@ LIB_OBJS += archive-tar.o
 LIB_OBJS += archive-zip.o
 LIB_OBJS += archive.o
 LIB_OBJS += attr.o
+LIB_OBJS += autocorrect.o
 LIB_OBJS += base85.o
 LIB_OBJS += bisect.o
 LIB_OBJS += blame.o
diff --git a/autocorrect.c b/autocorrect.c
new file mode 100644
index 000000000000..eaae01645910
--- /dev/null
+++ b/autocorrect.c
@@ -0,0 +1,92 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
+#include "git-compat-util.h"
+#include "autocorrect.h"
+#include "config.h"
+#include "parse.h"
+#include "strbuf.h"
+#include "prompt.h"
+#include "gettext.h"
+
+static enum autocorr_mode parse_autocorr(const char *value)
+{
+	switch (git_parse_maybe_bool_text(value)) {
+		case 1:
+			return AUTOCORR_IMMEDIATELY;
+		case 0:
+			return AUTOCORR_HINTONLY;
+		default: /* other random text */
+			break;
+	}
+
+	if (!strcmp(value, "prompt"))
+		return AUTOCORR_PROMPT;
+	else if (!strcmp(value, "never"))
+		return AUTOCORR_NEVER;
+	else if (!strcmp(value, "immediate"))
+		return AUTOCORR_IMMEDIATELY;
+	else if (!strcmp(value, "show"))
+		return AUTOCORR_HINTONLY;
+	else
+		return AUTOCORR_DELAY;
+}
+
+static int config_cb(const char *var, const char *value,
+		     const struct config_context *ctx, void *data)
+{
+	struct autocorr *conf = data;
+
+	if (strcmp(var, "help.autocorrect"))
+		return 0;
+
+	conf->mode = parse_autocorr(value);
+
+	/*
+	 * Disable autocorrection prompt in a non-interactive session
+	 */
+	if (conf->mode == AUTOCORR_PROMPT && (!isatty(0) || !isatty(2)))
+		conf->mode = AUTOCORR_NEVER;
+
+	if (conf->mode == AUTOCORR_DELAY) {
+		conf->delay = git_config_int(var, value, ctx->kvi);
+
+		if (conf->delay == 0)
+			conf->mode = AUTOCORR_HINTONLY;
+		else if (conf->delay <= 1)
+			conf->mode = AUTOCORR_IMMEDIATELY;
+	}
+
+	return 0;
+}
+
+void autocorr_resolve(struct autocorr *conf)
+{
+	read_early_config(the_repository, config_cb, conf);
+}
+
+void autocorr_prompt_or_delay(struct autocorr *conf, const char *assumed)
+{
+	if (conf->mode == AUTOCORR_IMMEDIATELY) {
+		fprintf_ln(stderr,
+			   _("Continuing under the assumption that you meant '%s'."),
+			   assumed);
+
+	} else if (conf->mode == AUTOCORR_PROMPT) {
+		char *answer;
+		struct strbuf msg = STRBUF_INIT;
+
+		strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
+		answer = git_prompt(msg.buf, PROMPT_ECHO);
+		strbuf_release(&msg);
+
+		if (!(starts_with(answer, "y") || starts_with(answer, "Y")))
+			exit(1);
+
+	} else if (conf->mode == AUTOCORR_DELAY) {
+		fprintf_ln(stderr,
+			   _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
+			   (float)conf->delay / 10.0, assumed);
+
+		sleep_millisec(conf->delay * 100);
+	}
+}
diff --git a/autocorrect.h b/autocorrect.h
new file mode 100644
index 000000000000..ea21811a43f2
--- /dev/null
+++ b/autocorrect.h
@@ -0,0 +1,23 @@
+#ifndef AUTOCORRECT_H
+#define AUTOCORRECT_H
+
+struct config_context;
+
+enum autocorr_mode {
+	AUTOCORR_HINTONLY,
+	AUTOCORR_NEVER,
+	AUTOCORR_PROMPT,
+	AUTOCORR_IMMEDIATELY,
+	AUTOCORR_DELAY,
+};
+
+struct autocorr {
+	enum autocorr_mode mode;
+	int delay;
+};
+
+void autocorr_resolve(struct autocorr *conf);
+
+void autocorr_prompt_or_delay(struct autocorr *conf, const char *assumed);
+
+#endif /* AUTOCORRECT_H */
diff --git a/help.c b/help.c
index 95f576c5c81d..5eec6a34de33 100644
--- a/help.c
+++ b/help.c
@@ -22,6 +22,7 @@
 #include "repository.h"
 #include "alias.h"
 #include "utf8.h"
+#include "autocorrect.h"
 
 #ifndef NO_CURL
 #include "git-curl-compat.h" /* For LIBCURL_VERSION only */
@@ -536,70 +537,23 @@ int is_in_cmdlist(struct cmdnames *c, const char *s)
 	return 0;
 }
 
-struct help_unknown_cmd_config {
-	int autocorrect;
-	struct cmdnames aliases;
-};
-
-#define AUTOCORRECT_SHOW (-4)
-#define AUTOCORRECT_PROMPT (-3)
-#define AUTOCORRECT_NEVER (-2)
-#define AUTOCORRECT_IMMEDIATELY (-1)
-
-static int parse_autocorrect(const char *value)
+static int resolve_aliases(const char *var, const char *value,
+			   const struct config_context *ctx, void *data)
 {
-	switch (git_parse_maybe_bool_text(value)) {
-		case 1:
-			return AUTOCORRECT_IMMEDIATELY;
-		case 0:
-			return AUTOCORRECT_SHOW;
-		default: /* other random text */
-			break;
-	}
-
-	if (!strcmp(value, "prompt"))
-		return AUTOCORRECT_PROMPT;
-	if (!strcmp(value, "never"))
-		return AUTOCORRECT_NEVER;
-	if (!strcmp(value, "immediate"))
-		return AUTOCORRECT_IMMEDIATELY;
-	if (!strcmp(value, "show"))
-		return AUTOCORRECT_SHOW;
-
-	return 0;
-}
-
-static int git_unknown_cmd_config(const char *var, const char *value,
-				  const struct config_context *ctx,
-				  void *cb)
-{
-	struct help_unknown_cmd_config *cfg = cb;
+	struct cmdnames *aliases = data;
 	const char *subsection, *key;
 	size_t subsection_len;
 
-	if (!strcmp(var, "help.autocorrect")) {
-		int v = parse_autocorrect(value);
-
-		if (!v) {
-			v = git_config_int(var, value, ctx->kvi);
-			if (v < 0 || v == 1)
-				v = AUTOCORRECT_IMMEDIATELY;
-		}
-
-		cfg->autocorrect = v;
-	}
-
-	/* Also use aliases for command lookup */
 	if (!parse_config_key(var, "alias", &subsection, &subsection_len,
 			      &key)) {
 		if (subsection) {
 			/* [alias "name"] command = value */
 			if (!strcmp(key, "command"))
-				add_cmdname(&cfg->aliases, subsection,
+				add_cmdname(aliases, subsection,
 					    subsection_len);
 		} else {
 			/* alias.name = value */
-			add_cmdname(&cfg->aliases, key, strlen(key));
+			add_cmdname(aliases, key, strlen(key));
 		}
 	}
 
@@ -636,28 +590,27 @@ static const char bad_interpreter_advice[] =
 
 char *help_unknown_cmd(const char *cmd)
 {
-	struct help_unknown_cmd_config cfg = { 0 };
+	struct cmdnames aliases = { 0 };
+	struct autocorr autocorr = { 0 };
+
 	int i, n, best_similarity = 0;
 	struct cmdnames main_cmds = { 0 };
 	struct cmdnames other_cmds = { 0 };
 	struct cmdname_help *common_cmds;
 
-	read_early_config(the_repository, git_unknown_cmd_config, &cfg);
+	autocorr_resolve(&autocorr);
 
-	/*
-	 * Disable autocorrection prompt in a non-interactive session
-	 */
-	if ((cfg.autocorrect == AUTOCORRECT_PROMPT) && (!isatty(0) || !isatty(2)))
-		cfg.autocorrect = AUTOCORRECT_NEVER;
+	/* Also use aliases for command lookup */
+	read_early_config(the_repository, resolve_aliases, &aliases);
 
-	if (cfg.autocorrect == AUTOCORRECT_NEVER) {
+	if (autocorr.mode == AUTOCORR_NEVER) {
 		fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
 		exit(1);
 	}
 
 	load_command_list("git-", &main_cmds, &other_cmds);
 
-	add_cmd_list(&main_cmds, &cfg.aliases);
+	add_cmd_list(&main_cmds, &aliases);
 	add_cmd_list(&main_cmds, &other_cmds);
 	QSORT(main_cmds.names, main_cmds.cnt, cmdname_compare);
 	uniq(&main_cmds);
@@ -716,37 +669,18 @@ char *help_unknown_cmd(const char *cmd)
 		     n++)
 			; /* still counting */
 	}
-	if (cfg.autocorrect && cfg.autocorrect != AUTOCORRECT_SHOW && n == 1 &&
+
+	if (autocorr.mode != AUTOCORR_HINTONLY && n == 1 &&
 	    SIMILAR_ENOUGH(best_similarity)) {
 		char *assumed = xstrdup(main_cmds.names[0]->name);
 
 		fprintf_ln(stderr,
-			   _("WARNING: You called a Git command named '%s', "
-			     "which does not exist."),
+			   _("WARNING: You called a Git command named '%s', which does not exist."),
 			   cmd);
-		if (cfg.autocorrect == AUTOCORRECT_IMMEDIATELY)
-			fprintf_ln(stderr,
-				   _("Continuing under the assumption that "
-				     "you meant '%s'."),
-				   assumed);
-		else if (cfg.autocorrect == AUTOCORRECT_PROMPT) {
-			char *answer;
-			struct strbuf msg = STRBUF_INIT;
-			strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
-			answer = git_prompt(msg.buf, PROMPT_ECHO);
-			strbuf_release(&msg);
-			if (!(starts_with(answer, "y") ||
-			      starts_with(answer, "Y")))
-				exit(1);
-		} else {
-			fprintf_ln(stderr,
-				   _("Continuing in %0.1f seconds, "
-				     "assuming that you meant '%s'."),
-				   (float)cfg.autocorrect/10.0, assumed);
-			sleep_millisec(cfg.autocorrect * 100);
-		}
 
-		cmdnames_release(&cfg.aliases);
+		autocorr_prompt_or_delay(&autocorr, assumed);
+
+		cmdnames_release(&aliases);
 		cmdnames_release(&main_cmds);
 		cmdnames_release(&other_cmds);
 		return assumed;
-- 
2.53.0

[PATCH v2 3/5] parseopt: autocorrect mistyped subcommands

[Jiamu Sun]

Enable autocorrection for mandatory subcommands parsed through
parse-options.

AUTOCORR_HINTONLY is equivalent to AUTOCORR_NEVER here, because builtins
have a limited number of subcommands. They are currently small enough.
Therefore, displaying all subcommands via usage_with_options() is
good enough. This keeps the autocorrection handling simple, too.

Also, use a dynamic threshold for similar_enough(), which can yield more
accurate typo correction results. Although subcommands are often short,
they can still vary across builtins. And a fixed threshold won't do
better on both short and long subcommands at the same time.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
Changes in v2:
  - Minor rewording in the commit message regarding AUTOCORR_HINTONLY.

 parse-options.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 6bb0c5697099..cbbb04b9997a 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -6,6 +6,8 @@
 #include "strbuf.h"
 #include "string-list.h"
 #include "utf8.h"
+#include "autocorrect.h"
+#include "levenshtein.h"
 
 static int disallow_abbreviated_options;
 
@@ -621,6 +623,64 @@ static int parse_subcommand(const char *arg, const struct option *options)
 	return -1;
 }
 
+static void find_subcommands(struct string_list *list,
+			     const struct option *options)
+{
+	for (; options->type != OPTION_END; options++) {
+		if (options->type == OPTION_SUBCOMMAND)
+			string_list_append(list, options->long_name);
+	}
+}
+
+static int similar_enough(const char *cmd, unsigned int dist)
+{
+	size_t len = strlen(cmd);
+	unsigned int threshold = len < 3 ? 1 : len < 6 ? 3 : 6;
+
+	return dist < threshold;
+}
+
+static const char *autocorrect_subcmd(const char *cmd,
+				      struct string_list *cmds)
+{
+	struct autocorr autocorr = { 0 };
+
+	autocorr_resolve(&autocorr);
+
+	if (autocorr.mode == AUTOCORR_NEVER ||
+	    autocorr.mode == AUTOCORR_HINTONLY)
+		return NULL;
+
+	unsigned int min = -1;
+	unsigned int ties = 0;
+	struct string_list_item *cand;
+	struct string_list_item *best = NULL;
+
+	for_each_string_list_item(cand, cmds) {
+		unsigned int dist = levenshtein(cmd, cand->string, 0, 2, 1, 3);
+
+		if (dist < min) {
+			min = dist;
+			best = cand;
+			ties = 0;
+
+		} else if (dist == min) {
+			ties++;
+		}
+	}
+
+	if (ties == 0 && similar_enough(cmd, min)) {
+		fprintf_ln(stderr,
+			   _("WARNING: You called a subcommand named '%s', which does not exist."),
+			   cmd);
+
+		autocorr_prompt_or_delay(&autocorr, best->string);
+		return best->string;
+	}
+
+	return NULL;
+}
+
 static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 					       const char *arg,
 					       const struct option *options,
@@ -640,8 +700,20 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
 		return PARSE_OPT_DONE;
 
-	error(_("unknown subcommand: `%s'"), arg);
-	usage_with_options(usagestr, options);
+	struct string_list cmds = STRING_LIST_INIT_NODUP;
+
+	find_subcommands(&cmds, options);
+
+	const char *cmd = autocorrect_subcmd(arg, &cmds);
+
+	if (!cmd) {
+		error(_("unknown subcommand: `%s'"), arg);
+		usage_with_options(usagestr, options);
+	}
+
+	string_list_clear(&cmds, 0);
+	parse_subcommand(cmd, options);
+	return PARSE_OPT_SUBCOMMAND;
 }
 
 static void check_typos(const char *arg, const struct option *options)
-- 
2.53.0

[PATCH v2 4/5] parseopt: enable subcommand autocorrect for remote and notes

[Jiamu Sun]

Add PARSE_OPT_SUBCOMMAND_AUTOCORR to enable autocorrection for
subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL.

Use it for git-remote and git-notes, so mistyped subcommands can be
automatically corrected, and builtin entry points no longer need
to handle missing subcommand error path themselves.

This is safe for these two builtins, because they either resolve to a
single subcommand or take no subcommand at all. This means that if the
subcommand parser encounters an unknown argument, it must be a mistyped
subcommand.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 builtin/notes.c  | 10 +++-------
 builtin/remote.c | 12 ++++--------
 parse-options.c  | 16 +++++++++-------
 parse-options.h  |  1 +
 4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 9af602bdd7b4..087eb898a441 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -1149,14 +1149,10 @@ int cmd_notes(int argc,
 
 	repo_config(the_repository, git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, git_notes_usage,
-			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
-	if (!fn) {
-		if (argc) {
-			error(_("unknown subcommand: `%s'"), argv[0]);
-			usage_with_options(git_notes_usage, options);
-		}
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL |
+			     PARSE_OPT_SUBCOMMAND_AUTOCORR);
+	if (!fn)
 		fn = list;
-	}
 
 	if (override_notes_ref) {
 		struct strbuf sb = STRBUF_INIT;
diff --git a/builtin/remote.c b/builtin/remote.c
index ace390c671d6..d1d6244a662a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1949,15 +1949,11 @@ int cmd_remote(int argc,
 	};
 
 	argc = parse_options(argc, argv, prefix, options, builtin_remote_usage,
-			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL |
+			     PARSE_OPT_SUBCOMMAND_AUTOCORR);
 
-	if (fn) {
+	if (fn)
 		return !!fn(argc, argv, prefix, repo);
-	} else {
-		if (argc) {
-			error(_("unknown subcommand: `%s'"), argv[0]);
-			usage_with_options(builtin_remote_usage, options);
-		}
+	else
 		return !!show_all();
-	}
 }
diff --git a/parse-options.c b/parse-options.c
index cbbb04b9997a..b7c818e818fe 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -691,14 +691,16 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 	if (!err)
 		return PARSE_OPT_SUBCOMMAND;
 
-	/*
-	 * arg is neither a short or long option nor a subcommand.  Since this
-	 * command has a default operation mode, we have to treat this arg and
-	 * all remaining args as args meant to that default operation mode.
-	 * So we are done parsing.
-	 */
-	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
+	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL &&
+	    !(ctx->flags & PARSE_OPT_SUBCOMMAND_AUTOCORR)) {
+		/*
+		 * arg is neither a short or long option nor a subcommand.
+		 * Since this command has a default operation mode, we have to
+		 * treat this arg and all remaining args as args meant to that
+		 * default operation mode.  So we are done parsing.
+		 */
 		return PARSE_OPT_DONE;
+	}
 
 	struct string_list cmds = STRING_LIST_INIT_NODUP;
 
diff --git a/parse-options.h b/parse-options.h
index 706de9729f6b..f29ac337893c 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -40,6 +40,7 @@ enum parse_opt_flags {
 	PARSE_OPT_ONE_SHOT = 1 << 5,
 	PARSE_OPT_SHELL_EVAL = 1 << 6,
 	PARSE_OPT_SUBCOMMAND_OPTIONAL = 1 << 7,
+	PARSE_OPT_SUBCOMMAND_AUTOCORR = 1 << 8,
 };
 
 enum parse_opt_option_flags {
-- 
2.53.0

[PATCH v2 5/5] help: add tests for subcommand autocorrection

[Jiamu Sun]

These tests cover default behavior (help.autocorrect is unset), no
correction, immediate correction, delayed correction, and rejection
when the typo is too dissimilar.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 t/t9004-autocorrect-subcommand.sh | 49 +++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100755 t/t9004-autocorrect-subcommand.sh

diff --git a/t/t9004-autocorrect-subcommand.sh b/t/t9004-autocorrect-subcommand.sh
new file mode 100755
index 000000000000..760760c8851a
--- /dev/null
+++ b/t/t9004-autocorrect-subcommand.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='subcommand auto-correction test
+
+Test autocorrection for subcommands with different
+help.autocorrect mode.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' "
+	echo '^error: unknown subcommand: ' >grep_unknown
+"
+
+test_expect_success 'default is not to autocorrect' '
+	test_must_fail git worktree lsit 2>actual &&
+	head -n1 actual >first && test_grep -f grep_unknown first
+'
+
+for mode in false no off 0 show never; do
+	test_expect_success "'$mode' disables autocorrection" "
+		test_config help.autocorrect $mode &&
+
+		test_must_fail git worktree lsit 2>actual &&
+		head -n1 actual >first && test_grep -f grep_unknown first
+	"
+done
+
+for mode in -39 immediate 1; do
+	test_expect_success "autocorrect immediately with '$mode'" - <<-EOT
+		test_config help.autocorrect $mode &&
+
+		git worktree lsit 2>actual &&
+		test_grep "you meant 'list'\.$" actual
+	EOT
+done
+
+test_expect_success 'delay path is executed' - <<-\EOT
+	test_config help.autocorrect 2 &&
+
+	git worktree lsit 2>actual &&
+	test_grep '^Continuing in 0.2 seconds, ' actual
+EOT
+
+test_expect_success 'deny if too dissimilar' - <<-\EOT
+	test_must_fail git remote rensnr 2>actual &&
+	head -n1 actual >first && test_grep -f grep_unknown first
+EOT
+
+test_done
-- 
2.53.0

Re: [PATCH 1/5] parseopt: extract subcommand handling from parse_options_step()

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> -static enum parse_opt_result parse_subcommand(const char *arg,
> -					      const struct option *options)

So, this function used to return either PARSE_OPT_SUBCOMMAND or
PARSE_OPT_UNKNOWN, and the caller (in an "switch ()" statement in
the parse_options_step() we see below) expected only these two.  We
now instead ...

> +static int parse_subcommand(const char *arg, const struct option *options)

... make it return either 0 (success - we found a subcommand) or -1
(failure - we did not).  The updated caller does use the return
value in "if ()" condition to return early when we found a
subcommand successfully, which is a lot more straight-forward than
the original "switch()".  The original is even worse in that it
enumerates other PARSE_OPT_* values that are possible at the time of
writing it, instead of catching everything else with "default:",
which makes the switch() statement a maintenance burden.  Getting
rid of that switch and clarifying that there are only two possible
outcome from this function alone is a good enough justification to
have this clean-up patch.  Very well done.


Editing the hunk to show only the postimage reveals that the new
implementation has CodingGuidelines violation ...

>  {
> +	for (; options->type != OPTION_END; options++) {
> +		if (options->type != OPTION_SUBCOMMAND ||
> +		    strcmp(options->long_name, arg))
> +			continue;
>  
> +		parse_opt_subcommand_fn **opt_val = options->value;

... here.  Move the variable definition up in the block introduced
by the for(;;) statement, perhaps?

> +		*opt_val = options->subcommand_fn;
> +
> +		return 0;
> +	}
> +
> +	return -1;
> +}
> +


And the part of the caller of parse_subcommand() that used to be the
switch() statement in the parse_options_step() is now extracted into
yet another helper function here.

> +static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
> +					       const char *arg,
> +					       const struct option *options,
> +					       const char * const usagestr[])
> +{
> +	int err = parse_subcommand(arg, options);
> +
> +	if (!err)
> +		return PARSE_OPT_SUBCOMMAND;
> +
> +	/*
> +	 * arg is neither a short or long option nor a subcommand.  Since this
> +	 * command has a default operation mode, we have to treat this arg and
> +	 * all remaining args as args meant to that default operation mode.
> +	 * So we are done parsing.
> +	 */

Thanks to being a small helper function, we lost two levels of
indentation which made this comment a lot more readable with
reflowing the lines ;-)

> +	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
> +		return PARSE_OPT_DONE;
> +
> +	error(_("unknown subcommand: `%s'"), arg);
> +	usage_with_options(usagestr, options);
>  }

It is clear that there is no unintended behaviour change around this
helper function and parse_subcommand().  Very nice.

> @@ -990,37 +1016,16 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
>  		if (*arg != '-' || !arg[1]) {
>  			if (parse_nodash_opt(ctx, arg, options) == 0)
>  				continue;
> +
>  			if (!ctx->has_subcommands) {
>  				if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
>  					return PARSE_OPT_NON_OPTION;
>  				ctx->out[ctx->cpidx++] = ctx->argv[0];
>  				continue;
> +
> +			} else {
> +				return handle_subcommand(ctx, arg,
> +							 options, usagestr);
>  			}
>  		}

Editing the hunk to only show the postimage shows us that the blank
line at the end of the "if ()" block is funny.  Drop it.  Or even
better, as the block either returns or continues anyway, lose the
"else" block, perhaps?  Which will make the above read like so:

			if (!ctx->has_subcommands) {
				if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
                                	return PARSE_OPT_NON_OPTION;
				ctx->out[ctx->cpidx++] = ctx->argv[0];
				continue;
			}

                        return has_subcommands(ctx, arg, options, usagestr);

or swapping the logic the other way around, i.e.,

			if (ctx->has_subcommands)
				return has_subcommands(ctx, arg,
						       options, usagestr);

                        if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
                                return PARSE_OPT_NON_OPTION;

                        ctx->out[ctx->cpidx++] = ctx->argv[0];
                        continue;

may make the result even easier to read, perhaps?

Re: [PATCH 2/5] help: refactor command autocorrection handling

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> Refactor autocorrect config into struct autocorr, with an explicit mode
> enum and a separate delay field. Move config parsing, TTY validation,
> and prompt handling into autocorrect.c.
>
> This simplifies help.c and makes state handling for autocorrect explicit
> and easier to maintain.
>
> Signed-off-by: Jiamu Sun <39@barroit.sh>
> ---
>  Makefile      |   1 +
>  autocorrect.c |  92 +++++++++++++++++++++++++++++++++++++++++++
>  autocorrect.h |  23 +++++++++++
>  help.c        | 106 ++++++++++----------------------------------------
>  4 files changed, 136 insertions(+), 86 deletions(-)
>  create mode 100644 autocorrect.c
>  create mode 100644 autocorrect.h
>
> diff --git a/Makefile b/Makefile
> index f3264d0a37cc..6111631c2caa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1098,6 +1098,7 @@ LIB_OBJS += archive-tar.o
>  LIB_OBJS += archive-zip.o
>  LIB_OBJS += archive.o
>  LIB_OBJS += attr.o
> +LIB_OBJS += autocorrect.o
>  LIB_OBJS += base85.o
>  LIB_OBJS += bisect.o
>  LIB_OBJS += blame.o
> diff --git a/autocorrect.c b/autocorrect.c
> new file mode 100644
> index 000000000000..eaae01645910
> --- /dev/null
> +++ b/autocorrect.c
> @@ -0,0 +1,92 @@
> +#define USE_THE_REPOSITORY_VARIABLE
> +
> +#include "git-compat-util.h"
> +#include "autocorrect.h"
> +#include "config.h"
> +#include "parse.h"
> +#include "strbuf.h"
> +#include "prompt.h"
> +#include "gettext.h"
> +
> +static enum autocorr_mode parse_autocorr(const char *value)
> +{
> +	switch (git_parse_maybe_bool_text(value)) {
> +		case 1:
> +			return AUTOCORR_IMMEDIATELY;
> +		case 0:
> +			return AUTOCORR_HINTONLY;
> +		default: /* other random text */
> +			break;
> +	}
> +
> +	if (!strcmp(value, "prompt"))
> +		return AUTOCORR_PROMPT;
> +	else if (!strcmp(value, "never"))
> +		return AUTOCORR_NEVER;
> +	else if (!strcmp(value, "immediate"))
> +		return AUTOCORR_IMMEDIATELY;
> +	else if (!strcmp(value, "show"))
> +		return AUTOCORR_HINTONLY;
> +	else
> +		return AUTOCORR_DELAY;
> +}

You snuck in unnecessary "style fixes" to make bunch of "if()return"
into if/else if cascade.  Also what was AUTOCORRECT_SHOW is now
returned as AUTOCORR_HINTONLY.  There is no explanation on the
reason behind these changes in the proposed log message, and hiding
these small changes in a code movement patch makes reviewing the
series harder than necessary.

The patch is doing too many things (namely, (1) code movement that
will make it later reusable as a side effect but has no semantic
changes in the current code, plus (2) change in style (like the one
we see here), semantics (possibly the difference in SHOW and
HINTONLY we see here) and features, possibly including the renaming
of AUTOCORRECT_* into AUTOCORR_*.)  Let's have "restructure with
code movement and nothing else", followed by "other changes

I'll stop here, and expect this step to be split into at least two
patches to make it more readable before we can review it again.

Thanks.

Re: [PATCH 3/5] parseopt: autocorrect mistyped subcommands

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> +static const char *autocorrect_subcmd(const char *cmd,
> +				      struct string_list *cmds)
> +{
> +	struct autocorr autocorr = { 0 };
> +
> +	autocorr_resolve(&autocorr);
> +
> +	if (autocorr.mode == AUTOCORR_NEVER ||
> +	    autocorr.mode == AUTOCORR_HINTONLY)
> +		return NULL;
> +
> +	unsigned int min = -1;
> +	unsigned int ties = 0;
> +	struct string_list_item *cand;
> +	struct string_list_item *best = NULL;

CodingGuidelines: -Wdeclaration-after-statement is here.

Using "-1" as "the maximum value this unsigned type can represent"
is somewhat misleading.  Can't we avoid that?

Re: [PATCH 1/5] parseopt: extract subcommand handling from parse_options_step()

[Jiamu Sun]

On Sun, Mar 08, 2026 at 04:40:44PM -0700, Junio C Hamano wrote:
> ... here.  Move the variable definition up in the block introduced
> by the for(;;) statement, perhaps?
> 
> > +		*opt_val = options->subcommand_fn;
> > +
> > +		return 0;
> > +	}

Agree. Actually, I forgot to check the CodingGuidelines.  Will fix the
coding style issue in v3.

> Editing the hunk to only show the postimage shows us that the blank
> line at the end of the "if ()" block is funny.  Drop it.  Or even
> better, as the block either returns or continues anyway, lose the
> "else" block, perhaps?  Which will make the above read like so:
> 
> 			if (!ctx->has_subcommands) {
> 				if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
>                                 	return PARSE_OPT_NON_OPTION;
> 				ctx->out[ctx->cpidx++] = ctx->argv[0];
> 				continue;
> 			}
> 
>                         return has_subcommands(ctx, arg, options, usagestr);
> 
> or swapping the logic the other way around, i.e.,
> 
> 			if (ctx->has_subcommands)
> 				return has_subcommands(ctx, arg,
> 						       options, usagestr);
> 
>                         if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
>                                 return PARSE_OPT_NON_OPTION;
> 
>                         ctx->out[ctx->cpidx++] = ctx->argv[0];
>                         continue;
> 
> may make the result even easier to read, perhaps?

Swapping the logic works better here. Returning early in the
ctx->has_subcommands path lets the rest of the block assume
!ctx->has_subcommands, so the extra check can go away. That makes
the code easier to read. Will do that.

-- 
Jiamu Sun <39@barroit.sh>

Re: [PATCH 2/5] help: refactor command autocorrection handling

[Jiamu Sun]

On Sun, Mar 08, 2026 at 04:52:54PM -0700, Junio C Hamano wrote:
> You snuck in unnecessary "style fixes" to make bunch of "if()return"
> into if/else if cascade.  Also what was AUTOCORRECT_SHOW is now
> returned as AUTOCORR_HINTONLY.  There is no explanation on the
> reason behind these changes in the proposed log message, and hiding
> these small changes in a code movement patch makes reviewing the
> series harder than necessary.
> 
> The patch is doing too many things (namely, (1) code movement that
> will make it later reusable as a side effect but has no semantic
> changes in the current code, plus (2) change in style (like the one
> we see here), semantics (possibly the difference in SHOW and
> HINTONLY we see here) and features, possibly including the renaming
> of AUTOCORRECT_* into AUTOCORR_*.)  Let's have "restructure with
> code movement and nothing else", followed by "other changes
> 
> I'll stop here, and expect this step to be split into at least two
> patches to make it more readable before we can review it again.
> 
> Thanks.

Ah, I realized this patch is completely unreadable. You are right, the
code movement ended up hiding other changes entirely. Will split it to
make each change obvious.

-- 
Jiamu Sun <39@barroit.sh>

Re: [PATCH 3/5] parseopt: autocorrect mistyped subcommands

[Jiamu Sun]

On Sun, Mar 08, 2026 at 05:04:49PM -0700, Junio C Hamano wrote:
> > +	unsigned int min = -1;
> > +	unsigned int ties = 0;
> > +	struct string_list_item *cand;
> > +	struct string_list_item *best = NULL;
> 
> CodingGuidelines: -Wdeclaration-after-statement is here.

Will fix that to follow CodingGuidelines.

> Using "-1" as "the maximum value this unsigned type can represent"
> is somewhat misleading.  Can't we avoid that?
>

Yes, UINT_MAX is a better fit here.

-- 
Jiamu Sun <39@barroit.sh>

[PATCH v3 0/8] parseopt: add subcommand autocorrection

[Jiamu Sun]

Git currently provides auto-correction for builtins and aliases, but
lacks this functionality for subcommands parsed via the parse-options
API. Subcommands are also commands, and typos will occur, too. Like:

	git remote add-rul

So, this series introduces subcommand auto-correction.

By default, this implementation enables autocorrection for builtins
with mandatory subcommands. However, for those using
PARSE_OPT_SUBCOMMAND_OPTIONAL, autocorrection is skipped to avoid
misinterpreting legitimate unknown arguments as mistyped subcommands.

To allow builtins with optional subcommands to explicitly opt in,
this series adds the PARSE_OPT_SUBCOMMAND_AUTOCORR flag. This flag
is subsequently enabled for git-remote and git-notes.

Additionally, the existing autocorrection logic is extracted from
help.c so subcommand handling can reuse the same config parsing and
prompt/delay logic.

Some split string literals are also combined so the full text is easier
to grep for.

Changes in v3:
  - Align with the coding guildline

Changes in v2:
  - Reword the explanation of default autocorrection behavior

Jiamu Sun (8):
  parseopt: extract subcommand handling from parse_options_step()
  help: make autocorrect handling reusable
  help: move tty check for autocorrection to autocorrect.c
  autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINTONLY
  autocorrect: provide config resolution API
  parseopt: autocorrect mistyped subcommands
  parseopt: enable subcommand autocorrection for git-remote and
    git-notes
  help: add tests for subcommand autocorrection

 Makefile                          |   1 +
 autocorrect.c                     |  89 +++++++++++++++++
 autocorrect.h                     |  21 ++++
 builtin/notes.c                   |  10 +-
 builtin/remote.c                  |  12 +--
 help.c                            | 104 ++++----------------
 parse-options.c                   | 156 ++++++++++++++++++++++--------
 parse-options.h                   |   1 +
 t/t9004-autocorrect-subcommand.sh |  51 ++++++++++
 9 files changed, 305 insertions(+), 140 deletions(-)
 create mode 100644 autocorrect.c
 create mode 100644 autocorrect.h
 create mode 100755 t/t9004-autocorrect-subcommand.sh


base-commit: 795c338de725e13bd361214c6b768019fc45a2c1
-- 
2.53.0

[PATCH v3 1/8] parseopt: extract subcommand handling from parse_options_step()

[Jiamu Sun]

Move the subcommand branch out of parse_options_step() into a new
handle_subcommand() helper. Also, make parse_subcommand() return a
simple success/failure status.

This removes the switch over impossible parse_opt_result values and
makes the non-option path easier to follow and maintain.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
Changes in v3:
  - Fix coding style issue

 parse-options.c | 86 ++++++++++++++++++++++++++-----------------------
 1 file changed, 46 insertions(+), 40 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index c9cafc21b903..33f26d6b6179 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -605,17 +605,44 @@ static enum parse_opt_result parse_nodash_opt(struct parse_opt_ctx_t *p,
 	return PARSE_OPT_ERROR;
 }
 
-static enum parse_opt_result parse_subcommand(const char *arg,
-					      const struct option *options)
+static int parse_subcommand(const char *arg, const struct option *options)
 {
-	for (; options->type != OPTION_END; options++)
-		if (options->type == OPTION_SUBCOMMAND &&
-		    !strcmp(options->long_name, arg)) {
-			*(parse_opt_subcommand_fn **)options->value = options->subcommand_fn;
-			return PARSE_OPT_SUBCOMMAND;
-		}
+	for (; options->type != OPTION_END; options++) {
+		parse_opt_subcommand_fn **opt_val;
 
-	return PARSE_OPT_UNKNOWN;
+		if (options->type != OPTION_SUBCOMMAND ||
+		    strcmp(options->long_name, arg))
+			continue;
+
+		opt_val = options->value;
+		*opt_val = options->subcommand_fn;
+		return 0;
+	}
+
+	return -1;
+}
+
+static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
+					       const char *arg,
+					       const struct option *options,
+					       const char * const usagestr[])
+{
+	int err = parse_subcommand(arg, options);
+
+	if (!err)
+		return PARSE_OPT_SUBCOMMAND;
+
+	/*
+	 * arg is neither a short or long option nor a subcommand.  Since this
+	 * command has a default operation mode, we have to treat this arg and
+	 * all remaining args as args meant to that default operation mode.
+	 * So we are done parsing.
+	 */
+	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
+		return PARSE_OPT_DONE;
+
+	error(_("unknown subcommand: `%s'"), arg);
+	usage_with_options(usagestr, options);
 }
 
 static void check_typos(const char *arg, const struct option *options)
@@ -990,38 +1017,17 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
 		if (*arg != '-' || !arg[1]) {
 			if (parse_nodash_opt(ctx, arg, options) == 0)
 				continue;
-			if (!ctx->has_subcommands) {
-				if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
-					return PARSE_OPT_NON_OPTION;
-				ctx->out[ctx->cpidx++] = ctx->argv[0];
-				continue;
-			}
-			switch (parse_subcommand(arg, options)) {
-			case PARSE_OPT_SUBCOMMAND:
-				return PARSE_OPT_SUBCOMMAND;
-			case PARSE_OPT_UNKNOWN:
-				if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
-					/*
-					 * arg is neither a short or long
-					 * option nor a subcommand.  Since
-					 * this command has a default
-					 * operation mode, we have to treat
-					 * this arg and all remaining args
-					 * as args meant to that default
-					 * operation mode.
-					 * So we are done parsing.
-					 */
-					return PARSE_OPT_DONE;
-				error(_("unknown subcommand: `%s'"), arg);
-				usage_with_options(usagestr, options);
-			case PARSE_OPT_COMPLETE:
-			case PARSE_OPT_HELP:
-			case PARSE_OPT_ERROR:
-			case PARSE_OPT_DONE:
-			case PARSE_OPT_NON_OPTION:
-				/* Impossible. */
-				BUG("parse_subcommand() cannot return these");
+
+			if (ctx->has_subcommands) {
+				return handle_subcommand(ctx, arg, options,
+							 usagestr);
 			}
+
+			if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
+				return PARSE_OPT_NON_OPTION;
+
+			ctx->out[ctx->cpidx++] = ctx->argv[0];
+			continue;
 		}
 
 		/* lone -h asks for help */
-- 
2.53.0

[PATCH v3 2/8] help: make autocorrect handling reusable

[Jiamu Sun]

Move config parsing and prompt/delay handling into autocorrect.c and
expose them in autocorrect.h. This makes autocorrect reusable regardless
of which target links against it.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
This patch moves autocorrect handling out of help.c as a preparatory
step for subcommand autocorrection.

This is necessary because help.c uses load_builtin_commands(), which is
provided by git.o. Executables that do not link against git.o, such as
git-daemon, will hit a link error if they use symbols defined in help.o.
A simple and clean fix is to make the relevant functions independent of
help.c and move them to a dedicated file.

Changes in v3:
  - Split patch so diffs don't get hidden by code movement

 Makefile      |  1 +
 autocorrect.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
 autocorrect.h | 16 ++++++++++++
 help.c        | 64 +++------------------------------------------
 4 files changed, 93 insertions(+), 60 deletions(-)
 create mode 100644 autocorrect.c
 create mode 100644 autocorrect.h

diff --git a/Makefile b/Makefile
index f3264d0a37cc..6111631c2caa 100644
--- a/Makefile
+++ b/Makefile
@@ -1098,6 +1098,7 @@ LIB_OBJS += archive-tar.o
 LIB_OBJS += archive-zip.o
 LIB_OBJS += archive.o
 LIB_OBJS += attr.o
+LIB_OBJS += autocorrect.o
 LIB_OBJS += base85.o
 LIB_OBJS += bisect.o
 LIB_OBJS += blame.o
diff --git a/autocorrect.c b/autocorrect.c
new file mode 100644
index 000000000000..1037f032019b
--- /dev/null
+++ b/autocorrect.c
@@ -0,0 +1,72 @@
+#include "git-compat-util.h"
+#include "autocorrect.h"
+#include "config.h"
+#include "parse.h"
+#include "strbuf.h"
+#include "prompt.h"
+#include "gettext.h"
+
+static int parse_autocorrect(const char *value)
+{
+	switch (git_parse_maybe_bool_text(value)) {
+		case 1:
+			return AUTOCORRECT_IMMEDIATELY;
+		case 0:
+			return AUTOCORRECT_SHOW;
+		default: /* other random text */
+			break;
+	}
+
+	if (!strcmp(value, "prompt"))
+		return AUTOCORRECT_PROMPT;
+	if (!strcmp(value, "never"))
+		return AUTOCORRECT_NEVER;
+	if (!strcmp(value, "immediate"))
+		return AUTOCORRECT_IMMEDIATELY;
+	if (!strcmp(value, "show"))
+		return AUTOCORRECT_SHOW;
+
+	return 0;
+}
+
+void autocorr_resolve_config(const char *var, const char *value,
+			     const struct config_context *ctx, void *data)
+{
+	int *out = data;
+
+	if (!strcmp(var, "help.autocorrect")) {
+		int v = parse_autocorrect(value);
+
+		if (!v) {
+			v = git_config_int(var, value, ctx->kvi);
+			if (v < 0 || v == 1)
+				v = AUTOCORRECT_IMMEDIATELY;
+		}
+
+		*out = v;
+	}
+}
+
+void autocorr_confirm(int autocorrect, const char *assumed)
+{
+	if (autocorrect == AUTOCORRECT_IMMEDIATELY) {
+		fprintf_ln(stderr,
+			   _("Continuing under the assumption that you meant '%s'."),
+			   assumed);
+	} else if (autocorrect == AUTOCORRECT_PROMPT) {
+		char *answer;
+		struct strbuf msg = STRBUF_INIT;
+
+		strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
+		answer = git_prompt(msg.buf, PROMPT_ECHO);
+		strbuf_release(&msg);
+
+		if (!(starts_with(answer, "y") || starts_with(answer, "Y")))
+			exit(1);
+	} else {
+		fprintf_ln(stderr,
+			   _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
+			   (float)autocorrect / 10.0, assumed);
+		sleep_millisec(autocorrect * 100);
+	}
+}
diff --git a/autocorrect.h b/autocorrect.h
new file mode 100644
index 000000000000..45609990c77e
--- /dev/null
+++ b/autocorrect.h
@@ -0,0 +1,16 @@
+#ifndef AUTOCORRECT_H
+#define AUTOCORRECT_H
+
+#define AUTOCORRECT_SHOW (-4)
+#define AUTOCORRECT_PROMPT (-3)
+#define AUTOCORRECT_NEVER (-2)
+#define AUTOCORRECT_IMMEDIATELY (-1)
+
+struct config_context;
+
+void autocorr_resolve_config(const char *var, const char *value,
+			     const struct config_context *ctx, void *data);
+
+void autocorr_confirm(int autocorr, const char *assumed);
+
+#endif /* AUTOCORRECT_H */
diff --git a/help.c b/help.c
index 95f576c5c81d..6be3ec9dfb12 100644
--- a/help.c
+++ b/help.c
@@ -22,6 +22,7 @@
 #include "repository.h"
 #include "alias.h"
 #include "utf8.h"
+#include "autocorrect.h"
 
 #ifndef NO_CURL
 #include "git-curl-compat.h" /* For LIBCURL_VERSION only */
@@ -541,34 +542,6 @@ struct help_unknown_cmd_config {
 	struct cmdnames aliases;
 };
 
-#define AUTOCORRECT_SHOW (-4)
-#define AUTOCORRECT_PROMPT (-3)
-#define AUTOCORRECT_NEVER (-2)
-#define AUTOCORRECT_IMMEDIATELY (-1)
-
-static int parse_autocorrect(const char *value)
-{
-	switch (git_parse_maybe_bool_text(value)) {
-		case 1:
-			return AUTOCORRECT_IMMEDIATELY;
-		case 0:
-			return AUTOCORRECT_SHOW;
-		default: /* other random text */
-			break;
-	}
-
-	if (!strcmp(value, "prompt"))
-		return AUTOCORRECT_PROMPT;
-	if (!strcmp(value, "never"))
-		return AUTOCORRECT_NEVER;
-	if (!strcmp(value, "immediate"))
-		return AUTOCORRECT_IMMEDIATELY;
-	if (!strcmp(value, "show"))
-		return AUTOCORRECT_SHOW;
-
-	return 0;
-}
-
 static int git_unknown_cmd_config(const char *var, const char *value,
 				  const struct config_context *ctx,
 				  void *cb)
@@ -577,17 +550,7 @@ static int git_unknown_cmd_config(const char *var, const char *value,
 	const char *subsection, *key;
 	size_t subsection_len;
 
-	if (!strcmp(var, "help.autocorrect")) {
-		int v = parse_autocorrect(value);
-
-		if (!v) {
-			v = git_config_int(var, value, ctx->kvi);
-			if (v < 0 || v == 1)
-				v = AUTOCORRECT_IMMEDIATELY;
-		}
-
-		cfg->autocorrect = v;
-	}
+	autocorr_resolve_config(var, value, ctx, &cfg->autocorrect);
 
 	/* Also use aliases for command lookup */
 	if (!parse_config_key(var, "alias", &subsection, &subsection_len,
@@ -724,27 +687,8 @@ char *help_unknown_cmd(const char *cmd)
 			   _("WARNING: You called a Git command named '%s', "
 			     "which does not exist."),
 			   cmd);
-		if (cfg.autocorrect == AUTOCORRECT_IMMEDIATELY)
-			fprintf_ln(stderr,
-				   _("Continuing under the assumption that "
-				     "you meant '%s'."),
-				   assumed);
-		else if (cfg.autocorrect == AUTOCORRECT_PROMPT) {
-			char *answer;
-			struct strbuf msg = STRBUF_INIT;
-			strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
-			answer = git_prompt(msg.buf, PROMPT_ECHO);
-			strbuf_release(&msg);
-			if (!(starts_with(answer, "y") ||
-			      starts_with(answer, "Y")))
-				exit(1);
-		} else {
-			fprintf_ln(stderr,
-				   _("Continuing in %0.1f seconds, "
-				     "assuming that you meant '%s'."),
-				   (float)cfg.autocorrect/10.0, assumed);
-			sleep_millisec(cfg.autocorrect * 100);
-		}
+
+		autocorr_confirm(cfg.autocorrect, assumed);
 
 		cmdnames_release(&cfg.aliases);
 		cmdnames_release(&main_cmds);
-- 
2.53.0

[PATCH v3 3/8] help: move tty check for autocorrection to autocorrect.c

[Jiamu Sun]

TTY checking is the autocorrect config parser's responsibility. It must
ensure the parsed value is correct and reliable. Thus, move the check to
autocorr_resolve_config().

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
In parse_autocorrect() I kept the if/else if cascade. In my opinion,
this is not a style change. It's a control-flow clarification, because
those returns describe different states of the mode. Chaining them
together is better than leaving them discrete.

Also, I dropped the float cast in the delay calculation, which is
redundant.

 autocorrect.c | 50 +++++++++++++++++++++++++++++---------------------
 autocorrect.h | 20 ++++++++++++++------
 help.c        | 17 ++++++-----------
 3 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/autocorrect.c b/autocorrect.c
index 1037f032019b..50d7f116d85d 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -6,7 +6,7 @@
 #include "prompt.h"
 #include "gettext.h"
 
-static int parse_autocorrect(const char *value)
+static enum autocorr_mode parse_autocorrect(const char *value)
 {
 	switch (git_parse_maybe_bool_text(value)) {
 		case 1:
@@ -19,41 +19,49 @@ static int parse_autocorrect(const char *value)
 
 	if (!strcmp(value, "prompt"))
 		return AUTOCORRECT_PROMPT;
-	if (!strcmp(value, "never"))
+	else if (!strcmp(value, "never"))
 		return AUTOCORRECT_NEVER;
-	if (!strcmp(value, "immediate"))
+	else if (!strcmp(value, "immediate"))
 		return AUTOCORRECT_IMMEDIATELY;
-	if (!strcmp(value, "show"))
+	else if (!strcmp(value, "show"))
 		return AUTOCORRECT_SHOW;
-
-	return 0;
+	else
+		return AUTOCORRECT_DELAY;
 }
 
 void autocorr_resolve_config(const char *var, const char *value,
 			     const struct config_context *ctx, void *data)
 {
-	int *out = data;
+	struct autocorr *conf = data;
+
+	if (strcmp(var, "help.autocorrect"))
+		return;
+
+	conf->mode = parse_autocorrect(value);
 
-	if (!strcmp(var, "help.autocorrect")) {
-		int v = parse_autocorrect(value);
+	/*
+	 * Disable autocorrection prompt in a non-interactive session.
+	 */
+	if (conf->mode == AUTOCORRECT_PROMPT && (!isatty(0) || !isatty(2)))
+		conf->mode = AUTOCORRECT_NEVER;
 
-		if (!v) {
-			v = git_config_int(var, value, ctx->kvi);
-			if (v < 0 || v == 1)
-				v = AUTOCORRECT_IMMEDIATELY;
-		}
+	if (conf->mode == AUTOCORRECT_DELAY) {
+		conf->delay = git_config_int(var, value, ctx->kvi);
 
-		*out = v;
+		if (!conf->delay)
+			conf->mode = AUTOCORRECT_SHOW;
+		else if (conf->delay <= 1)
+			conf->mode = AUTOCORRECT_IMMEDIATELY;
 	}
 }
 
-void autocorr_confirm(int autocorrect, const char *assumed)
+void autocorr_confirm(struct autocorr *conf, const char *assumed)
 {
-	if (autocorrect == AUTOCORRECT_IMMEDIATELY) {
+	if (conf->mode == AUTOCORRECT_IMMEDIATELY) {
 		fprintf_ln(stderr,
 			   _("Continuing under the assumption that you meant '%s'."),
 			   assumed);
-	} else if (autocorrect == AUTOCORRECT_PROMPT) {
+	} else if (conf->mode == AUTOCORRECT_PROMPT) {
 		char *answer;
 		struct strbuf msg = STRBUF_INIT;
 
@@ -63,10 +71,10 @@ void autocorr_confirm(int autocorrect, const char *assumed)
 
 		if (!(starts_with(answer, "y") || starts_with(answer, "Y")))
 			exit(1);
-	} else {
+	} else if (conf->mode == AUTOCORRECT_DELAY) {
 		fprintf_ln(stderr,
 			   _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
-			   (float)autocorrect / 10.0, assumed);
-		sleep_millisec(autocorrect * 100);
+			   conf->delay / 10.0, assumed);
+		sleep_millisec(conf->delay * 100);
 	}
 }
diff --git a/autocorrect.h b/autocorrect.h
index 45609990c77e..ce4a68379f2f 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -1,16 +1,24 @@
 #ifndef AUTOCORRECT_H
 #define AUTOCORRECT_H
 
-#define AUTOCORRECT_SHOW (-4)
-#define AUTOCORRECT_PROMPT (-3)
-#define AUTOCORRECT_NEVER (-2)
-#define AUTOCORRECT_IMMEDIATELY (-1)
-
 struct config_context;
 
+enum autocorr_mode {
+	AUTOCORRECT_SHOW,
+	AUTOCORRECT_NEVER,
+	AUTOCORRECT_PROMPT,
+	AUTOCORRECT_IMMEDIATELY,
+	AUTOCORRECT_DELAY,
+};
+
+struct autocorr {
+	enum autocorr_mode mode;
+	int delay;
+};
+
 void autocorr_resolve_config(const char *var, const char *value,
 			     const struct config_context *ctx, void *data);
 
-void autocorr_confirm(int autocorr, const char *assumed);
+void autocorr_confirm(struct autocorr *conf, const char *assumed);
 
 #endif /* AUTOCORRECT_H */
diff --git a/help.c b/help.c
index 6be3ec9dfb12..566d33299b9a 100644
--- a/help.c
+++ b/help.c
@@ -538,7 +538,7 @@ int is_in_cmdlist(struct cmdnames *c, const char *s)
 }
 
 struct help_unknown_cmd_config {
-	int autocorrect;
+	struct autocorr autocorr;
 	struct cmdnames aliases;
 };
 
@@ -550,7 +550,7 @@ static int git_unknown_cmd_config(const char *var, const char *value,
 	const char *subsection, *key;
 	size_t subsection_len;
 
-	autocorr_resolve_config(var, value, ctx, &cfg->autocorrect);
+	autocorr_resolve_config(var, value, ctx, &cfg->autocorr);
 
 	/* Also use aliases for command lookup */
 	if (!parse_config_key(var, "alias", &subsection, &subsection_len,
@@ -607,13 +607,7 @@ char *help_unknown_cmd(const char *cmd)
 
 	read_early_config(the_repository, git_unknown_cmd_config, &cfg);
 
-	/*
-	 * Disable autocorrection prompt in a non-interactive session
-	 */
-	if ((cfg.autocorrect == AUTOCORRECT_PROMPT) && (!isatty(0) || !isatty(2)))
-		cfg.autocorrect = AUTOCORRECT_NEVER;
-
-	if (cfg.autocorrect == AUTOCORRECT_NEVER) {
+	if (cfg.autocorr.mode == AUTOCORRECT_NEVER) {
 		fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
 		exit(1);
 	}
@@ -679,7 +673,8 @@ char *help_unknown_cmd(const char *cmd)
 		     n++)
 			; /* still counting */
 	}
-	if (cfg.autocorrect && cfg.autocorrect != AUTOCORRECT_SHOW && n == 1 &&
+
+	if (cfg.autocorr.mode != AUTOCORRECT_SHOW && n == 1 &&
 	    SIMILAR_ENOUGH(best_similarity)) {
 		char *assumed = xstrdup(main_cmds.names[0]->name);
 
@@ -688,7 +683,7 @@ char *help_unknown_cmd(const char *cmd)
 			     "which does not exist."),
 			   cmd);
 
-		autocorr_confirm(cfg.autocorrect, assumed);
+		autocorr_confirm(&cfg.autocorr, assumed);
 
 		cmdnames_release(&cfg.aliases);
 		cmdnames_release(&main_cmds);
-- 
2.53.0

[PATCH v3 4/8] autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINTONLY

[Jiamu Sun]

AUTOCORRECT_SHOW is ambiguous. Its purpose is to show commands similar
to the unknown one and take no other action. Rename it to fit the
semantics.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 autocorrect.c | 6 +++---
 autocorrect.h | 2 +-
 help.c        | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/autocorrect.c b/autocorrect.c
index 50d7f116d85d..9c4b691fb003 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -12,7 +12,7 @@ static enum autocorr_mode parse_autocorrect(const char *value)
 		case 1:
 			return AUTOCORRECT_IMMEDIATELY;
 		case 0:
-			return AUTOCORRECT_SHOW;
+			return AUTOCORRECT_HINTONLY;
 		default: /* other random text */
 			break;
 	}
@@ -24,7 +24,7 @@ static enum autocorr_mode parse_autocorrect(const char *value)
 	else if (!strcmp(value, "immediate"))
 		return AUTOCORRECT_IMMEDIATELY;
 	else if (!strcmp(value, "show"))
-		return AUTOCORRECT_SHOW;
+		return AUTOCORRECT_HINTONLY;
 	else
 		return AUTOCORRECT_DELAY;
 }
@@ -49,7 +49,7 @@ void autocorr_resolve_config(const char *var, const char *value,
 		conf->delay = git_config_int(var, value, ctx->kvi);
 
 		if (!conf->delay)
-			conf->mode = AUTOCORRECT_SHOW;
+			conf->mode = AUTOCORRECT_HINTONLY;
 		else if (conf->delay <= 1)
 			conf->mode = AUTOCORRECT_IMMEDIATELY;
 	}
diff --git a/autocorrect.h b/autocorrect.h
index ce4a68379f2f..be4e3e8b2043 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -4,7 +4,7 @@
 struct config_context;
 
 enum autocorr_mode {
-	AUTOCORRECT_SHOW,
+	AUTOCORRECT_HINTONLY,
 	AUTOCORRECT_NEVER,
 	AUTOCORRECT_PROMPT,
 	AUTOCORRECT_IMMEDIATELY,
diff --git a/help.c b/help.c
index 566d33299b9a..6158545e48a1 100644
--- a/help.c
+++ b/help.c
@@ -674,7 +674,7 @@ char *help_unknown_cmd(const char *cmd)
 			; /* still counting */
 	}
 
-	if (cfg.autocorr.mode != AUTOCORRECT_SHOW && n == 1 &&
+	if (cfg.autocorr.mode != AUTOCORRECT_HINTONLY && n == 1 &&
 	    SIMILAR_ENOUGH(best_similarity)) {
 		char *assumed = xstrdup(main_cmds.names[0]->name);
 
-- 
2.53.0

[PATCH v3 5/8] autocorrect: provide config resolution API

[Jiamu Sun]

Add autocorr_resolve(). This resolves and populates the correct values
for autocorrect config.

Make autocorrect config callback internal. The API is meant to provide
a high-level way to retrieve the config. Allowing access to the config
callback from outside violates that intent.

Additionally, in some cases, without access to the config callback, two
config iterations cannot be merged into one, which can hurt performance.
This is fine, as the code path that calls autocorr_resolve() is cold.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 autocorrect.c | 15 ++++++++++++---
 autocorrect.h |  5 +----
 help.c        | 37 +++++++++++++++----------------------
 3 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/autocorrect.c b/autocorrect.c
index 9c4b691fb003..63fa331ef5e2 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "autocorrect.h"
 #include "config.h"
@@ -29,13 +31,13 @@ static enum autocorr_mode parse_autocorrect(const char *value)
 		return AUTOCORRECT_DELAY;
 }
 
-void autocorr_resolve_config(const char *var, const char *value,
-			     const struct config_context *ctx, void *data)
+static int resolve_autocorr(const char *var, const char *value,
+			    const struct config_context *ctx, void *data)
 {
 	struct autocorr *conf = data;
 
 	if (strcmp(var, "help.autocorrect"))
-		return;
+		return 0;
 
 	conf->mode = parse_autocorrect(value);
 
@@ -53,6 +55,13 @@ void autocorr_resolve_config(const char *var, const char *value,
 		else if (conf->delay <= 1)
 			conf->mode = AUTOCORRECT_IMMEDIATELY;
 	}
+
+	return 0;
+}
+
+void autocorr_resolve(struct autocorr *conf)
+{
+	read_early_config(the_repository, resolve_autocorr, conf);
 }
 
 void autocorr_confirm(struct autocorr *conf, const char *assumed)
diff --git a/autocorrect.h b/autocorrect.h
index be4e3e8b2043..5d82c49903be 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -1,8 +1,6 @@
 #ifndef AUTOCORRECT_H
 #define AUTOCORRECT_H
 
-struct config_context;
-
 enum autocorr_mode {
 	AUTOCORRECT_HINTONLY,
 	AUTOCORRECT_NEVER,
@@ -16,8 +14,7 @@ struct autocorr {
 	int delay;
 };
 
-void autocorr_resolve_config(const char *var, const char *value,
-			     const struct config_context *ctx, void *data);
+void autocorr_resolve(struct autocorr *conf);
 
 void autocorr_confirm(struct autocorr *conf, const char *assumed);
 
diff --git a/help.c b/help.c
index 6158545e48a1..4adb1fcf0726 100644
--- a/help.c
+++ b/help.c
@@ -537,32 +537,24 @@ int is_in_cmdlist(struct cmdnames *c, const char *s)
 	return 0;
 }
 
-struct help_unknown_cmd_config {
-	struct autocorr autocorr;
-	struct cmdnames aliases;
-};
-
-static int git_unknown_cmd_config(const char *var, const char *value,
-				  const struct config_context *ctx,
-				  void *cb)
+static int resolve_aliases(const char *var, const char *value UNUSED,
+			   const struct config_context *ctx UNUSED, void *data)
 {
-	struct help_unknown_cmd_config *cfg = cb;
+	struct cmdnames *aliases = data;
 	const char *subsection, *key;
 	size_t subsection_len;
 
-	autocorr_resolve_config(var, value, ctx, &cfg->autocorr);
-
 	/* Also use aliases for command lookup */
 	if (!parse_config_key(var, "alias", &subsection, &subsection_len,
 			      &key)) {
 		if (subsection) {
 			/* [alias "name"] command = value */
 			if (!strcmp(key, "command"))
-				add_cmdname(&cfg->aliases, subsection,
+				add_cmdname(aliases, subsection,
 					    subsection_len);
 		} else {
 			/* alias.name = value */
-			add_cmdname(&cfg->aliases, key, strlen(key));
+			add_cmdname(aliases, key, strlen(key));
 		}
 	}
 
@@ -599,22 +591,24 @@ static const char bad_interpreter_advice[] =
 
 char *help_unknown_cmd(const char *cmd)
 {
-	struct help_unknown_cmd_config cfg = { 0 };
+	struct cmdnames aliases = { 0 };
+	struct autocorr autocorr = { 0 };
 	int i, n, best_similarity = 0;
 	struct cmdnames main_cmds = { 0 };
 	struct cmdnames other_cmds = { 0 };
 	struct cmdname_help *common_cmds;
 
-	read_early_config(the_repository, git_unknown_cmd_config, &cfg);
+	autocorr_resolve(&autocorr);
 
-	if (cfg.autocorr.mode == AUTOCORRECT_NEVER) {
+	if (autocorr.mode == AUTOCORRECT_NEVER) {
 		fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
 		exit(1);
 	}
 
 	load_command_list("git-", &main_cmds, &other_cmds);
+	read_early_config(the_repository, resolve_aliases, &aliases);
 
-	add_cmd_list(&main_cmds, &cfg.aliases);
+	add_cmd_list(&main_cmds, &aliases);
 	add_cmd_list(&main_cmds, &other_cmds);
 	QSORT(main_cmds.names, main_cmds.cnt, cmdname_compare);
 	uniq(&main_cmds);
@@ -674,18 +668,17 @@ char *help_unknown_cmd(const char *cmd)
 			; /* still counting */
 	}
 
-	if (cfg.autocorr.mode != AUTOCORRECT_HINTONLY && n == 1 &&
+	if (autocorr.mode != AUTOCORRECT_HINTONLY && n == 1 &&
 	    SIMILAR_ENOUGH(best_similarity)) {
 		char *assumed = xstrdup(main_cmds.names[0]->name);
 
 		fprintf_ln(stderr,
-			   _("WARNING: You called a Git command named '%s', "
-			     "which does not exist."),
+			   _("WARNING: You called a Git command named '%s', which does not exist."),
 			   cmd);
 
-		autocorr_confirm(&cfg.autocorr, assumed);
+		autocorr_confirm(&autocorr, assumed);
 
-		cmdnames_release(&cfg.aliases);
+		cmdnames_release(&aliases);
 		cmdnames_release(&main_cmds);
 		cmdnames_release(&other_cmds);
 		return assumed;
-- 
2.53.0

[PATCH v3 6/8] parseopt: autocorrect mistyped subcommands

[Jiamu Sun]

Try to autocorrect the mistyped mandatory subcommand before showing an
error and exiting. Subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL
are skipped.

In autocorrect_subcommand(), AUTOCORR_HINTONLY does the same as
AUTOCORR_NEVER, because builtins have a limited number of subcommands.
Those lists are currently not too large. Therefore, displaying all
subcommands via usage_with_options() is good enough here. This also
keeps the autocorrection handling simple.

Use a dynamic threshold for similar_enough() to check if the result is
usable. This can yield more accurate typo corrections. Even though
subcommands are often short, they can still vary across builtins. And in
the current implementation, a fixed threshold can't do better on both
short and long subcommands at the same time.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
Changes in v3:
  - Improve commit message
  - Fix coding style issue

 parse-options.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 3 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 33f26d6b6179..227bc7499115 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -6,6 +6,8 @@
 #include "strbuf.h"
 #include "string-list.h"
 #include "utf8.h"
+#include "autocorrect.h"
+#include "levenshtein.h"
 
 static int disallow_abbreviated_options;
 
@@ -622,13 +624,72 @@ static int parse_subcommand(const char *arg, const struct option *options)
 	return -1;
 }
 
+static void find_subcommands(struct string_list *list,
+			     const struct option *options)
+{
+	for (; options->type != OPTION_END; options++) {
+		if (options->type == OPTION_SUBCOMMAND)
+			string_list_append(list, options->long_name);
+	}
+}
+
+static int similar_enough(const char *cmd, unsigned int dist)
+{
+	size_t len = strlen(cmd);
+	unsigned int threshold = len < 3 ? 1 : len < 6 ? 3 : 6;
+
+	return dist < threshold;
+}
+
+static const char *autocorrect_subcommand(const char *cmd,
+					  struct string_list *cmds)
+{
+	struct autocorr autocorr = { 0 };
+	unsigned int min = UINT_MAX;
+	unsigned int ties = 0;
+	struct string_list_item *cand;
+	struct string_list_item *best = NULL;
+
+	autocorr_resolve(&autocorr);
+
+	if (autocorr.mode == AUTOCORRECT_NEVER ||
+	    autocorr.mode == AUTOCORRECT_HINTONLY)
+		return NULL;
+
+	for_each_string_list_item(cand, cmds) {
+		unsigned int dist = levenshtein(cmd, cand->string, 0, 2, 1, 3);
+
+		if (dist < min) {
+			min = dist;
+			best = cand;
+			ties = 0;
+		} else if (dist == min) {
+			ties++;
+		}
+	}
+
+	if (!ties && similar_enough(cmd, min)) {
+		fprintf_ln(stderr,
+			   _("WARNING: You called a subcommand named '%s', which does not exist."),
+			   cmd);
+
+		autocorr_confirm(&autocorr, best->string);
+		return best->string;
+	}
+
+	return NULL;
+}
+
 static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 					       const char *arg,
 					       const struct option *options,
 					       const char * const usagestr[])
 {
-	int err = parse_subcommand(arg, options);
+	int err;
+	const char *assumed;
+	struct string_list cmds = STRING_LIST_INIT_NODUP;
 
+	err = parse_subcommand(arg, options);
 	if (!err)
 		return PARSE_OPT_SUBCOMMAND;
 
@@ -641,8 +702,17 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
 		return PARSE_OPT_DONE;
 
-	error(_("unknown subcommand: `%s'"), arg);
-	usage_with_options(usagestr, options);
+	find_subcommands(&cmds, options);
+	assumed = autocorrect_subcommand(arg, &cmds);
+
+	if (!assumed) {
+		error(_("unknown subcommand: `%s'"), arg);
+		usage_with_options(usagestr, options);
+	}
+
+	string_list_clear(&cmds, 0);
+	parse_subcommand(assumed, options);
+	return PARSE_OPT_SUBCOMMAND;
 }
 
 static void check_typos(const char *arg, const struct option *options)
-- 
2.53.0

[PATCH v3 7/8] parseopt: enable subcommand autocorrection for git-remote and git-notes

[Jiamu Sun]

Add PARSE_OPT_SUBCOMMAND_AUTOCORR to enable autocorrection for
subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL.

Use it for git-remote and git-notes, so mistyped subcommands can be
automatically corrected, and builtin entry points no longer need to
handle the unknown subcommand error path themselves.

This is safe for these two builtins, because they either resolve to a
single subcommand or take no subcommand at all. This means that if the
subcommand parser encounters an unknown argument, it must be a mistyped
subcommand.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
Changes in v3:
  - Improve commit message

 builtin/notes.c  | 10 +++-------
 builtin/remote.c | 12 ++++--------
 parse-options.c  | 16 +++++++++-------
 parse-options.h  |  1 +
 4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 9af602bdd7b4..087eb898a441 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -1149,14 +1149,10 @@ int cmd_notes(int argc,
 
 	repo_config(the_repository, git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, git_notes_usage,
-			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
-	if (!fn) {
-		if (argc) {
-			error(_("unknown subcommand: `%s'"), argv[0]);
-			usage_with_options(git_notes_usage, options);
-		}
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL |
+			     PARSE_OPT_SUBCOMMAND_AUTOCORR);
+	if (!fn)
 		fn = list;
-	}
 
 	if (override_notes_ref) {
 		struct strbuf sb = STRBUF_INIT;
diff --git a/builtin/remote.c b/builtin/remote.c
index ace390c671d6..d1d6244a662a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1949,15 +1949,11 @@ int cmd_remote(int argc,
 	};
 
 	argc = parse_options(argc, argv, prefix, options, builtin_remote_usage,
-			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL |
+			     PARSE_OPT_SUBCOMMAND_AUTOCORR);
 
-	if (fn) {
+	if (fn)
 		return !!fn(argc, argv, prefix, repo);
-	} else {
-		if (argc) {
-			error(_("unknown subcommand: `%s'"), argv[0]);
-			usage_with_options(builtin_remote_usage, options);
-		}
+	else
 		return !!show_all();
-	}
 }
diff --git a/parse-options.c b/parse-options.c
index 227bc7499115..6ac4bd30e23b 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -693,14 +693,16 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 	if (!err)
 		return PARSE_OPT_SUBCOMMAND;
 
-	/*
-	 * arg is neither a short or long option nor a subcommand.  Since this
-	 * command has a default operation mode, we have to treat this arg and
-	 * all remaining args as args meant to that default operation mode.
-	 * So we are done parsing.
-	 */
-	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
+	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL &&
+	    !(ctx->flags & PARSE_OPT_SUBCOMMAND_AUTOCORR)) {
+		/*
+		 * arg is neither a short or long option nor a subcommand.
+		 * Since this command has a default operation mode, we have to
+		 * treat this arg and all remaining args as args meant to that
+		 * default operation mode.  So we are done parsing.
+		 */
 		return PARSE_OPT_DONE;
+	}
 
 	find_subcommands(&cmds, options);
 	assumed = autocorrect_subcommand(arg, &cmds);
diff --git a/parse-options.h b/parse-options.h
index 706de9729f6b..f29ac337893c 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -40,6 +40,7 @@ enum parse_opt_flags {
 	PARSE_OPT_ONE_SHOT = 1 << 5,
 	PARSE_OPT_SHELL_EVAL = 1 << 6,
 	PARSE_OPT_SUBCOMMAND_OPTIONAL = 1 << 7,
+	PARSE_OPT_SUBCOMMAND_AUTOCORR = 1 << 8,
 };
 
 enum parse_opt_option_flags {
-- 
2.53.0

[PATCH v3 8/8] help: add tests for subcommand autocorrection

[Jiamu Sun]

These tests cover default behavior (help.autocorrect is unset), no
correction, immediate correction, delayed correction, and rejection
when the typo is too dissimilar.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
Changes in v3:
  - Fix coding style issue

 t/t9004-autocorrect-subcommand.sh | 51 +++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100755 t/t9004-autocorrect-subcommand.sh

diff --git a/t/t9004-autocorrect-subcommand.sh b/t/t9004-autocorrect-subcommand.sh
new file mode 100755
index 000000000000..d10031659b94
--- /dev/null
+++ b/t/t9004-autocorrect-subcommand.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+
+test_description='subcommand auto-correction test
+
+Test autocorrection for subcommands with different
+help.autocorrect mode.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' "
+	echo '^error: unknown subcommand: ' >grep_unknown
+"
+
+test_expect_success 'default is not to autocorrect' '
+	test_must_fail git worktree lsit 2>actual &&
+	head -n1 actual >first && test_grep -f grep_unknown first
+'
+
+for mode in false no off 0 show never
+do
+	test_expect_success "'$mode' disables autocorrection" "
+		test_config help.autocorrect $mode &&
+
+		test_must_fail git worktree lsit 2>actual &&
+		head -n1 actual >first && test_grep -f grep_unknown first
+	"
+done
+
+for mode in -39 immediate 1
+do
+	test_expect_success "autocorrect immediately with '$mode'" - <<-EOT
+		test_config help.autocorrect $mode &&
+
+		git worktree lsit 2>actual &&
+		test_grep "you meant 'list'\.$" actual
+	EOT
+done
+
+test_expect_success 'delay path is executed' - <<-\EOT
+	test_config help.autocorrect 2 &&
+
+	git worktree lsit 2>actual &&
+	test_grep '^Continuing in 0.2 seconds, ' actual
+EOT
+
+test_expect_success 'deny if too dissimilar' - <<-\EOT
+	test_must_fail git remote rensnr 2>actual &&
+	head -n1 actual >first && test_grep -f grep_unknown first
+EOT
+
+test_done
-- 
2.53.0

Re: [PATCH v3 1/8] parseopt: extract subcommand handling from parse_options_step()

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]

Jiamu Sun <39@barroit.sh> writes:

[snip]

> @@ -990,38 +1017,17 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
>  		if (*arg != '-' || !arg[1]) {
>  			if (parse_nodash_opt(ctx, arg, options) == 0)
>  				continue;
> -			if (!ctx->has_subcommands) {
> -				if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
> -					return PARSE_OPT_NON_OPTION;
> -				ctx->out[ctx->cpidx++] = ctx->argv[0];
> -				continue;
> -			}
> -			switch (parse_subcommand(arg, options)) {
> -			case PARSE_OPT_SUBCOMMAND:
> -				return PARSE_OPT_SUBCOMMAND;
> -			case PARSE_OPT_UNKNOWN:
> -				if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
> -					/*
> -					 * arg is neither a short or long
> -					 * option nor a subcommand.  Since
> -					 * this command has a default
> -					 * operation mode, we have to treat
> -					 * this arg and all remaining args
> -					 * as args meant to that default
> -					 * operation mode.
> -					 * So we are done parsing.
> -					 */
> -					return PARSE_OPT_DONE;
> -				error(_("unknown subcommand: `%s'"), arg);
> -				usage_with_options(usagestr, options);
> -			case PARSE_OPT_COMPLETE:
> -			case PARSE_OPT_HELP:
> -			case PARSE_OPT_ERROR:
> -			case PARSE_OPT_DONE:
> -			case PARSE_OPT_NON_OPTION:
> -				/* Impossible. */
> -				BUG("parse_subcommand() cannot return these");
> +
> +			if (ctx->has_subcommands) {
> +				return handle_subcommand(ctx, arg, options,
> +							 usagestr);
>  			}
> +

Nit: we try to avoid braces around single statement blocks.

> +			if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
> +				return PARSE_OPT_NON_OPTION;
> +
> +			ctx->out[ctx->cpidx++] = ctx->argv[0];
> +			continue;
>  		}
>
>  		/* lone -h asks for help */
> --
> 2.53.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 2/8] help: make autocorrect handling reusable

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 3092 bytes --]

Jiamu Sun <39@barroit.sh> writes:

> Move config parsing and prompt/delay handling into autocorrect.c and
> expose them in autocorrect.h. This makes autocorrect reusable regardless
> of which target links against it.
>
> Signed-off-by: Jiamu Sun <39@barroit.sh>
> ---
> This patch moves autocorrect handling out of help.c as a preparatory
> step for subcommand autocorrection.
>
> This is necessary because help.c uses load_builtin_commands(), which is
> provided by git.o. Executables that do not link against git.o, such as
> git-daemon, will hit a link error if they use symbols defined in help.o.
> A simple and clean fix is to make the relevant functions independent of
> help.c and move them to a dedicated file.
>
> Changes in v3:
>   - Split patch so diffs don't get hidden by code movement
>
>  Makefile      |  1 +
>  autocorrect.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  autocorrect.h | 16 ++++++++++++
>  help.c        | 64 +++------------------------------------------
>  4 files changed, 93 insertions(+), 60 deletions(-)
>  create mode 100644 autocorrect.c
>  create mode 100644 autocorrect.h
>

This should also be added to meson.build.

[snip]

> diff --git a/autocorrect.h b/autocorrect.h
> new file mode 100644
> index 000000000000..45609990c77e
> --- /dev/null
> +++ b/autocorrect.h
> @@ -0,0 +1,16 @@
> +#ifndef AUTOCORRECT_H
> +#define AUTOCORRECT_H
> +
> +#define AUTOCORRECT_SHOW (-4)
> +#define AUTOCORRECT_PROMPT (-3)
> +#define AUTOCORRECT_NEVER (-2)
> +#define AUTOCORRECT_IMMEDIATELY (-1)
> +
> +struct config_context;
> +
> +void autocorr_resolve_config(const char *var, const char *value,
> +			     const struct config_context *ctx, void *data);
> +
> +void autocorr_confirm(int autocorr, const char *assumed);
> +

Why not use s/autocorr/autocorrect/ ? Also would be nice to add some
documentation about each of the functions here.

[snip]

Also got this from running `git-clang-format` on this commit. Generally
applying changes while moving code makes it harder to review. But here
the changes are small enough that we could get away with it. I'll leave
it to you.

diff --git a/autocorrect.c b/autocorrect.c
index 1037f03201..87351fd08f 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -9,12 +9,12 @@
 static int parse_autocorrect(const char *value)
 {
 	switch (git_parse_maybe_bool_text(value)) {
-		case 1:
-			return AUTOCORRECT_IMMEDIATELY;
-		case 0:
-			return AUTOCORRECT_SHOW;
-		default: /* other random text */
-			break;
+	case 1:
+		return AUTOCORRECT_IMMEDIATELY;
+	case 0:
+		return AUTOCORRECT_SHOW;
+	default: /* other random text */
+		break;
 	}

 	if (!strcmp(value, "prompt"))
diff --git a/autocorrect.h b/autocorrect.h
index 45609990c7..38f1e73131 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -1,9 +1,9 @@
 #ifndef AUTOCORRECT_H
 #define AUTOCORRECT_H

-#define AUTOCORRECT_SHOW (-4)
-#define AUTOCORRECT_PROMPT (-3)
-#define AUTOCORRECT_NEVER (-2)
+#define AUTOCORRECT_SHOW	(-4)
+#define AUTOCORRECT_PROMPT	(-3)
+#define AUTOCORRECT_NEVER	(-2)
 #define AUTOCORRECT_IMMEDIATELY (-1)

 struct config_context;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 3/8] help: move tty check for autocorrection to autocorrect.c

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 4466 bytes --]

Jiamu Sun <39@barroit.sh> writes:

> TTY checking is the autocorrect config parser's responsibility. It must
> ensure the parsed value is correct and reliable. Thus, move the check to
> autocorr_resolve_config().
>
> Signed-off-by: Jiamu Sun <39@barroit.sh>
> ---
> In parse_autocorrect() I kept the if/else if cascade. In my opinion,
> this is not a style change. It's a control-flow clarification, because
> those returns describe different states of the mode. Chaining them
> together is better than leaving them discrete.
>
> Also, I dropped the float cast in the delay calculation, which is
> redundant.
>
>  autocorrect.c | 50 +++++++++++++++++++++++++++++---------------------
>  autocorrect.h | 20 ++++++++++++++------
>  help.c        | 17 ++++++-----------
>  3 files changed, 49 insertions(+), 38 deletions(-)
>
> diff --git a/autocorrect.c b/autocorrect.c
> index 1037f032019b..50d7f116d85d 100644
> --- a/autocorrect.c
> +++ b/autocorrect.c
> @@ -6,7 +6,7 @@
>  #include "prompt.h"
>  #include "gettext.h"
>
> -static int parse_autocorrect(const char *value)
> +static enum autocorr_mode parse_autocorrect(const char *value)
>  {
>  	switch (git_parse_maybe_bool_text(value)) {
>  		case 1:
> @@ -19,41 +19,49 @@ static int parse_autocorrect(const char *value)
>
>  	if (!strcmp(value, "prompt"))
>  		return AUTOCORRECT_PROMPT;
> -	if (!strcmp(value, "never"))
> +	else if (!strcmp(value, "never"))
>  		return AUTOCORRECT_NEVER;
> -	if (!strcmp(value, "immediate"))
> +	else if (!strcmp(value, "immediate"))
>  		return AUTOCORRECT_IMMEDIATELY;
> -	if (!strcmp(value, "show"))
> +	else if (!strcmp(value, "show"))
>  		return AUTOCORRECT_SHOW;
> -
> -	return 0;
> +	else
> +		return AUTOCORRECT_DELAY;
>  }
>

Okay so since we introduce an enum we use that here.

>  void autocorr_resolve_config(const char *var, const char *value,
>  			     const struct config_context *ctx, void *data)
>  {
> -	int *out = data;
> +	struct autocorr *conf = data;
> +
> +	if (strcmp(var, "help.autocorrect"))
> +		return;
> +
> +	conf->mode = parse_autocorrect(value);
>
> -	if (!strcmp(var, "help.autocorrect")) {
> -		int v = parse_autocorrect(value);
> +	/*
> +	 * Disable autocorrection prompt in a non-interactive session.
> +	 */
> +	if (conf->mode == AUTOCORRECT_PROMPT && (!isatty(0) || !isatty(2)))
> +		conf->mode = AUTOCORRECT_NEVER;
>
> -		if (!v) {
> -			v = git_config_int(var, value, ctx->kvi);
> -			if (v < 0 || v == 1)
> -				v = AUTOCORRECT_IMMEDIATELY;
> -		}
> +	if (conf->mode == AUTOCORRECT_DELAY) {
> +		conf->delay = git_config_int(var, value, ctx->kvi);
>
> -		*out = v;
> +		if (!conf->delay)
> +			conf->mode = AUTOCORRECT_SHOW;
> +		else if (conf->delay <= 1)
> +			conf->mode = AUTOCORRECT_IMMEDIATELY;
>  	}
>  }
>
> -void autocorr_confirm(int autocorrect, const char *assumed)
> +void autocorr_confirm(struct autocorr *conf, const char *assumed)
>  {
> -	if (autocorrect == AUTOCORRECT_IMMEDIATELY) {
> +	if (conf->mode == AUTOCORRECT_IMMEDIATELY) {
>  		fprintf_ln(stderr,
>  			   _("Continuing under the assumption that you meant '%s'."),
>  			   assumed);
> -	} else if (autocorrect == AUTOCORRECT_PROMPT) {
> +	} else if (conf->mode == AUTOCORRECT_PROMPT) {
>  		char *answer;
>  		struct strbuf msg = STRBUF_INIT;
>
> @@ -63,10 +71,10 @@ void autocorr_confirm(int autocorrect, const char *assumed)
>
>  		if (!(starts_with(answer, "y") || starts_with(answer, "Y")))
>  			exit(1);
> -	} else {
> +	} else if (conf->mode == AUTOCORRECT_DELAY) {
>  		fprintf_ln(stderr,
>  			   _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
> -			   (float)autocorrect / 10.0, assumed);
> -		sleep_millisec(autocorrect * 100);
> +			   conf->delay / 10.0, assumed);
> +		sleep_millisec(conf->delay * 100);
>  	}
>  }
> diff --git a/autocorrect.h b/autocorrect.h
> index 45609990c77e..ce4a68379f2f 100644
> --- a/autocorrect.h
> +++ b/autocorrect.h
> @@ -1,16 +1,24 @@
>  #ifndef AUTOCORRECT_H
>  #define AUTOCORRECT_H
>
> -#define AUTOCORRECT_SHOW (-4)
> -#define AUTOCORRECT_PROMPT (-3)
> -#define AUTOCORRECT_NEVER (-2)
> -#define AUTOCORRECT_IMMEDIATELY (-1)
> -
>  struct config_context;
>
> +enum autocorr_mode {
> +	AUTOCORRECT_SHOW,
> +	AUTOCORRECT_NEVER,
> +	AUTOCORRECT_PROMPT,
> +	AUTOCORRECT_IMMEDIATELY,
> +	AUTOCORRECT_DELAY,
> +};
> +
> +struct autocorr {
> +	enum autocorr_mode mode;
> +	int delay;
> +};
> +

I would say the naming doesn't indicate what it is used for. How about
'autocorrect_config'?

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 4/8] autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINTONLY

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

Jiamu Sun <39@barroit.sh> writes:

> AUTOCORRECT_SHOW is ambiguous. Its purpose is to show commands similar
> to the unknown one and take no other action. Rename it to fit the
> semantics.
>

I'm not sure if AUTOCORRECT_HINTONLY is any better than
AUTOCORRECT_SHOW. The latter indicates that we show the user something.
Doesn't the former also mean the same?

If we do decide to go forward with AUTOCORRECT_HINTONLY, can we rename
it to AUTOCORRECT_HINT_ONLY? It's easier to read that way.

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 5/8] autocorrect: provide config resolution API

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1578 bytes --]

Jiamu Sun <39@barroit.sh> writes:

> Add autocorr_resolve(). This resolves and populates the correct values
> for autocorrect config.
>
> Make autocorrect config callback internal. The API is meant to provide
> a high-level way to retrieve the config. Allowing access to the config
> callback from outside violates that intent.
>
> Additionally, in some cases, without access to the config callback, two
> config iterations cannot be merged into one, which can hurt performance.
> This is fine, as the code path that calls autocorr_resolve() is cold.
>
> Signed-off-by: Jiamu Sun <39@barroit.sh>
> ---
>  autocorrect.c | 15 ++++++++++++---
>  autocorrect.h |  5 +----
>  help.c        | 37 +++++++++++++++----------------------
>  3 files changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/autocorrect.c b/autocorrect.c
> index 9c4b691fb003..63fa331ef5e2 100644
> --- a/autocorrect.c
> +++ b/autocorrect.c
> @@ -1,3 +1,5 @@
> +#define USE_THE_REPOSITORY_VARIABLE
> +
>  #include "git-compat-util.h"
>  #include "autocorrect.h"
>  #include "config.h"
> @@ -29,13 +31,13 @@ static enum autocorr_mode parse_autocorrect(const char *value)
>  		return AUTOCORRECT_DELAY;
>  }
>
> -void autocorr_resolve_config(const char *var, const char *value,
> -			     const struct config_context *ctx, void *data)
> +static int resolve_autocorr(const char *var, const char *value,
> +			    const struct config_context *ctx, void *data)

So we made this an internal, the return type was changed as
`read_early_config()` expects that. Okay

[snip]

The rest of the code makes sense too

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 2/8] help: make autocorrect handling reusable

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

>> +#define AUTOCORRECT_SHOW (-4)
>> +#define AUTOCORRECT_PROMPT (-3)
>> +#define AUTOCORRECT_NEVER (-2)
>> +#define AUTOCORRECT_IMMEDIATELY (-1)
>> +
>> +struct config_context;
>> +
>> +void autocorr_resolve_config(const char *var, const char *value,
>> +			     const struct config_context *ctx, void *data);
>> +
>> +void autocorr_confirm(int autocorr, const char *assumed);
>> +
>
> Why not use s/autocorr/autocorrect/ ? Also would be nice to add some
> documentation about each of the functions here.

Good.

>
> [snip]
>
> Also got this from running `git-clang-format` on this commit. Generally
> applying changes while moving code makes it harder to review. But here
> the changes are small enough that we could get away with it. I'll leave
> it to you.

No, you cannot leave it to the author.  

Leave it to CodingGuidelines; case and switch would start at the
same column.

> diff --git a/autocorrect.c b/autocorrect.c
> index 1037f03201..87351fd08f 100644
> --- a/autocorrect.c
> +++ b/autocorrect.c
> @@ -9,12 +9,12 @@
>  static int parse_autocorrect(const char *value)
>  {
>  	switch (git_parse_maybe_bool_text(value)) {
> -		case 1:
> -			return AUTOCORRECT_IMMEDIATELY;
> -		case 0:
> -			return AUTOCORRECT_SHOW;
> -		default: /* other random text */
> -			break;
> +	case 1:
> +		return AUTOCORRECT_IMMEDIATELY;
> +	case 0:
> +		return AUTOCORRECT_SHOW;
> +	default: /* other random text */
> +		break;
>  	}
>
>  	if (!strcmp(value, "prompt"))
> diff --git a/autocorrect.h b/autocorrect.h
> index 45609990c7..38f1e73131 100644
> --- a/autocorrect.h
> +++ b/autocorrect.h
> @@ -1,9 +1,9 @@
>  #ifndef AUTOCORRECT_H
>  #define AUTOCORRECT_H
>
> -#define AUTOCORRECT_SHOW (-4)
> -#define AUTOCORRECT_PROMPT (-3)
> -#define AUTOCORRECT_NEVER (-2)
> +#define AUTOCORRECT_SHOW	(-4)
> +#define AUTOCORRECT_PROMPT	(-3)
> +#define AUTOCORRECT_NEVER	(-2)
>  #define AUTOCORRECT_IMMEDIATELY (-1)
>
>  struct config_context;

Re: [PATCH v3 6/8] parseopt: autocorrect mistyped subcommands

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> +static int similar_enough(const char *cmd, unsigned int dist)
> +{
> +	size_t len = strlen(cmd);
> +	unsigned int threshold = len < 3 ? 1 : len < 6 ? 3 : 6;
> +
> +	return dist < threshold;
> +}

There should be some explanation on the reason why this is very
different from SIMILAR_ENOUGH used in help.c for main commands,
especially given that the levenshtein() call here uses identical
weight parameters (0,2,1,3) as used by the call there.

> +static const char *autocorrect_subcommand(const char *cmd,
> + ...
> +	for_each_string_list_item(cand, cmds) {
> +		unsigned int dist = levenshtein(cmd, cand->string, 0, 2, 1, 3);
> +
> +		if (dist < min) {
> +			min = dist;
> +			best = cand;
> +			ties = 0;
> +		} else if (dist == min) {
> +			ties++;
> +		}
> +	}
> +
> +	if (!ties && similar_enough(cmd, min)) {

Re: [PATCH v3 1/8] parseopt: extract subcommand handling from parse_options_step()

[Jiamu Sun]

On Tue, Mar 10, 2026 at 05:46:12AM -0700, Karthik Nayak wrote:
> > +			if (ctx->has_subcommands) {
> > +				return handle_subcommand(ctx, arg, options,
> > +							 usagestr);
> >  			}
> > +
> 
> Nit: we try to avoid braces around single statement blocks.

I'm not sure if we should drop the braces in this case. I mean, the
statement is indeed a single one, but it spans multiple lines. Keeping
the braces improves readability. Also, CodingGuidelines says: "When the
statement extends over a few lines" use braces. So I think we should
keep those braces?

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH v3 2/8] help: make autocorrect handling reusable

[Jiamu Sun]

On Tue, Mar 10, 2026 at 05:52:58AM -0700, Karthik Nayak wrote:
> >  autocorrect.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  autocorrect.h | 16 ++++++++++++
> >  help.c        | 64 +++------------------------------------------
> >  4 files changed, 93 insertions(+), 60 deletions(-)
> >  create mode 100644 autocorrect.c
> >  create mode 100644 autocorrect.h
> >
> 
> This should also be added to meson.build.

Sure, and if I missed anything else, please tell me, thanks.

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH v3 2/8] help: make autocorrect handling reusable

[Jiamu Sun]

On Tue, Mar 10, 2026 at 01:10:22PM -0700, Junio C Hamano wrote:
> > Why not use s/autocorr/autocorrect/ ? Also would be nice to add some
> > documentation about each of the functions here.
> 
> Good.

Used autocorr just because it's shorter than autocorrect. Will rename it
and add docs in v4.

> > Also got this from running `git-clang-format` on this commit. Generally
> > applying changes while moving code makes it harder to review. But here
> > the changes are small enough that we could get away with it. I'll leave
> > it to you.
> 
> No, you cannot leave it to the author.  
> 
> Leave it to CodingGuidelines; case and switch would start at the
> same column.

Was just copy-pasted as-is. Will fix.

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH v3 3/8] help: move tty check for autocorrection to autocorrect.c

[Jiamu Sun]

On Tue, Mar 10, 2026 at 07:06:17AM -0700, Karthik Nayak wrote:
> > +struct autocorr {
> > +	enum autocorr_mode mode;
> > +	int delay;
> > +};
> > +
> 
> I would say the naming doesn't indicate what it is used for. How about
> 'autocorrect_config'?

Using autocorrect_config is best for semantics. The only problem is that
it's too long.  Since currently, this struct contains all fields needed
by our autocorrect_*, using "autocorrect" to describe it is enough,
perhaps?

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH v3 4/8] autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINTONLY

[Jiamu Sun]

On Tue, Mar 10, 2026 at 07:08:52AM -0700, Karthik Nayak wrote:
> 
> [snip]
> 
> I'm not sure if AUTOCORRECT_HINTONLY is any better than
> AUTOCORRECT_SHOW. The latter indicates that we show the user something.
> Doesn't the former also mean the same?

You are right, it indicates that we show the user something.  But show
what?  A prompt can also contain a message that acts as a "show", and I
was quite confused when I first saw this part.  On the other hand,
HINTONLY has a clear intent, which is "hint".

> If we do decide to go forward with AUTOCORRECT_HINTONLY, can we rename
> it to AUTOCORRECT_HINT_ONLY? It's easier to read that way.

AUTOCORRECT_HINT_ONLY breaks the current naming pattern of the mode
enum.  Maybe we can use "AUTOCORRECT_HINT", as the "only" intent is
clear with other enum items.

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH v3 6/8] parseopt: autocorrect mistyped subcommands

[Jiamu Sun]

On Tue, Mar 10, 2026 at 01:16:39PM -0700, Junio C Hamano wrote:
> > +	unsigned int threshold = len < 3 ? 1 : len < 6 ? 3 : 6;
> > +
> > +	return dist < threshold;
> > +}
> 
> There should be some explanation on the reason why this is very
> different from SIMILAR_ENOUGH used in help.c for main commands,
> especially given that the levenshtein() call here uses identical
> weight parameters (0,2,1,3) as used by the call there.

Will add a comment to explain it.

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH v3 1/8] parseopt: extract subcommand handling from parse_options_step()

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> On Tue, Mar 10, 2026 at 05:46:12AM -0700, Karthik Nayak wrote:
>> > +			if (ctx->has_subcommands) {
>> > +				return handle_subcommand(ctx, arg, options,
>> > +							 usagestr);
>> >  			}
>> > +
>> 
>> Nit: we try to avoid braces around single statement blocks.
>
> I'm not sure if we should drop the braces in this case.

You should.

You can tell that it is a single statement immediately after seeing
the beginning of the line, which says "return".  It does not matter
how many lines the function call that follows "return" spans.

Re: [PATCH v3 1/8] parseopt: extract subcommand handling from parse_options_step()

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> On Tue, Mar 10, 2026 at 05:46:12AM -0700, Karthik Nayak wrote:
>> > +			if (ctx->has_subcommands) {
>> > +				return handle_subcommand(ctx, arg, options,
>> > +							 usagestr);
>> >  			}
>> > +
>> 
>> Nit: we try to avoid braces around single statement blocks.
>
> I'm not sure if we should drop the braces in this case.

You should.

You can tell that it is a single statement immediately after seeing
the beginning of the line, which says "return".  It does not matter
how many lines the function call that follows "return" spans.

Re: [PATCH v3 8/8] help: add tests for subcommand autocorrection

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> These tests cover default behavior (help.autocorrect is unset), no
> correction, immediate correction, delayed correction, and rejection
> when the typo is too dissimilar.
>
> Signed-off-by: Jiamu Sun <39@barroit.sh>
> ---
> Changes in v3:
>   - Fix coding style issue
>
>  t/t9004-autocorrect-subcommand.sh | 51 +++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100755 t/t9004-autocorrect-subcommand.sh

t/meson.build needs to be told about this file, or you'll break CI,
like this:

    https://github.com/git/git/actions/runs/22929006339/job/66546202060

Re: [PATCH v3 8/8] help: add tests for subcommand autocorrection

[Jiamu Sun]

On Tue, Mar 10, 2026 at 09:23:03PM -0700, Junio C Hamano wrote:
> t/meson.build needs to be told about this file, or you'll break CI,
> like this:
> 
>     https://github.com/git/git/actions/runs/22929006339/job/66546202060

Will update and verify the meson build. Thanks for the catch.

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH v3 1/8] parseopt: extract subcommand handling from parse_options_step()

[Jiamu Sun]

On Tue, Mar 10, 2026 at 09:20:30PM -0700, Junio C Hamano wrote:
> You can tell that it is a single statement immediately after seeing
> the beginning of the line, which says "return".  It does not matter
> how many lines the function call that follows "return" spans.

Makes sense, will drop it.

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH 5/5] help: add tests for subcommand autocorrection

[Aaron Plattner]

On 3/8/26 5:17 AM, Jiamu Sun wrote:
> These tests cover default behavior (help.autocorrect is unset), no
> correction, immediate correction, delayed correction, and rejection
> when the typo is too dissimilar.
> 
> Signed-off-by: Jiamu Sun <39@barroit.sh>
> ---
>   t/t9004-autocorrect-subcommand.sh | 49 +++++++++++++++++++++++++++++++
>   1 file changed, 49 insertions(+)
>   create mode 100755 t/t9004-autocorrect-subcommand.sh

Adding this causes meson setup to fail:

t/meson.build:1193:6: ERROR: Problem encountered: Test files found, but 
not configured:

  - t9004-autocorrect-subcommand.sh


I think you just need to add it to meson.build:

diff --git a/t/meson.build b/t/meson.build
index f66a73f8a0..bf0503d705 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -973,6 +973,7 @@ integration_tests = [
    't9001-send-email.sh',
    't9002-column.sh',
    't9003-help-autocorrect.sh',
+  't9004-autocorrect-subcommand.sh',
    't9100-git-svn-basic.sh',
    't9101-git-svn-props.sh',
    't9102-git-svn-deep-rmdir.sh',

-- Aaron

> 
> diff --git a/t/t9004-autocorrect-subcommand.sh b/t/t9004-autocorrect-subcommand.sh
> new file mode 100755
> index 000000000000..760760c8851a
> --- /dev/null
> +++ b/t/t9004-autocorrect-subcommand.sh
> @@ -0,0 +1,49 @@
> +#!/bin/sh
> +
> +test_description='subcommand auto-correction test
> +
> +Test autocorrection for subcommands with different
> +help.autocorrect mode.'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' "
> +	echo '^error: unknown subcommand: ' >grep_unknown
> +"
> +
> +test_expect_success 'default is not to autocorrect' '
> +	test_must_fail git worktree lsit 2>actual &&
> +	head -n1 actual >first && test_grep -f grep_unknown first
> +'
> +
> +for mode in false no off 0 show never; do
> +	test_expect_success "'$mode' disables autocorrection" "
> +		test_config help.autocorrect $mode &&
> +
> +		test_must_fail git worktree lsit 2>actual &&
> +		head -n1 actual >first && test_grep -f grep_unknown first
> +	"
> +done
> +
> +for mode in -39 immediate 1; do
> +	test_expect_success "autocorrect immediately with '$mode'" - <<-EOT
> +		test_config help.autocorrect $mode &&
> +
> +		git worktree lsit 2>actual &&
> +		test_grep "you meant 'list'\.$" actual
> +	EOT
> +done
> +
> +test_expect_success 'delay path is executed' - <<-\EOT
> +	test_config help.autocorrect 2 &&
> +
> +	git worktree lsit 2>actual &&
> +	test_grep '^Continuing in 0.2 seconds, ' actual
> +EOT
> +
> +test_expect_success 'deny if too dissimilar' - <<-\EOT
> +	test_must_fail git remote rensnr 2>actual &&
> +	head -n1 actual >first && test_grep -f grep_unknown first
> +EOT
> +
> +test_done

Re: [PATCH 5/5] help: add tests for subcommand autocorrection

[Jiamu Sun]

On Wed, Mar 11, 2026 at 10:01:55AM -0700, Aaron Plattner wrote:
> t/meson.build:1193:6: ERROR: Problem encountered: Test files found, but not
> configured:
> 
>  - t9004-autocorrect-subcommand.sh
> 
> 
> I think you just need to add it to meson.build:

Will add it, thanks!

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH v3 6/8] parseopt: autocorrect mistyped subcommands

[Jiamu Sun]

On Wed, Mar 11, 2026 at 11:48:41AM +0900, Jiamu Sun wrote:
> > There should be some explanation on the reason why this is very
> > different from SIMILAR_ENOUGH used in help.c for main commands,
> > especially given that the levenshtein() call here uses identical
> > weight parameters (0,2,1,3) as used by the call there.
> 
> Will add a comment to explain it.

	/* skip and count prefix matches */
	for (n = 0; n < main_cmds.cnt && !main_cmds.names[n]->len; n++)
		; /* still counting */

	if (main_cmds.cnt <= n) {
		/* prefix matches with everything? that is too ambiguous */
		best_similarity = SIMILARITY_FLOOR + 1;
	} else {
		/* count all the most similar ones */
		for (best_similarity = main_cmds.names[n++]->len;
		     (n < main_cmds.cnt &&
		      best_similarity == main_cmds.names[n]->len);
		     n++)
			; /* still counting */
	}

	if (autocorrect.mode != AUTOCORRECT_HINTONLY && n == 1 &&

If I read the code correctly, for the prefix matched case, the similar
command finding in help.c skips all prefix matched strings and increases
"n". Inside the "else", since that "n++", the "n" will always be greater
than one, thus no correction happens, e.g., it doesn't autocorrect
"commi" to "commit". Do you know if this behavior is by design or
something else?

Re: [PATCH v3 3/8] help: move tty check for autocorrection to autocorrect.c

[Jiamu Sun]

On Tue, Mar 10, 2026 at 07:06:17AM -0700, Karthik Nayak wrote:
> > +		return AUTOCORRECT_DELAY;
> >  }
> >
> 
> Okay so since we introduce an enum we use that here.

Ah actually, I messed up this patch. There was a commit that moved
autocorrect config into a struct and introduced the mode enum. But
somehow it's gone. I'll split this patch into two.

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH v3 6/8] parseopt: autocorrect mistyped subcommands

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> "commi" to "commit". Do you know if this behavior is by design or
> something else?

I don't, but others who have their hand in the code may.  "git
blame" or "git shortlog --no-merges" may be good tools to use to
find out who they are.

Thanks.

Re: [PATCH v3 6/8] parseopt: autocorrect mistyped subcommands

[Jiamu Sun]

On Wed, Mar 11, 2026 at 07:38:51PM -0700, Junio C Hamano wrote:
> I don't, but others who have their hand in the code may.  "git
> blame" or "git shortlog --no-merges" may be good tools to use to
> find out who they are.

Thanks for the pointer!

+Cc Erik Faye-Lund <kusmabite@gmail.com>

The reason I asked is that I'm documenting similar_enough() for
subcommand autocorrection in this series. It allows "lis" to correct to
"list", but avoids "ad" to "add".

I noticed that the command autocorrection ignores prefix-matched strings
entirely (e.g., not correcting "commi" to "commit", as introduced in
commit 6612b9e471a3). I wanted to understand the rationale behind this
to see if subcommands should align with that design.

Thanks.

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>

Re: [PATCH 5/5] help: add tests for subcommand autocorrection

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> On Wed, Mar 11, 2026 at 10:01:55AM -0700, Aaron Plattner wrote:
>> t/meson.build:1193:6: ERROR: Problem encountered: Test files found, but not
>> configured:
>> 
>>  - t9004-autocorrect-subcommand.sh
>> 
>> 
>> I think you just need to add it to meson.build:
>
> Will add it, thanks!

In the meantime, I have the following as a fixup patch on top of the
series.  The 'seen' branch with the topic still seems to be failing
with meson build in my environment, but I didn't have time to dig
further.

diff --git a/meson.build b/meson.build
index 4b536e0124..0429e80a5c 100644
--- a/meson.build
+++ b/meson.build
@@ -283,6 +283,7 @@ libgit_sources = [
   'archive-zip.c',
   'archive.c',
   'attr.c',
+  'autocorrect.c',
   'base85.c',
   'bisect.c',
   'blame.c',
diff --git a/t/meson.build b/t/meson.build
index f66a73f8a0..bf0503d705 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -973,6 +973,7 @@ integration_tests = [
   't9001-send-email.sh',
   't9002-column.sh',
   't9003-help-autocorrect.sh',
+  't9004-autocorrect-subcommand.sh',
   't9100-git-svn-basic.sh',
   't9101-git-svn-props.sh',
   't9102-git-svn-deep-rmdir.sh',

[PATCH v4 00/10] parseopt: add subcommand autocorrection

[Jiamu Sun]

Git currently provides auto-correction for builtins and aliases, but
lacks this functionality for subcommands parsed via the parse-options
API. Subcommands are also commands, and typos will occur, too. Like:

	git remote add-rul

So, this series introduces subcommand auto-correction.

By default, this implementation enables autocorrection for builtins
with mandatory subcommands. However, for those using
PARSE_OPT_SUBCOMMAND_OPTIONAL, autocorrection is skipped to avoid
misinterpreting legitimate unknown arguments as mistyped subcommands.

To allow builtins with optional subcommands to explicitly opt in,
this series adds the PARSE_OPT_SUBCOMMAND_AUTOCORR flag, and enables
it for git-remote and git-notes.

Additionally, the existing autocorrection logic is extracted from
help.c so subcommand handling can reuse the same config parsing and
prompt/delay logic.

Some split string literals are also combined so the full text is easier
to grep for.

Changes in v4:
  - Add missing files to Meson build
  - Change API prefix from autocorr to autocorrect
  - Split the commit that moves tty code
  - Add API documentation
  - Use standard Damerau-Levenshtein distance and common practice
    fuzziness thresholds
  - Rename AUTOCORRECT_HINTONLY to AUTOCORRECT_HINT
  - Change commit subject prefix for tests from "help:" to "parseopt:"
  - Fix coding style issues

Note on the autocorrection thresholds:
In v3, there was a discussion about why specific thresholds were chosen.
While attempting to document the legacy magic penalties (0, 2, 1, 3), I
realized those weights exist in a system where prefix matches are
completely ignored (due to a historical side-effect). Applying them to
subcommands (which do evaluate prefix matches) makes the tolerance
overly permissive. Therefore, v4 abandons the legacy weights in favor of
a standard Damerau-Levenshtein distance and common practice length-based
thresholds.

Changes in v3:
  - Align with the coding guildline
  - Split patch so diffs don't get hidden by code movement
  - Improve commit messages

Changes in v2:
  - Reword the explanation of default autocorrection behavior

Jiamu Sun (10):
  parseopt: extract subcommand handling from parse_options_step()
  help: make autocorrect handling reusable
  help: move tty check for autocorrection to autocorrect.c
  autocorrect: use mode and delay instead of magic numbers
  autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINT
  autocorrect: provide config resolution API
  parseopt: autocorrect mistyped subcommands
  parseopt: enable subcommand autocorrection for git-remote and
    git-notes
  parseopt: add tests for subcommand autocorrection
  doc: document autocorrect API

 Makefile                          |   1 +
 autocorrect.c                     |  89 ++++++++++++++++
 autocorrect.h                     |  32 ++++++
 builtin/notes.c                   |  10 +-
 builtin/remote.c                  |  12 +--
 help.c                            | 107 ++++----------------
 meson.build                       |   1 +
 parse-options.c                   | 162 ++++++++++++++++++++++--------
 parse-options.h                   |   1 +
 t/meson.build                     |   1 +
 t/t9004-autocorrect-subcommand.sh |  51 ++++++++++
 11 files changed, 325 insertions(+), 142 deletions(-)
 create mode 100644 autocorrect.c
 create mode 100644 autocorrect.h
 create mode 100755 t/t9004-autocorrect-subcommand.sh


base-commit: 795c338de725e13bd361214c6b768019fc45a2c1
-- 
2.53.0

[PATCH v4 01/10] parseopt: extract subcommand handling from parse_options_step()

[Jiamu Sun]

Move the subcommand branch out of parse_options_step() into a new
handle_subcommand() helper. Also, make parse_subcommand() return a
simple success/failure status.

This removes the switch over impossible parse_opt_result values and
makes the non-option path easier to follow and maintain.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 parse-options.c | 87 ++++++++++++++++++++++++++-----------------------
 1 file changed, 46 insertions(+), 41 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index c9cafc21b903..02a4f00919f6 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -605,17 +605,44 @@ static enum parse_opt_result parse_nodash_opt(struct parse_opt_ctx_t *p,
 	return PARSE_OPT_ERROR;
 }
 
-static enum parse_opt_result parse_subcommand(const char *arg,
-					      const struct option *options)
+static int parse_subcommand(const char *arg, const struct option *options)
 {
-	for (; options->type != OPTION_END; options++)
-		if (options->type == OPTION_SUBCOMMAND &&
-		    !strcmp(options->long_name, arg)) {
-			*(parse_opt_subcommand_fn **)options->value = options->subcommand_fn;
-			return PARSE_OPT_SUBCOMMAND;
-		}
+	for (; options->type != OPTION_END; options++) {
+		parse_opt_subcommand_fn **opt_val;
 
-	return PARSE_OPT_UNKNOWN;
+		if (options->type != OPTION_SUBCOMMAND ||
+		    strcmp(options->long_name, arg))
+			continue;
+
+		opt_val = options->value;
+		*opt_val = options->subcommand_fn;
+		return 0;
+	}
+
+	return -1;
+}
+
+static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
+					       const char *arg,
+					       const struct option *options,
+					       const char * const usagestr[])
+{
+	int err = parse_subcommand(arg, options);
+
+	if (!err)
+		return PARSE_OPT_SUBCOMMAND;
+
+	/*
+	 * arg is neither a short or long option nor a subcommand.  Since this
+	 * command has a default operation mode, we have to treat this arg and
+	 * all remaining args as args meant to that default operation mode.
+	 * So we are done parsing.
+	 */
+	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
+		return PARSE_OPT_DONE;
+
+	error(_("unknown subcommand: `%s'"), arg);
+	usage_with_options(usagestr, options);
 }
 
 static void check_typos(const char *arg, const struct option *options)
@@ -990,38 +1017,16 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
 		if (*arg != '-' || !arg[1]) {
 			if (parse_nodash_opt(ctx, arg, options) == 0)
 				continue;
-			if (!ctx->has_subcommands) {
-				if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
-					return PARSE_OPT_NON_OPTION;
-				ctx->out[ctx->cpidx++] = ctx->argv[0];
-				continue;
-			}
-			switch (parse_subcommand(arg, options)) {
-			case PARSE_OPT_SUBCOMMAND:
-				return PARSE_OPT_SUBCOMMAND;
-			case PARSE_OPT_UNKNOWN:
-				if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
-					/*
-					 * arg is neither a short or long
-					 * option nor a subcommand.  Since
-					 * this command has a default
-					 * operation mode, we have to treat
-					 * this arg and all remaining args
-					 * as args meant to that default
-					 * operation mode.
-					 * So we are done parsing.
-					 */
-					return PARSE_OPT_DONE;
-				error(_("unknown subcommand: `%s'"), arg);
-				usage_with_options(usagestr, options);
-			case PARSE_OPT_COMPLETE:
-			case PARSE_OPT_HELP:
-			case PARSE_OPT_ERROR:
-			case PARSE_OPT_DONE:
-			case PARSE_OPT_NON_OPTION:
-				/* Impossible. */
-				BUG("parse_subcommand() cannot return these");
-			}
+
+			if (ctx->has_subcommands)
+				return handle_subcommand(ctx, arg, options,
+							 usagestr);
+
+			if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
+				return PARSE_OPT_NON_OPTION;
+
+			ctx->out[ctx->cpidx++] = ctx->argv[0];
+			continue;
 		}
 
 		/* lone -h asks for help */
-- 
2.53.0

[PATCH v4 02/10] help: make autocorrect handling reusable

[Jiamu Sun]

Move config parsing and prompt/delay handling into autocorrect.c and
expose them in autocorrect.h. This makes autocorrect reusable regardless
of which target links against it.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 Makefile      |  1 +
 autocorrect.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
 autocorrect.h | 16 ++++++++++++
 help.c        | 64 +++------------------------------------------
 meson.build   |  1 +
 5 files changed, 94 insertions(+), 60 deletions(-)
 create mode 100644 autocorrect.c
 create mode 100644 autocorrect.h

diff --git a/Makefile b/Makefile
index f3264d0a37cc..6111631c2caa 100644
--- a/Makefile
+++ b/Makefile
@@ -1098,6 +1098,7 @@ LIB_OBJS += archive-tar.o
 LIB_OBJS += archive-zip.o
 LIB_OBJS += archive.o
 LIB_OBJS += attr.o
+LIB_OBJS += autocorrect.o
 LIB_OBJS += base85.o
 LIB_OBJS += bisect.o
 LIB_OBJS += blame.o
diff --git a/autocorrect.c b/autocorrect.c
new file mode 100644
index 000000000000..97145d3a53ce
--- /dev/null
+++ b/autocorrect.c
@@ -0,0 +1,72 @@
+#include "git-compat-util.h"
+#include "autocorrect.h"
+#include "config.h"
+#include "parse.h"
+#include "strbuf.h"
+#include "prompt.h"
+#include "gettext.h"
+
+static int parse_autocorrect(const char *value)
+{
+	switch (git_parse_maybe_bool_text(value)) {
+	case 1:
+		return AUTOCORRECT_IMMEDIATELY;
+	case 0:
+		return AUTOCORRECT_SHOW;
+	default: /* other random text */
+		break;
+	}
+
+	if (!strcmp(value, "prompt"))
+		return AUTOCORRECT_PROMPT;
+	if (!strcmp(value, "never"))
+		return AUTOCORRECT_NEVER;
+	if (!strcmp(value, "immediate"))
+		return AUTOCORRECT_IMMEDIATELY;
+	if (!strcmp(value, "show"))
+		return AUTOCORRECT_SHOW;
+
+	return 0;
+}
+
+void autocorrect_resolve_config(const char *var, const char *value,
+				const struct config_context *ctx, void *data)
+{
+	int *out = data;
+
+	if (!strcmp(var, "help.autocorrect")) {
+		int v = parse_autocorrect(value);
+
+		if (!v) {
+			v = git_config_int(var, value, ctx->kvi);
+			if (v < 0 || v == 1)
+				v = AUTOCORRECT_IMMEDIATELY;
+		}
+
+		*out = v;
+	}
+}
+
+void autocorrect_confirm(int autocorrect, const char *assumed)
+{
+	if (autocorrect == AUTOCORRECT_IMMEDIATELY) {
+		fprintf_ln(stderr,
+			   _("Continuing under the assumption that you meant '%s'."),
+			   assumed);
+	} else if (autocorrect == AUTOCORRECT_PROMPT) {
+		char *answer;
+		struct strbuf msg = STRBUF_INIT;
+
+		strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
+		answer = git_prompt(msg.buf, PROMPT_ECHO);
+		strbuf_release(&msg);
+
+		if (!(starts_with(answer, "y") || starts_with(answer, "Y")))
+			exit(1);
+	} else {
+		fprintf_ln(stderr,
+			   _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
+			   (float)autocorrect / 10.0, assumed);
+		sleep_millisec(autocorrect * 100);
+	}
+}
diff --git a/autocorrect.h b/autocorrect.h
new file mode 100644
index 000000000000..f5fadf9d9605
--- /dev/null
+++ b/autocorrect.h
@@ -0,0 +1,16 @@
+#ifndef AUTOCORRECT_H
+#define AUTOCORRECT_H
+
+#define AUTOCORRECT_SHOW (-4)
+#define AUTOCORRECT_PROMPT (-3)
+#define AUTOCORRECT_NEVER (-2)
+#define AUTOCORRECT_IMMEDIATELY (-1)
+
+struct config_context;
+
+void autocorrect_resolve_config(const char *var, const char *value,
+				const struct config_context *ctx, void *data);
+
+void autocorrect_confirm(int autocorrect, const char *assumed);
+
+#endif /* AUTOCORRECT_H */
diff --git a/help.c b/help.c
index 95f576c5c81d..4acb6ca585ff 100644
--- a/help.c
+++ b/help.c
@@ -22,6 +22,7 @@
 #include "repository.h"
 #include "alias.h"
 #include "utf8.h"
+#include "autocorrect.h"
 
 #ifndef NO_CURL
 #include "git-curl-compat.h" /* For LIBCURL_VERSION only */
@@ -541,34 +542,6 @@ struct help_unknown_cmd_config {
 	struct cmdnames aliases;
 };
 
-#define AUTOCORRECT_SHOW (-4)
-#define AUTOCORRECT_PROMPT (-3)
-#define AUTOCORRECT_NEVER (-2)
-#define AUTOCORRECT_IMMEDIATELY (-1)
-
-static int parse_autocorrect(const char *value)
-{
-	switch (git_parse_maybe_bool_text(value)) {
-		case 1:
-			return AUTOCORRECT_IMMEDIATELY;
-		case 0:
-			return AUTOCORRECT_SHOW;
-		default: /* other random text */
-			break;
-	}
-
-	if (!strcmp(value, "prompt"))
-		return AUTOCORRECT_PROMPT;
-	if (!strcmp(value, "never"))
-		return AUTOCORRECT_NEVER;
-	if (!strcmp(value, "immediate"))
-		return AUTOCORRECT_IMMEDIATELY;
-	if (!strcmp(value, "show"))
-		return AUTOCORRECT_SHOW;
-
-	return 0;
-}
-
 static int git_unknown_cmd_config(const char *var, const char *value,
 				  const struct config_context *ctx,
 				  void *cb)
@@ -577,17 +550,7 @@ static int git_unknown_cmd_config(const char *var, const char *value,
 	const char *subsection, *key;
 	size_t subsection_len;
 
-	if (!strcmp(var, "help.autocorrect")) {
-		int v = parse_autocorrect(value);
-
-		if (!v) {
-			v = git_config_int(var, value, ctx->kvi);
-			if (v < 0 || v == 1)
-				v = AUTOCORRECT_IMMEDIATELY;
-		}
-
-		cfg->autocorrect = v;
-	}
+	autocorrect_resolve_config(var, value, ctx, &cfg->autocorrect);
 
 	/* Also use aliases for command lookup */
 	if (!parse_config_key(var, "alias", &subsection, &subsection_len,
@@ -724,27 +687,8 @@ char *help_unknown_cmd(const char *cmd)
 			   _("WARNING: You called a Git command named '%s', "
 			     "which does not exist."),
 			   cmd);
-		if (cfg.autocorrect == AUTOCORRECT_IMMEDIATELY)
-			fprintf_ln(stderr,
-				   _("Continuing under the assumption that "
-				     "you meant '%s'."),
-				   assumed);
-		else if (cfg.autocorrect == AUTOCORRECT_PROMPT) {
-			char *answer;
-			struct strbuf msg = STRBUF_INIT;
-			strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
-			answer = git_prompt(msg.buf, PROMPT_ECHO);
-			strbuf_release(&msg);
-			if (!(starts_with(answer, "y") ||
-			      starts_with(answer, "Y")))
-				exit(1);
-		} else {
-			fprintf_ln(stderr,
-				   _("Continuing in %0.1f seconds, "
-				     "assuming that you meant '%s'."),
-				   (float)cfg.autocorrect/10.0, assumed);
-			sleep_millisec(cfg.autocorrect * 100);
-		}
+
+		autocorrect_confirm(cfg.autocorrect, assumed);
 
 		cmdnames_release(&cfg.aliases);
 		cmdnames_release(&main_cmds);
diff --git a/meson.build b/meson.build
index 4b536e012481..0429e80a5c96 100644
--- a/meson.build
+++ b/meson.build
@@ -283,6 +283,7 @@ libgit_sources = [
   'archive-zip.c',
   'archive.c',
   'attr.c',
+  'autocorrect.c',
   'base85.c',
   'bisect.c',
   'blame.c',
-- 
2.53.0

[PATCH v4 03/10] help: move tty check for autocorrection to autocorrect.c

[Jiamu Sun]

TTY checking is the autocorrect config parser's responsibility. It must
ensure the parsed value is correct and reliable. Thus, move the check to
autocorrect_resolve_config().

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 autocorrect.c | 24 ++++++++++++++++--------
 help.c        |  6 ------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/autocorrect.c b/autocorrect.c
index 97145d3a53ce..887d2396da44 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -33,18 +33,26 @@ void autocorrect_resolve_config(const char *var, const char *value,
 				const struct config_context *ctx, void *data)
 {
 	int *out = data;
+	int parsed;
 
-	if (!strcmp(var, "help.autocorrect")) {
-		int v = parse_autocorrect(value);
+	if (strcmp(var, "help.autocorrect"))
+		return;
 
-		if (!v) {
-			v = git_config_int(var, value, ctx->kvi);
-			if (v < 0 || v == 1)
-				v = AUTOCORRECT_IMMEDIATELY;
-		}
+	parsed = parse_autocorrect(value);
 
-		*out = v;
+	/*
+	 * Disable autocorrection prompt in a non-interactive session
+	 */
+	if (parsed == AUTOCORRECT_PROMPT && (!isatty(0) || !isatty(2)))
+		parsed = AUTOCORRECT_NEVER;
+
+	if (!parsed) {
+		parsed = git_config_int(var, value, ctx->kvi);
+		if (parsed < 0 || parsed == 1)
+			parsed = AUTOCORRECT_IMMEDIATELY;
 	}
+
+	*out = parsed;
 }
 
 void autocorrect_confirm(int autocorrect, const char *assumed)
diff --git a/help.c b/help.c
index 4acb6ca585ff..983057970e7c 100644
--- a/help.c
+++ b/help.c
@@ -607,12 +607,6 @@ char *help_unknown_cmd(const char *cmd)
 
 	read_early_config(the_repository, git_unknown_cmd_config, &cfg);
 
-	/*
-	 * Disable autocorrection prompt in a non-interactive session
-	 */
-	if ((cfg.autocorrect == AUTOCORRECT_PROMPT) && (!isatty(0) || !isatty(2)))
-		cfg.autocorrect = AUTOCORRECT_NEVER;
-
 	if (cfg.autocorrect == AUTOCORRECT_NEVER) {
 		fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
 		exit(1);
-- 
2.53.0

[PATCH v4 04/10] autocorrect: use mode and delay instead of magic numbers

[Jiamu Sun]

Drop magic numbers and describe autocorrect config with a mode enum and
an integer delay. This reduces errors when mutating config values and
makes the values easier to access.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 autocorrect.c | 46 +++++++++++++++++++++++-----------------------
 autocorrect.h | 20 ++++++++++++++------
 help.c        |  9 +++++----
 3 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/autocorrect.c b/autocorrect.c
index 887d2396da44..2484546fc731 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -6,7 +6,7 @@
 #include "prompt.h"
 #include "gettext.h"
 
-static int parse_autocorrect(const char *value)
+static enum autocorrect_mode parse_autocorrect(const char *value)
 {
 	switch (git_parse_maybe_bool_text(value)) {
 	case 1:
@@ -19,49 +19,49 @@ static int parse_autocorrect(const char *value)
 
 	if (!strcmp(value, "prompt"))
 		return AUTOCORRECT_PROMPT;
-	if (!strcmp(value, "never"))
+	else if (!strcmp(value, "never"))
 		return AUTOCORRECT_NEVER;
-	if (!strcmp(value, "immediate"))
+	else if (!strcmp(value, "immediate"))
 		return AUTOCORRECT_IMMEDIATELY;
-	if (!strcmp(value, "show"))
+	else if (!strcmp(value, "show"))
 		return AUTOCORRECT_SHOW;
-
-	return 0;
+	else
+		return AUTOCORRECT_DELAY;
 }
 
 void autocorrect_resolve_config(const char *var, const char *value,
 				const struct config_context *ctx, void *data)
 {
-	int *out = data;
-	int parsed;
+	struct autocorrect *conf = data;
 
 	if (strcmp(var, "help.autocorrect"))
 		return;
 
-	parsed = parse_autocorrect(value);
+	conf->mode = parse_autocorrect(value);
 
 	/*
 	 * Disable autocorrection prompt in a non-interactive session
 	 */
-	if (parsed == AUTOCORRECT_PROMPT && (!isatty(0) || !isatty(2)))
-		parsed = AUTOCORRECT_NEVER;
+	if (conf->mode == AUTOCORRECT_PROMPT && (!isatty(0) || !isatty(2)))
+		conf->mode = AUTOCORRECT_NEVER;
 
-	if (!parsed) {
-		parsed = git_config_int(var, value, ctx->kvi);
-		if (parsed < 0 || parsed == 1)
-			parsed = AUTOCORRECT_IMMEDIATELY;
-	}
+	if (conf->mode == AUTOCORRECT_DELAY) {
+		conf->delay = git_config_int(var, value, ctx->kvi);
 
-	*out = parsed;
+		if (!conf->delay)
+			conf->mode = AUTOCORRECT_SHOW;
+		else if (conf->delay < 0 || conf->delay == 1)
+			conf->mode = AUTOCORRECT_IMMEDIATELY;
+	}
 }
 
-void autocorrect_confirm(int autocorrect, const char *assumed)
+void autocorrect_confirm(struct autocorrect *conf, const char *assumed)
 {
-	if (autocorrect == AUTOCORRECT_IMMEDIATELY) {
+	if (conf->mode == AUTOCORRECT_IMMEDIATELY) {
 		fprintf_ln(stderr,
 			   _("Continuing under the assumption that you meant '%s'."),
 			   assumed);
-	} else if (autocorrect == AUTOCORRECT_PROMPT) {
+	} else if (conf->mode == AUTOCORRECT_PROMPT) {
 		char *answer;
 		struct strbuf msg = STRBUF_INIT;
 
@@ -71,10 +71,10 @@ void autocorrect_confirm(int autocorrect, const char *assumed)
 
 		if (!(starts_with(answer, "y") || starts_with(answer, "Y")))
 			exit(1);
-	} else {
+	} else if (conf->mode == AUTOCORRECT_DELAY) {
 		fprintf_ln(stderr,
 			   _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
-			   (float)autocorrect / 10.0, assumed);
-		sleep_millisec(autocorrect * 100);
+			   conf->delay / 10.0, assumed);
+		sleep_millisec(conf->delay * 100);
 	}
 }
diff --git a/autocorrect.h b/autocorrect.h
index f5fadf9d9605..5506a36f11a7 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -1,16 +1,24 @@
 #ifndef AUTOCORRECT_H
 #define AUTOCORRECT_H
 
-#define AUTOCORRECT_SHOW (-4)
-#define AUTOCORRECT_PROMPT (-3)
-#define AUTOCORRECT_NEVER (-2)
-#define AUTOCORRECT_IMMEDIATELY (-1)
-
 struct config_context;
 
+enum autocorrect_mode {
+	AUTOCORRECT_SHOW,
+	AUTOCORRECT_NEVER,
+	AUTOCORRECT_PROMPT,
+	AUTOCORRECT_IMMEDIATELY,
+	AUTOCORRECT_DELAY,
+};
+
+struct autocorrect {
+	enum autocorrect_mode mode;
+	int delay;
+};
+
 void autocorrect_resolve_config(const char *var, const char *value,
 				const struct config_context *ctx, void *data);
 
-void autocorrect_confirm(int autocorrect, const char *assumed);
+void autocorrect_confirm(struct autocorrect *conf, const char *assumed);
 
 #endif /* AUTOCORRECT_H */
diff --git a/help.c b/help.c
index 983057970e7c..a89ac5aced99 100644
--- a/help.c
+++ b/help.c
@@ -538,7 +538,7 @@ int is_in_cmdlist(struct cmdnames *c, const char *s)
 }
 
 struct help_unknown_cmd_config {
-	int autocorrect;
+	struct autocorrect autocorrect;
 	struct cmdnames aliases;
 };
 
@@ -607,7 +607,7 @@ char *help_unknown_cmd(const char *cmd)
 
 	read_early_config(the_repository, git_unknown_cmd_config, &cfg);
 
-	if (cfg.autocorrect == AUTOCORRECT_NEVER) {
+	if (cfg.autocorrect.mode == AUTOCORRECT_NEVER) {
 		fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
 		exit(1);
 	}
@@ -673,7 +673,8 @@ char *help_unknown_cmd(const char *cmd)
 		     n++)
 			; /* still counting */
 	}
-	if (cfg.autocorrect && cfg.autocorrect != AUTOCORRECT_SHOW && n == 1 &&
+
+	if (cfg.autocorrect.mode != AUTOCORRECT_SHOW && n == 1 &&
 	    SIMILAR_ENOUGH(best_similarity)) {
 		char *assumed = xstrdup(main_cmds.names[0]->name);
 
@@ -682,7 +683,7 @@ char *help_unknown_cmd(const char *cmd)
 			     "which does not exist."),
 			   cmd);
 
-		autocorrect_confirm(cfg.autocorrect, assumed);
+		autocorrect_confirm(&cfg.autocorrect, assumed);
 
 		cmdnames_release(&cfg.aliases);
 		cmdnames_release(&main_cmds);
-- 
2.53.0

[PATCH v4 05/10] autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINT

[Jiamu Sun]

AUTOCORRECT_SHOW is ambiguous. Its purpose is to show commands similar
to the unknown one and take no other action. Rename it to fit the
semantics.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 autocorrect.c | 6 +++---
 autocorrect.h | 2 +-
 help.c        | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/autocorrect.c b/autocorrect.c
index 2484546fc731..de0fa282c934 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -12,7 +12,7 @@ static enum autocorrect_mode parse_autocorrect(const char *value)
 	case 1:
 		return AUTOCORRECT_IMMEDIATELY;
 	case 0:
-		return AUTOCORRECT_SHOW;
+		return AUTOCORRECT_HINT;
 	default: /* other random text */
 		break;
 	}
@@ -24,7 +24,7 @@ static enum autocorrect_mode parse_autocorrect(const char *value)
 	else if (!strcmp(value, "immediate"))
 		return AUTOCORRECT_IMMEDIATELY;
 	else if (!strcmp(value, "show"))
-		return AUTOCORRECT_SHOW;
+		return AUTOCORRECT_HINT;
 	else
 		return AUTOCORRECT_DELAY;
 }
@@ -49,7 +49,7 @@ void autocorrect_resolve_config(const char *var, const char *value,
 		conf->delay = git_config_int(var, value, ctx->kvi);
 
 		if (!conf->delay)
-			conf->mode = AUTOCORRECT_SHOW;
+			conf->mode = AUTOCORRECT_HINT;
 		else if (conf->delay < 0 || conf->delay == 1)
 			conf->mode = AUTOCORRECT_IMMEDIATELY;
 	}
diff --git a/autocorrect.h b/autocorrect.h
index 5506a36f11a7..328807242c15 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -4,7 +4,7 @@
 struct config_context;
 
 enum autocorrect_mode {
-	AUTOCORRECT_SHOW,
+	AUTOCORRECT_HINT,
 	AUTOCORRECT_NEVER,
 	AUTOCORRECT_PROMPT,
 	AUTOCORRECT_IMMEDIATELY,
diff --git a/help.c b/help.c
index a89ac5aced99..2d441ded3f14 100644
--- a/help.c
+++ b/help.c
@@ -674,7 +674,7 @@ char *help_unknown_cmd(const char *cmd)
 			; /* still counting */
 	}
 
-	if (cfg.autocorrect.mode != AUTOCORRECT_SHOW && n == 1 &&
+	if (cfg.autocorrect.mode != AUTOCORRECT_HINT && n == 1 &&
 	    SIMILAR_ENOUGH(best_similarity)) {
 		char *assumed = xstrdup(main_cmds.names[0]->name);
 
-- 
2.53.0

[PATCH v4 06/10] autocorrect: provide config resolution API

[Jiamu Sun]

Add autocorrect_resolve(). This resolves and populates the correct
values for autocorrect config.

Make autocorrect config callback internal. The API is meant to provide
a high-level way to retrieve the config. Allowing access to the config
callback from outside violates that intent.

Additionally, in some cases, without access to the config callback, two
config iterations cannot be merged into one, which can hurt performance.
This is fine, as the code path that calls autocorrect_resolve() is cold.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 autocorrect.c | 15 ++++++++++++---
 autocorrect.h |  5 +----
 help.c        | 40 +++++++++++++++++-----------------------
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/autocorrect.c b/autocorrect.c
index de0fa282c934..b2ee9f51e8c0 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "autocorrect.h"
 #include "config.h"
@@ -29,13 +31,13 @@ static enum autocorrect_mode parse_autocorrect(const char *value)
 		return AUTOCORRECT_DELAY;
 }
 
-void autocorrect_resolve_config(const char *var, const char *value,
-				const struct config_context *ctx, void *data)
+static int resolve_autocorrect(const char *var, const char *value,
+			       const struct config_context *ctx, void *data)
 {
 	struct autocorrect *conf = data;
 
 	if (strcmp(var, "help.autocorrect"))
-		return;
+		return 0;
 
 	conf->mode = parse_autocorrect(value);
 
@@ -53,6 +55,13 @@ void autocorrect_resolve_config(const char *var, const char *value,
 		else if (conf->delay < 0 || conf->delay == 1)
 			conf->mode = AUTOCORRECT_IMMEDIATELY;
 	}
+
+	return 0;
+}
+
+void autocorrect_resolve(struct autocorrect *conf)
+{
+	read_early_config(the_repository, resolve_autocorrect, conf);
 }
 
 void autocorrect_confirm(struct autocorrect *conf, const char *assumed)
diff --git a/autocorrect.h b/autocorrect.h
index 328807242c15..0d3e819262ed 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -1,8 +1,6 @@
 #ifndef AUTOCORRECT_H
 #define AUTOCORRECT_H
 
-struct config_context;
-
 enum autocorrect_mode {
 	AUTOCORRECT_HINT,
 	AUTOCORRECT_NEVER,
@@ -16,8 +14,7 @@ struct autocorrect {
 	int delay;
 };
 
-void autocorrect_resolve_config(const char *var, const char *value,
-				const struct config_context *ctx, void *data);
+void autocorrect_resolve(struct autocorrect *conf);
 
 void autocorrect_confirm(struct autocorrect *conf, const char *assumed);
 
diff --git a/help.c b/help.c
index 2d441ded3f14..81efdb13d4a3 100644
--- a/help.c
+++ b/help.c
@@ -537,32 +537,23 @@ int is_in_cmdlist(struct cmdnames *c, const char *s)
 	return 0;
 }
 
-struct help_unknown_cmd_config {
-	struct autocorrect autocorrect;
-	struct cmdnames aliases;
-};
-
-static int git_unknown_cmd_config(const char *var, const char *value,
-				  const struct config_context *ctx,
-				  void *cb)
+static int resolve_aliases(const char *var, const char *value UNUSED,
+			   const struct config_context *ctx UNUSED, void *data)
 {
-	struct help_unknown_cmd_config *cfg = cb;
+	struct cmdnames *aliases = data;
 	const char *subsection, *key;
 	size_t subsection_len;
 
-	autocorrect_resolve_config(var, value, ctx, &cfg->autocorrect);
-
-	/* Also use aliases for command lookup */
 	if (!parse_config_key(var, "alias", &subsection, &subsection_len,
 			      &key)) {
 		if (subsection) {
 			/* [alias "name"] command = value */
 			if (!strcmp(key, "command"))
-				add_cmdname(&cfg->aliases, subsection,
+				add_cmdname(aliases, subsection,
 					    subsection_len);
 		} else {
 			/* alias.name = value */
-			add_cmdname(&cfg->aliases, key, strlen(key));
+			add_cmdname(aliases, key, strlen(key));
 		}
 	}
 
@@ -599,22 +590,26 @@ static const char bad_interpreter_advice[] =
 
 char *help_unknown_cmd(const char *cmd)
 {
-	struct help_unknown_cmd_config cfg = { 0 };
+	struct cmdnames aliases = { 0 };
+	struct autocorrect autocorrect = { 0 };
 	int i, n, best_similarity = 0;
 	struct cmdnames main_cmds = { 0 };
 	struct cmdnames other_cmds = { 0 };
 	struct cmdname_help *common_cmds;
 
-	read_early_config(the_repository, git_unknown_cmd_config, &cfg);
+	autocorrect_resolve(&autocorrect);
 
-	if (cfg.autocorrect.mode == AUTOCORRECT_NEVER) {
+	if (autocorrect.mode == AUTOCORRECT_NEVER) {
 		fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
 		exit(1);
 	}
 
 	load_command_list("git-", &main_cmds, &other_cmds);
 
-	add_cmd_list(&main_cmds, &cfg.aliases);
+	/* Also use aliases for command lookup */
+	read_early_config(the_repository, resolve_aliases, &aliases);
+
+	add_cmd_list(&main_cmds, &aliases);
 	add_cmd_list(&main_cmds, &other_cmds);
 	QSORT(main_cmds.names, main_cmds.cnt, cmdname_compare);
 	uniq(&main_cmds);
@@ -674,18 +669,17 @@ char *help_unknown_cmd(const char *cmd)
 			; /* still counting */
 	}
 
-	if (cfg.autocorrect.mode != AUTOCORRECT_HINT && n == 1 &&
+	if (autocorrect.mode != AUTOCORRECT_HINT && n == 1 &&
 	    SIMILAR_ENOUGH(best_similarity)) {
 		char *assumed = xstrdup(main_cmds.names[0]->name);
 
 		fprintf_ln(stderr,
-			   _("WARNING: You called a Git command named '%s', "
-			     "which does not exist."),
+			   _("WARNING: You called a Git command named '%s', which does not exist."),
 			   cmd);
 
-		autocorrect_confirm(&cfg.autocorrect, assumed);
+		autocorrect_confirm(&autocorrect, assumed);
 
-		cmdnames_release(&cfg.aliases);
+		cmdnames_release(&aliases);
 		cmdnames_release(&main_cmds);
 		cmdnames_release(&other_cmds);
 		return assumed;
-- 
2.53.0

[PATCH v4 07/10] parseopt: autocorrect mistyped subcommands

[Jiamu Sun]

Try to autocorrect the mistyped mandatory subcommand before showing an
error and exiting. Subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL
are skipped.

Use standard Damerau-Levenshtein distance (weights 1, 1, 1, 1) to
establish a predictable, mathematically sound baseline.

Scale the allowed edit distance based on input length to prevent
false positives on short commands, following common practice for
fuzziness thresholds (e.g., Elasticsearch's AUTO fuzziness):
  - Length 0-2: 0 edits allowed
  - Length 3-5: 1 edit allowed
  - Length 6+:  2 edits allowed

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 parse-options.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 3 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 02a4f00919f6..1f1b72762790 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -6,6 +6,8 @@
 #include "strbuf.h"
 #include "string-list.h"
 #include "utf8.h"
+#include "autocorrect.h"
+#include "levenshtein.h"
 
 static int disallow_abbreviated_options;
 
@@ -622,13 +624,77 @@ static int parse_subcommand(const char *arg, const struct option *options)
 	return -1;
 }
 
+static void find_subcommands(struct string_list *list,
+			     const struct option *options)
+{
+	for (; options->type != OPTION_END; options++) {
+		if (options->type == OPTION_SUBCOMMAND)
+			string_list_append(list, options->long_name);
+	}
+}
+
+static int similar_enough(const char *cmd, unsigned int edit)
+{
+	size_t len = strlen(cmd);
+	unsigned int allowed = len < 3 ? 0 : len < 6 ? 1 : 2;
+
+	return edit <= allowed;
+}
+
+static const char *autocorrect_subcommand(const char *cmd,
+					  struct string_list *cmds)
+{
+	struct autocorrect autocorrect = { 0 };
+	unsigned int min = UINT_MAX;
+	unsigned int ties = 0;
+	struct string_list_item *cand;
+	struct string_list_item *best = NULL;
+
+	autocorrect_resolve(&autocorrect);
+
+	/*
+	 * Builtin subcommands are small enough that printing them all via
+	 * usage_with_options() is sufficient. Therefore, AUTOCORRECT_HINT
+	 * acts like AUTOCORRECT_NEVER.
+	 */
+	if (autocorrect.mode == AUTOCORRECT_HINT ||
+	    autocorrect.mode == AUTOCORRECT_NEVER)
+		return NULL;
+
+	for_each_string_list_item(cand, cmds) {
+		unsigned int edit = levenshtein(cmd, cand->string, 1, 1, 1, 1);
+
+		if (edit < min) {
+			min = edit;
+			best = cand;
+			ties = 0;
+		} else if (edit == min) {
+			ties++;
+		}
+	}
+
+	if (!ties && similar_enough(cmd, min)) {
+		fprintf_ln(stderr,
+			   _("WARNING: You called a subcommand named '%s', which does not exist."),
+			   cmd);
+
+		autocorrect_confirm(&autocorrect, best->string);
+		return best->string;
+	}
+
+	return NULL;
+}
+
 static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 					       const char *arg,
 					       const struct option *options,
 					       const char * const usagestr[])
 {
-	int err = parse_subcommand(arg, options);
+	int err;
+	const char *assumed;
+	struct string_list cmds = STRING_LIST_INIT_NODUP;
 
+	err = parse_subcommand(arg, options);
 	if (!err)
 		return PARSE_OPT_SUBCOMMAND;
 
@@ -641,8 +707,17 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
 		return PARSE_OPT_DONE;
 
-	error(_("unknown subcommand: `%s'"), arg);
-	usage_with_options(usagestr, options);
+	find_subcommands(&cmds, options);
+	assumed = autocorrect_subcommand(arg, &cmds);
+
+	if (!assumed) {
+		error(_("unknown subcommand: `%s'"), arg);
+		usage_with_options(usagestr, options);
+	}
+
+	string_list_clear(&cmds, 0);
+	parse_subcommand(assumed, options);
+	return PARSE_OPT_SUBCOMMAND;
 }
 
 static void check_typos(const char *arg, const struct option *options)
-- 
2.53.0

[PATCH v4 08/10] parseopt: enable subcommand autocorrection for git-remote and git-notes

[Jiamu Sun]

Add PARSE_OPT_SUBCOMMAND_AUTOCORR to enable autocorrection for
subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL.

Use it for git-remote and git-notes, so mistyped subcommands can be
automatically corrected, and builtin entry points no longer need to
handle the unknown subcommand error path themselves.

This is safe for these two builtins, because they either resolve to a
single subcommand or take no subcommand at all. This means that if the
subcommand parser encounters an unknown argument, it must be a mistyped
subcommand.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 builtin/notes.c  | 10 +++-------
 builtin/remote.c | 12 ++++--------
 parse-options.c  | 16 +++++++++-------
 parse-options.h  |  1 +
 4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 9af602bdd7b4..087eb898a441 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -1149,14 +1149,10 @@ int cmd_notes(int argc,
 
 	repo_config(the_repository, git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, git_notes_usage,
-			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
-	if (!fn) {
-		if (argc) {
-			error(_("unknown subcommand: `%s'"), argv[0]);
-			usage_with_options(git_notes_usage, options);
-		}
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL |
+			     PARSE_OPT_SUBCOMMAND_AUTOCORR);
+	if (!fn)
 		fn = list;
-	}
 
 	if (override_notes_ref) {
 		struct strbuf sb = STRBUF_INIT;
diff --git a/builtin/remote.c b/builtin/remote.c
index ace390c671d6..d1d6244a662a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1949,15 +1949,11 @@ int cmd_remote(int argc,
 	};
 
 	argc = parse_options(argc, argv, prefix, options, builtin_remote_usage,
-			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
+			     PARSE_OPT_SUBCOMMAND_OPTIONAL |
+			     PARSE_OPT_SUBCOMMAND_AUTOCORR);
 
-	if (fn) {
+	if (fn)
 		return !!fn(argc, argv, prefix, repo);
-	} else {
-		if (argc) {
-			error(_("unknown subcommand: `%s'"), argv[0]);
-			usage_with_options(builtin_remote_usage, options);
-		}
+	else
 		return !!show_all();
-	}
 }
diff --git a/parse-options.c b/parse-options.c
index 1f1b72762790..0b84061a3811 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -698,14 +698,16 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
 	if (!err)
 		return PARSE_OPT_SUBCOMMAND;
 
-	/*
-	 * arg is neither a short or long option nor a subcommand.  Since this
-	 * command has a default operation mode, we have to treat this arg and
-	 * all remaining args as args meant to that default operation mode.
-	 * So we are done parsing.
-	 */
-	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
+	if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL &&
+	    !(ctx->flags & PARSE_OPT_SUBCOMMAND_AUTOCORR)) {
+		/*
+		 * arg is neither a short or long option nor a subcommand.
+		 * Since this command has a default operation mode, we have to
+		 * treat this arg and all remaining args as args meant to that
+		 * default operation mode.  So we are done parsing.
+		 */
 		return PARSE_OPT_DONE;
+	}
 
 	find_subcommands(&cmds, options);
 	assumed = autocorrect_subcommand(arg, &cmds);
diff --git a/parse-options.h b/parse-options.h
index 706de9729f6b..f29ac337893c 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -40,6 +40,7 @@ enum parse_opt_flags {
 	PARSE_OPT_ONE_SHOT = 1 << 5,
 	PARSE_OPT_SHELL_EVAL = 1 << 6,
 	PARSE_OPT_SUBCOMMAND_OPTIONAL = 1 << 7,
+	PARSE_OPT_SUBCOMMAND_AUTOCORR = 1 << 8,
 };
 
 enum parse_opt_option_flags {
-- 
2.53.0

[PATCH v4 09/10] parseopt: add tests for subcommand autocorrection

[Jiamu Sun]

These tests cover default behavior (help.autocorrect is unset), no
correction, immediate correction, delayed correction, and rejection
when the typo is too dissimilar.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 t/meson.build                     |  1 +
 t/t9004-autocorrect-subcommand.sh | 51 +++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100755 t/t9004-autocorrect-subcommand.sh

diff --git a/t/meson.build b/t/meson.build
index 106c68df3d01..311be82af346 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -972,6 +972,7 @@ integration_tests = [
   't9001-send-email.sh',
   't9002-column.sh',
   't9003-help-autocorrect.sh',
+  't9004-autocorrect-subcommand.sh',
   't9100-git-svn-basic.sh',
   't9101-git-svn-props.sh',
   't9102-git-svn-deep-rmdir.sh',
diff --git a/t/t9004-autocorrect-subcommand.sh b/t/t9004-autocorrect-subcommand.sh
new file mode 100755
index 000000000000..d10031659b94
--- /dev/null
+++ b/t/t9004-autocorrect-subcommand.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+
+test_description='subcommand auto-correction test
+
+Test autocorrection for subcommands with different
+help.autocorrect mode.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' "
+	echo '^error: unknown subcommand: ' >grep_unknown
+"
+
+test_expect_success 'default is not to autocorrect' '
+	test_must_fail git worktree lsit 2>actual &&
+	head -n1 actual >first && test_grep -f grep_unknown first
+'
+
+for mode in false no off 0 show never
+do
+	test_expect_success "'$mode' disables autocorrection" "
+		test_config help.autocorrect $mode &&
+
+		test_must_fail git worktree lsit 2>actual &&
+		head -n1 actual >first && test_grep -f grep_unknown first
+	"
+done
+
+for mode in -39 immediate 1
+do
+	test_expect_success "autocorrect immediately with '$mode'" - <<-EOT
+		test_config help.autocorrect $mode &&
+
+		git worktree lsit 2>actual &&
+		test_grep "you meant 'list'\.$" actual
+	EOT
+done
+
+test_expect_success 'delay path is executed' - <<-\EOT
+	test_config help.autocorrect 2 &&
+
+	git worktree lsit 2>actual &&
+	test_grep '^Continuing in 0.2 seconds, ' actual
+EOT
+
+test_expect_success 'deny if too dissimilar' - <<-\EOT
+	test_must_fail git remote rensnr 2>actual &&
+	head -n1 actual >first && test_grep -f grep_unknown first
+EOT
+
+test_done
-- 
2.53.0

[PATCH v4 10/10] doc: document autocorrect API

[Jiamu Sun]

Explain behaviors for autocorrect_resolve(), autocorrect_confirm(), and
struct autocorrect.

Signed-off-by: Jiamu Sun <39@barroit.sh>
---
 autocorrect.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/autocorrect.h b/autocorrect.h
index 0d3e819262ed..bfa3ba20a4fb 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -9,13 +9,24 @@ enum autocorrect_mode {
 	AUTOCORRECT_DELAY,
 };
 
+/**
+ * `mode` indicates which action will be performed by autocorrect_confirm().
+ * `delay` is the timeout before autocorrect_confirm() returns, in tenths of a
+ * second. Use it only with AUTOCORRECT_DELAY.
+ */
 struct autocorrect {
 	enum autocorrect_mode mode;
 	int delay;
 };
 
+/**
+ * Resolve the autocorrect configuration into `conf`.
+ */
 void autocorrect_resolve(struct autocorrect *conf);
 
+/**
+ * Interact with the user in different ways depending on `conf->mode`.
+ */
 void autocorrect_confirm(struct autocorrect *conf, const char *assumed);
 
 #endif /* AUTOCORRECT_H */
-- 
2.53.0

Re: [PATCH v4 07/10] parseopt: autocorrect mistyped subcommands

[Junio C Hamano]

Jiamu Sun <39@barroit.sh> writes:

> Try to autocorrect the mistyped mandatory subcommand before showing an
> error and exiting. Subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL
> are skipped.
>
> Use standard Damerau-Levenshtein distance (weights 1, 1, 1, 1) to
> establish a predictable, mathematically sound baseline.
>
> Scale the allowed edit distance based on input length to prevent
> false positives on short commands, following common practice for
> fuzziness thresholds (e.g., Elasticsearch's AUTO fuzziness):
>   - Length 0-2: 0 edits allowed
>   - Length 3-5: 1 edit allowed
>   - Length 6+:  2 edits allowed

Is there a reason why this needs to differ from the settings for the
typo detection/fixes for main commands?  Would the same reasoning
apply to both, and if not why not?

I would have expected that we would just emulate what we already do
to the main commands, and later with experience with the subcommand
typo detection/fixes, would tweak the parameters either only to the
subcommand part or to the both with justifications.

> +	/*
> +	 * Builtin subcommands are small enough that printing them all via
> +	 * usage_with_options() is sufficient. Therefore, AUTOCORRECT_HINT
> +	 * acts like AUTOCORRECT_NEVER.
> +	 */

Sorry, but I am a bit confused with this reference to "Builtin
subcommands".  Are there subcommands that are not built-in?

Re: [PATCH v4 07/10] parseopt: autocorrect mistyped subcommands

[Jiamu Sun]

On Mon, Mar 16, 2026 at 12:41:30PM -0700, Junio C Hamano wrote:
> Is there a reason why this needs to differ from the settings for the
> typo detection/fixes for main commands?  Would the same reasoning
> apply to both, and if not why not?
> 
> I would have expected that we would just emulate what we already do
> to the main commands, and later with experience with the subcommand
> typo detection/fixes, would tweak the parameters either only to the
> subcommand part or to the both with justifications.

I initially wanted to emulate the existing behavior. However, I noticed
a huge difference in how prefix-matched commands are handled, which
affects the autocorrection behavior. And we can only use the same
parameters if we do the exact same thing on prefix match handling in
subcommand autocorrection.

I looked through the old mailing list. If I didn't miss anything, the
patch that introduced this behavior (e.g., not correcting "statu" to
"status") was only trying to fix a UX issue, where the suggestion output
didn't include prefix-matched commands. In that thread, they didn't
mention that this would also change the typo detection behavior, and no
one discussed this side effect. I treat it as an accident.

Because of this, I was confused about whether I should copy this
behavior. I chose not to.

However, if we want the main commands and subcommands to act the same, I
can do that and try to move the logic to autocorrect.c so both places
share the exact same typo detection.

Do you want me to do this?

> > +	/*
> > +	 * Builtin subcommands are small enough that printing them all via
> > +	 * usage_with_options() is sufficient. Therefore, AUTOCORRECT_HINT
> > +	 * acts like AUTOCORRECT_NEVER.
> > +	 */
> 
> Sorry, but I am a bit confused with this reference to "Builtin
> subcommands".  Are there subcommands that are not built-in?

No, that's just poor wording. Will fix it.

-- 
Jiamu Sun <39@barroit.sh>
          <sunjiamu@outlook.com>