patch, 5 partsparseopt: add subcommand autocorrection
96 messages between Mar 8, 2026 and Jun 6, 2026, from Jiamu Sun, Junio C Hamano, Karthik Nayak, Aaron Plattner.
Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.
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()
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(-)
Show changes to parse-options.c +40 −35
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
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
Show changes to 4 files +136 −86
Makefile, autocorrect.c, autocorrect.h, help.c
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
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(-)
Show changes to parse-options.c +74 −2
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
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(-)
Show changes to 4 files +17 −22
builtin/notes.c, builtin/remote.c, parse-options.c, parse-options.h
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
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
Show changes to t/t9004-autocorrect-subcommand.sh +49 −0
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
Jiamu Sun <39@barroit.sh> writes:
Show 11 quoted lines
> 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
On Sun, Mar 08, 2026 at 01:11:22PM -0700, Junio C Hamano wrote:
Show 21 quoted lines
> 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
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()
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(-)
Show changes to parse-options.c +40 −35
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
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
Show changes to 4 files +136 −86
Makefile, autocorrect.c, autocorrect.h, help.c
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
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(-)
Show changes to parse-options.c +74 −2
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
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(-)
Show changes to 4 files +17 −22
builtin/notes.c, builtin/remote.c, parse-options.c, parse-options.h
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
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
Show changes to t/t9004-autocorrect-subcommand.sh +49 −0
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()
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 ...
Show 7 quoted lines
> {
> + 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?
Show 8 quoted lines
> + *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.
Show 16 quoted lines
> +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 ;-)
Show 6 quoted 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.
Show 16 quoted lines
> @@ -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
Jiamu Sun <39@barroit.sh> writes:
Show 67 quoted lines
> 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
Jiamu Sun <39@barroit.sh> writes:
Show 15 quoted lines
> +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()
On Sun, Mar 08, 2026 at 04:40:44PM -0700, Junio C Hamano wrote:
Show 7 quoted lines
> ... 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.
Show 27 quoted lines
> 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
On Sun, Mar 08, 2026 at 04:52:54PM -0700, Junio C Hamano wrote:
Show 19 quoted lines
> 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
On Sun, Mar 08, 2026 at 05:04:49PM -0700, Junio C Hamano wrote:
Show 6 quoted lines
> > + 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
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()
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(-)
Show changes to parse-options.c +46 −40
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
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
Show changes to 4 files +93 −60
Makefile, autocorrect.c, autocorrect.h, help.c
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
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(-)
Show changes to 3 files +49 −38
autocorrect.c, autocorrect.h, help.c
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
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(-)
Show changes to 3 files +5 −5
autocorrect.c, autocorrect.h, help.c
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
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(-)
Show changes to 3 files +28 −29
autocorrect.c, autocorrect.h, help.c
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
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(-)
Show changes to parse-options.c +73 −3
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
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(-)
Show changes to 4 files +17 −22
builtin/notes.c, builtin/remote.c, parse-options.c, parse-options.h
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
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
Show changes to t/t9004-autocorrect-subcommand.sh +51 −0
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()
Jiamu Sun <39@barroit.sh> writes:
[snip]
Show 41 quoted lines
> @@ -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.
Show 10 quoted lines
> + 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
Re: [PATCH v3 2/8] help: make autocorrect handling reusable
Jiamu Sun <39@barroit.sh> writes:
Show 26 quoted lines
> 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]
Show 21 quoted lines
> 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.
Show changes to 2 files +9 −9
autocorrect.c, autocorrect.h
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 3/8] help: move tty check for autocorrection to autocorrect.c
Jiamu Sun <39@barroit.sh> writes:
Show 51 quoted lines
> 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.
Show 90 quoted lines
> 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]
Re: [PATCH v3 4/8] autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINTONLY
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]
Re: [PATCH v3 5/8] autocorrect: provide config resolution API
Jiamu Sun <39@barroit.sh> writes:
Show 36 quoted lines
> 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
Re: [PATCH v3 2/8] help: make autocorrect handling reusable
Karthik Nayak <karthik.188@gmail.com> writes:
Show 15 quoted lines
>> +#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.
Show 7 quoted lines
>
> [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.
Show 40 quoted lines
> 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
Jiamu Sun <39@barroit.sh> writes:
Show 7 quoted lines
> +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.
Show 15 quoted lines
> +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()
On Tue, Mar 10, 2026 at 05:46:12AM -0700, Karthik Nayak wrote:
Show 7 quoted lines
> > + 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
On Tue, Mar 10, 2026 at 05:52:58AM -0700, Karthik Nayak wrote:
Show 9 quoted lines
> > 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
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.
Show 9 quoted lines
> > 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
On Tue, Mar 10, 2026 at 07:06:17AM -0700, Karthik Nayak wrote:
Show 8 quoted lines
> > +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
On Tue, Mar 10, 2026 at 07:08:52AM -0700, Karthik Nayak wrote:
Show 6 quoted lines
>
> [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
On Tue, Mar 10, 2026 at 01:16:39PM -0700, Junio C Hamano wrote:
Show 9 quoted lines
> > + 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()
Jiamu Sun <39@barroit.sh> writes:
Show 10 quoted lines
> 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()
Jiamu Sun <39@barroit.sh> writes:
Show 10 quoted lines
> 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
Jiamu Sun <39@barroit.sh> writes:
Show 12 quoted lines
> 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
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()
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
On 3/8/26 5:17 AM, Jiamu Sun wrote:
Show 9 quoted lines
> 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:
Show changes to t/meson.build +1 −1
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
On Wed, Mar 11, 2026 at 10:01:55AM -0700, Aaron Plattner wrote:
Show 7 quoted lines
> 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:
--
Jiamu Sun <39@barroit.sh>
<sunjiamu@outlook.com>Re: [PATCH v3 6/8] parseopt: autocorrect mistyped subcommands
On Wed, Mar 11, 2026 at 11:48:41AM +0900, Jiamu Sun wrote:
Show 6 quoted lines
> > 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
On Tue, Mar 10, 2026 at 07:06:17AM -0700, Karthik Nayak wrote:
Show 5 quoted lines
> > + 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
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
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
Jiamu Sun <39@barroit.sh> writes:
Show 10 quoted lines
> 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.
Show changes to 2 files +2 −0
meson.build, t/meson.build
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
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 issuesNote 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()
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(-)
Show changes to parse-options.c +46 −41
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
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
Show changes to 5 files +94 −60
Makefile, autocorrect.c, autocorrect.h, help.c, meson.build
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
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(-)
Show changes to 2 files +16 −14
autocorrect.c, help.c
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
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(-)
Show changes to 3 files +42 −33
autocorrect.c, autocorrect.h, help.c
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
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(-)
Show changes to 3 files +5 −5
autocorrect.c, autocorrect.h, help.c
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
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(-)
Show changes to 3 files +30 −30
autocorrect.c, autocorrect.h, help.c
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
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(-)
Show changes to parse-options.c +78 −3
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
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(-)
Show changes to 4 files +17 −22
builtin/notes.c, builtin/remote.c, parse-options.c, parse-options.h
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
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
Show changes to 2 files +52 −0
t/meson.build, 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
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(+)
Show changes to autocorrect.h +11 −0
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
Jiamu Sun <39@barroit.sh> writes:
Show 13 quoted lines
> 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.
Show 5 quoted lines
> + /*
> + * 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
On Mon, Mar 16, 2026 at 12:41:30PM -0700, Junio C Hamano wrote:
Show 8 quoted lines
> 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?
Show 8 quoted lines
> > + /*
> > + * 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>Re: [PATCH v4 07/10] parseopt: autocorrect mistyped subcommands
Jiamu Sun <39@barroit.sh> writes:
Show 13 quoted lines
>> 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.
> ...
> 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?
Either do that (which is probably conceptually simpler), or explain in the proposed log message why they have to be different. My gut feeling is that you should start from identical settings that has been used for years, and then in a separate topic propose to improve the parameters to improve the behaviour for both main commands and sub commands at the same time, but that is largely because there weren't any explanation why subcommands correction should behave differently from main commands ("Correction for main commands behaves strangely and I do not want to inherit it" is not a good explanation as it invites a natural question "if you have a better behaviour you can use for subcommand correction, can't it be used also for main commands? If not, why not?").
Re: [PATCH v4 07/10] parseopt: autocorrect mistyped subcommands
On Wed, Apr 15, 2026 at 10:55:00AM -0700, Junio C Hamano wrote:
Show 6 quoted lines
> Either do that (which is probably conceptually simpler), or explain
> in the proposed log message why they have to be different. My gut
> feeling is that you should start from identical settings that has
> been used for years, and then in a separate topic propose to improve
> the parameters to improve the behaviour for both main commands and
> sub commands at the same time, but that is largely because there
Agreed. I will change the subcommand autocorrection behavior to be the same as the main command in the next revision.
Show 7 quoted lines
> weren't any explanation why subcommands correction should behave
> differently from main commands ("Correction for main commands
> behaves strangely and I do not want to inherit it" is not a good
> explanation as it invites a natural question "if you have a better
> behaviour you can use for subcommand correction, can't it be used
> also for main commands? If not, why not?").
> Currently, command autocorrection skips all prefix matched commands. In my mind, this shouldn't happen because "branc" can indeed be autocorrected to "branch". I studied the old thread, but didn't find any explanation that expands on why it's doing that. Instead of skipping that, applying the autocorrect handling to that case would be better, maybe? But for this patch, I'll keep them the same.
--
Jiamu Sun <39@barroit.sh>
<sunjiamu@outlook.com>[PATCH v5 00/10] parseopt: add subcommand autocorrection
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_AUTOCORRECT 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 string literals are also combined so the full text is easier to grep for.
Changes in v5:
- Make subcommand autocorrection behave the same as command
autocorrection
- Rename PARSE_OPT_SUBCOMMAND_AUTOCORR to
PARSE_OPT_SUBCOMMAND_AUTOCORRECT
- Adjust subcommand autocorrection tests to fit new behaviorChanges 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 issuesChanges 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 | 36 ++++++
builtin/notes.c | 10 +-
builtin/remote.c | 12 +-
help.c | 120 ++++----------------
meson.build | 1 +
parse-options.c | 183 +++++++++++++++++++++++-------
parse-options.h | 1 +
t/meson.build | 1 +
t/t9004-autocorrect-subcommand.sh | 58 ++++++++++
11 files changed, 361 insertions(+), 151 deletions(-)
create mode 100644 autocorrect.c
create mode 100644 autocorrect.h
create mode 100755 t/t9004-autocorrect-subcommand.sh
base-commit: f65aba1e87db64413b6d1ed5ae5a45b5a84a0997
--
2.53.0
[PATCH v5 02/10] help: make autocorrect handling reusable
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
Show changes to 5 files +94 −60
Makefile, autocorrect.c, autocorrect.h, help.c, meson.build
diff --git a/Makefile b/Makefile
index cedc234173e3..ced8a9b5abb4 100644
--- a/Makefile
+++ b/Makefile
@@ -1100,6 +1100,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 3e59d07c370b..ab619ed43c7a 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 11488623bfd8..be20d507826d 100644
--- a/meson.build
+++ b/meson.build
@@ -290,6 +290,7 @@ libgit_sources = [
'archive-zip.c',
'archive.c',
'attr.c',
+ 'autocorrect.c',
'base85.c',
'bisect.c',
'blame.c',
--
2.53.0
[PATCH v5 01/10] parseopt: extract subcommand handling from parse_options_step()
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(-)
Show changes to parse-options.c +46 −41
diff --git a/parse-options.c b/parse-options.c
index a676da86f5d6..803ce2ba4443 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -606,17 +606,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)
@@ -1011,38 +1038,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 v5 03/10] help: move tty check for autocorrection to autocorrect.c
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(-)
Show changes to 2 files +16 −14
autocorrect.c, help.c
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 ab619ed43c7a..d2b29715817e 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 v5 04/10] autocorrect: use mode and delay instead of magic numbers
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(-)
Show changes to 3 files +42 −33
autocorrect.c, autocorrect.h, help.c
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 d2b29715817e..353596c17d82 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 v5 05/10] autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINT
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(-)
Show changes to 3 files +5 −5
autocorrect.c, autocorrect.h, help.c
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 353596c17d82..c7dab8395ee2 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 v5 06/10] autocorrect: provide config resolution API
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(-)
Show changes to 3 files +30 −30
autocorrect.c, autocorrect.h, help.c
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 c7dab8395ee2..0fff43545cc1 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 v5 07/10] parseopt: autocorrect mistyped subcommands
Try to autocorrect the mistyped mandatory subcommand before showing an error and exiting. Subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL are skipped.
The subcommand autocorrection behaves the same as the command autocorrection.
Signed-off-by: Jiamu Sun <39@barroit.sh>
---
autocorrect.h | 4 ++
help.c | 13 ++----
parse-options.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 107 insertions(+), 12 deletions(-)
Show changes to 3 files +107 −12
autocorrect.h, help.c, parse-options.c
diff --git a/autocorrect.h b/autocorrect.h
index 0d3e819262ed..14ee7c4548d3 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -1,6 +1,10 @@
#ifndef AUTOCORRECT_H
#define AUTOCORRECT_H
+/* An empirically derived magic number */
+#define AUTOCORRECT_SIMILARITY_FLOOR 7
+#define AUTOCORRECT_SIMILAR_ENOUGH(x) ((x) < AUTOCORRECT_SIMILARITY_FLOOR)
+
enum autocorrect_mode {
AUTOCORRECT_HINT,
AUTOCORRECT_NEVER,
diff --git a/help.c b/help.c
index 0fff43545cc1..ecc5673b7243 100644
--- a/help.c
+++ b/help.c
@@ -580,10 +580,6 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
old->cnt = 0;
}
-/* An empirically derived magic number */
-#define SIMILARITY_FLOOR 7
-#define SIMILAR_ENOUGH(x) ((x) < SIMILARITY_FLOOR)
-
static const char bad_interpreter_advice[] =
N_("'%s' appears to be a git command, but we were not\n"
"able to execute it. Maybe git-%s is broken?");
@@ -659,7 +655,7 @@ char *help_unknown_cmd(const char *cmd)
if (main_cmds.cnt <= n) {
/* prefix matches with everything? that is too ambiguous */
- best_similarity = SIMILARITY_FLOOR + 1;
+ best_similarity = AUTOCORRECT_SIMILARITY_FLOOR + 1;
} else {
/* count all the most similar ones */
for (best_similarity = main_cmds.names[n++]->len;
@@ -670,7 +666,7 @@ char *help_unknown_cmd(const char *cmd)
}
if (autocorrect.mode != AUTOCORRECT_HINT && n == 1 &&
- SIMILAR_ENOUGH(best_similarity)) {
+ AUTOCORRECT_SIMILAR_ENOUGH(best_similarity)) {
char *assumed = xstrdup(main_cmds.names[0]->name);
fprintf_ln(stderr,
@@ -687,11 +683,10 @@ char *help_unknown_cmd(const char *cmd)
fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
- if (SIMILAR_ENOUGH(best_similarity)) {
+ if (AUTOCORRECT_SIMILAR_ENOUGH(best_similarity)) {
fprintf_ln(stderr,
Q_("\nThe most similar command is",
- "\nThe most similar commands are",
- n));
+ "\nThe most similar commands are", n));
for (i = 0; i < n; i++)
fprintf(stderr, "\t%s\n", main_cmds.names[i]->name);
diff --git a/parse-options.c b/parse-options.c
index 803ce2ba4443..a488f9a41df8 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -7,6 +7,8 @@
#include "string-list.h"
#include "strmap.h"
#include "utf8.h"
+#include "autocorrect.h"
+#include "levenshtein.h"
static int disallow_abbreviated_options;
@@ -623,13 +625,98 @@ 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 levenshtein_compare(const void *p1, const void *p2)
+{
+ const struct string_list_item *i1 = p1, *i2 = p2;
+ const char *s1 = i1->string, *s2 = i2->string;
+ int l1 = (intptr_t)i1->util;
+ int l2 = (intptr_t)i2->util;
+
+ return l1 != l2 ? l1 - l2 : strcmp(s1, s2);
+}
+
+static const char *autocorrect_subcommand(const char *cmd,
+ struct string_list *cmds)
+{
+ struct autocorrect autocorrect = { 0 };
+ unsigned int n = 0, best = 0;
+ struct string_list_item *cand;
+
+ autocorrect_resolve(&autocorrect);
+
+ if (autocorrect.mode == AUTOCORRECT_NEVER)
+ return NULL;
+
+ for_each_string_list_item(cand, cmds) {
+ if (starts_with(cand->string, cmd)) {
+ cand->util = 0;
+ } else {
+ int edit = levenshtein(cmd, cand->string,
+ 0, 2, 1, 3) + 1;
+
+ cand->util = (void *)(intptr_t)edit;
+ }
+ }
+
+ QSORT(cmds->items, cmds->nr, levenshtein_compare);
+
+ /* Match help.c:help_unknown_cmd */
+ for (; n < cmds->nr && !cmds->items[n].util; n++);
+
+ if (n == cmds->nr)
+ /* prefix matches with every subcommands */
+ best = AUTOCORRECT_SIMILARITY_FLOOR + 1;
+ else
+ for (best = (intptr_t)cmds->items[n++].util;
+ (n < cmds->nr && best == (intptr_t)cmds->items[n].util);
+ n++);
+
+ if (autocorrect.mode != AUTOCORRECT_HINT && n == 1 &&
+ AUTOCORRECT_SIMILAR_ENOUGH(best)) {
+ fprintf_ln(stderr,
+ _("WARNING: You called a subcommand named '%s', which does not exist."),
+ cmd);
+
+ autocorrect_confirm(&autocorrect, cmds->items[0].string);
+ return cmds->items[0].string;
+ }
+
+ if (AUTOCORRECT_SIMILAR_ENOUGH(best)) {
+ error(_("'%s' is not a subcommand."), cmd);
+
+ fprintf_ln(stderr,
+ Q_("\nThe most similar subcommand is",
+ "\nThe most similar subcommands are",
+ n));
+
+ for (unsigned int i = 0; i < n; i++)
+ fprintf(stderr, "\t%s\n", cmds->items[i].string);
+
+ exit(1);
+ }
+
+ 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;
@@ -642,8 +729,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 v5 08/10] parseopt: enable subcommand autocorrection for git-remote and git-notes
Add PARSE_OPT_SUBCOMMAND_AUTOCORRECT to enable autocorrection for subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL.
Apply this to 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. Both builtins either resolve to a single subcommand or take no subcommand at all, meaning any unknown argument encountered by the parser 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(-)
Show changes to 4 files +17 −22
builtin/notes.c, builtin/remote.c, parse-options.c, parse-options.h
diff --git a/builtin/notes.c b/builtin/notes.c
index 9af602bdd7b4..f9bf350df4e8 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_AUTOCORRECT);
+ if (!fn)
fn = list;
- }
if (override_notes_ref) {
struct strbuf sb = STRBUF_INIT;
diff --git a/builtin/remote.c b/builtin/remote.c
index de989ea3ba96..6a78ab8f4cd2 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1953,15 +1953,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_AUTOCORRECT);
- 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 a488f9a41df8..820683e982c1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -720,14 +720,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_AUTOCORRECT)) {
+ /*
+ * 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..e5fd4da4055b 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_AUTOCORRECT = 1 << 8,
};
enum parse_opt_option_flags {
--
2.53.0
[PATCH v5 09/10] parseopt: add tests for subcommand autocorrection
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 | 58 +++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+)
create mode 100755 t/t9004-autocorrect-subcommand.sh
Show changes to 2 files +59 −0
t/meson.build, t/t9004-autocorrect-subcommand.sh
diff --git a/t/meson.build b/t/meson.build
index 7528e5cda5fe..871cd7ab0a08 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -980,6 +980,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..09d281bd105b
--- /dev/null
+++ b/t/t9004-autocorrect-subcommand.sh
@@ -0,0 +1,58 @@
+#!/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 show hint only' '
+ test_must_fail git worktree lsit 2>actual &&
+ test_grep "most similar subcommand" actual
+'
+
+test_expect_success "'never' disables autocorrection" '
+ test_config help.autocorrect never &&
+
+ 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
+do
+ test_expect_success "'$mode' disables autocorrection and shows hints" "
+ test_config help.autocorrect $mode &&
+
+ test_must_fail git worktree lsit 2>actual &&
+ test_grep 'most similar subcommand' actual
+ "
+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 v5 10/10] doc: document autocorrect API
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(+)
Show changes to autocorrect.h +11 −0
diff --git a/autocorrect.h b/autocorrect.h
index 14ee7c4548d3..5bb67cf6debd 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -13,13 +13,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 v5 00/10] parseopt: add subcommand autocorrection
Jiamu Sun <39@barroit.sh> writes:
Show 9 quoted lines
> 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.
> ...
> base-commit: f65aba1e87db64413b6d1ed5ae5a45b5a84a0997
When the series is applied on top of Git 2.54, or on top of f65aba1e (codeql: bump actions/cache from 4 to 5, 2026-04-13), the result does not pass its self test, at least for me.
It seems that the exit status of some error path (subcommand parse failure, presumably) has changed unexpectedly? As I am mostly offline for this and next week, I didn't dig any further.
Thanks.
Test Summary Report
-------------------
t0040-parse-options.sh (Wstat: 256 (exited 1) Tests: 94 Failed: 1)
Failed test: 73
Non-zero exit status: 1
t7900-maintenance.sh (Wstat: 256 (exited 1) Tests: 72 Failed: 1)
Failed test: 1
Non-zero exit status: 1
Re: [PATCH v5 00/10] parseopt: add subcommand autocorrection
On Wed, Apr 22, 2026 at 05:08:41PM -0700, Junio C Hamano wrote:
> It seems that the exit status of some error path (subcommand parse
> failure, presumably) has changed unexpectedly? As I am mostly
> offline for this and next week, I didn't dig any further.
My fault, forgot to adjust these tests. Since subcommand parser learned to autocorrect misspelled names,
> t0040-parse-options.sh (Wstat: 256 (exited 1) Tests: 94 Failed: 1)
test-tool parse-subcommand cmd subcmd-o
in "subcommand - subcommands cannot be abbreviated" test now shows similar subcommands.
> t7900-maintenance.sh (Wstat: 256 (exited 1) Tests: 72 Failed: 1)
git maintenance barf
in "help text" test now corrects "barf" to "start".
After fixing these two, all tests pass.
----
All tests successful.
Files=1043, Tests=33202, 465 wallclock secs
( 7.02 usr 2.23 sys + 1244.78 cusr 1642.22 csys = 2896.25 CPU)
Result: PASS
Also, I found most error paths exit with 129 in parse-options.c, but the similar subcommand hint exits with 1. That's inconsistent, will change the exit code to 129.
--
Jiamu Sun <39@barroit.sh>
<sunjiamu@outlook.com>[PATCH v6 00/10] parseopt: add subcommand autocorrection
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_AUTOCORRECT 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 string literals are also combined so the full text is easier to grep for.
Changes in v6:
- Adjust existing tests to fit subcommand autocorrection behavior
- Change the similar subcommand hint exit code to 129
Changes in v5:
- Make subcommand autocorrection behave the same as command
autocorrection
- Rename PARSE_OPT_SUBCOMMAND_AUTOCORR to
PARSE_OPT_SUBCOMMAND_AUTOCORRECT
- Adjust subcommand autocorrection tests to fit new behaviorChanges 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 issuesChanges 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 | 36 ++++++
builtin/notes.c | 10 +-
builtin/remote.c | 12 +-
help.c | 120 ++++----------------
meson.build | 1 +
parse-options.c | 183 +++++++++++++++++++++++-------
parse-options.h | 1 +
t/meson.build | 1 +
t/t0040-parse-options.sh | 5 +-
t/t7900-maintenance.sh | 4 +-
t/t9004-autocorrect-subcommand.sh | 58 ++++++++++
13 files changed, 366 insertions(+), 155 deletions(-)
create mode 100644 autocorrect.c
create mode 100644 autocorrect.h
create mode 100755 t/t9004-autocorrect-subcommand.sh
base-commit: f65aba1e87db64413b6d1ed5ae5a45b5a84a0997
--
2.54.0
[PATCH v6 01/10] parseopt: extract subcommand handling from parse_options_step()
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(-)
Show changes to parse-options.c +46 −41
diff --git a/parse-options.c b/parse-options.c
index a676da86f5d6..803ce2ba4443 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -606,17 +606,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)
@@ -1011,38 +1038,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.54.0
[PATCH v6 02/10] help: make autocorrect handling reusable
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
Show changes to 5 files +94 −60
Makefile, autocorrect.c, autocorrect.h, help.c, meson.build
diff --git a/Makefile b/Makefile
index cedc234173e3..ced8a9b5abb4 100644
--- a/Makefile
+++ b/Makefile
@@ -1100,6 +1100,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 3e59d07c370b..ab619ed43c7a 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 11488623bfd8..be20d507826d 100644
--- a/meson.build
+++ b/meson.build
@@ -290,6 +290,7 @@ libgit_sources = [
'archive-zip.c',
'archive.c',
'attr.c',
+ 'autocorrect.c',
'base85.c',
'bisect.c',
'blame.c',
--
2.54.0
[PATCH v6 03/10] help: move tty check for autocorrection to autocorrect.c
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(-)
Show changes to 2 files +16 −14
autocorrect.c, help.c
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 ab619ed43c7a..d2b29715817e 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.54.0
[PATCH v6 04/10] autocorrect: use mode and delay instead of magic numbers
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(-)
Show changes to 3 files +42 −33
autocorrect.c, autocorrect.h, help.c
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 d2b29715817e..353596c17d82 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.54.0
[PATCH v6 06/10] autocorrect: provide config resolution API
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(-)
Show changes to 3 files +30 −30
autocorrect.c, autocorrect.h, help.c
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 c7dab8395ee2..0fff43545cc1 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.54.0
[PATCH v6 10/10] doc: document autocorrect API
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(+)
Show changes to autocorrect.h +11 −0
diff --git a/autocorrect.h b/autocorrect.h
index 14ee7c4548d3..5bb67cf6debd 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -13,13 +13,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.54.0
[PATCH v6 05/10] autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINT
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(-)
Show changes to 3 files +5 −5
autocorrect.c, autocorrect.h, help.c
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 353596c17d82..c7dab8395ee2 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.54.0
[PATCH v6 07/10] parseopt: autocorrect mistyped subcommands
Try to autocorrect the mistyped mandatory subcommand before showing an error and exiting. Subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL are skipped.
The subcommand autocorrection behaves the same as the command autocorrection.
Signed-off-by: Jiamu Sun <39@barroit.sh>
---
autocorrect.h | 4 ++
help.c | 13 ++---
parse-options.c | 102 +++++++++++++++++++++++++++++++++++++--
t/t0040-parse-options.sh | 5 +-
t/t7900-maintenance.sh | 4 +-
5 files changed, 112 insertions(+), 16 deletions(-)
Show changes to 5 files +112 −16
autocorrect.h, help.c, parse-options.c, t/t0040-parse-options.sh, t/t7900-maintenance.sh
diff --git a/autocorrect.h b/autocorrect.h
index 0d3e819262ed..14ee7c4548d3 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -1,6 +1,10 @@
#ifndef AUTOCORRECT_H
#define AUTOCORRECT_H
+/* An empirically derived magic number */
+#define AUTOCORRECT_SIMILARITY_FLOOR 7
+#define AUTOCORRECT_SIMILAR_ENOUGH(x) ((x) < AUTOCORRECT_SIMILARITY_FLOOR)
+
enum autocorrect_mode {
AUTOCORRECT_HINT,
AUTOCORRECT_NEVER,
diff --git a/help.c b/help.c
index 0fff43545cc1..ecc5673b7243 100644
--- a/help.c
+++ b/help.c
@@ -580,10 +580,6 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
old->cnt = 0;
}
-/* An empirically derived magic number */
-#define SIMILARITY_FLOOR 7
-#define SIMILAR_ENOUGH(x) ((x) < SIMILARITY_FLOOR)
-
static const char bad_interpreter_advice[] =
N_("'%s' appears to be a git command, but we were not\n"
"able to execute it. Maybe git-%s is broken?");
@@ -659,7 +655,7 @@ char *help_unknown_cmd(const char *cmd)
if (main_cmds.cnt <= n) {
/* prefix matches with everything? that is too ambiguous */
- best_similarity = SIMILARITY_FLOOR + 1;
+ best_similarity = AUTOCORRECT_SIMILARITY_FLOOR + 1;
} else {
/* count all the most similar ones */
for (best_similarity = main_cmds.names[n++]->len;
@@ -670,7 +666,7 @@ char *help_unknown_cmd(const char *cmd)
}
if (autocorrect.mode != AUTOCORRECT_HINT && n == 1 &&
- SIMILAR_ENOUGH(best_similarity)) {
+ AUTOCORRECT_SIMILAR_ENOUGH(best_similarity)) {
char *assumed = xstrdup(main_cmds.names[0]->name);
fprintf_ln(stderr,
@@ -687,11 +683,10 @@ char *help_unknown_cmd(const char *cmd)
fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
- if (SIMILAR_ENOUGH(best_similarity)) {
+ if (AUTOCORRECT_SIMILAR_ENOUGH(best_similarity)) {
fprintf_ln(stderr,
Q_("\nThe most similar command is",
- "\nThe most similar commands are",
- n));
+ "\nThe most similar commands are", n));
for (i = 0; i < n; i++)
fprintf(stderr, "\t%s\n", main_cmds.names[i]->name);
diff --git a/parse-options.c b/parse-options.c
index 803ce2ba4443..4370d9c623e4 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -7,6 +7,8 @@
#include "string-list.h"
#include "strmap.h"
#include "utf8.h"
+#include "autocorrect.h"
+#include "levenshtein.h"
static int disallow_abbreviated_options;
@@ -623,13 +625,98 @@ 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 levenshtein_compare(const void *p1, const void *p2)
+{
+ const struct string_list_item *i1 = p1, *i2 = p2;
+ const char *s1 = i1->string, *s2 = i2->string;
+ int l1 = (intptr_t)i1->util;
+ int l2 = (intptr_t)i2->util;
+
+ return l1 != l2 ? l1 - l2 : strcmp(s1, s2);
+}
+
+static const char *autocorrect_subcommand(const char *cmd,
+ struct string_list *cmds)
+{
+ struct autocorrect autocorrect = { 0 };
+ unsigned int n = 0, best = 0;
+ struct string_list_item *cand;
+
+ autocorrect_resolve(&autocorrect);
+
+ if (autocorrect.mode == AUTOCORRECT_NEVER)
+ return NULL;
+
+ for_each_string_list_item(cand, cmds) {
+ if (starts_with(cand->string, cmd)) {
+ cand->util = 0;
+ } else {
+ int edit = levenshtein(cmd, cand->string,
+ 0, 2, 1, 3) + 1;
+
+ cand->util = (void *)(intptr_t)edit;
+ }
+ }
+
+ QSORT(cmds->items, cmds->nr, levenshtein_compare);
+
+ /* Match help.c:help_unknown_cmd */
+ for (; n < cmds->nr && !cmds->items[n].util; n++);
+
+ if (n == cmds->nr)
+ /* prefix matches with every subcommands */
+ best = AUTOCORRECT_SIMILARITY_FLOOR + 1;
+ else
+ for (best = (intptr_t)cmds->items[n++].util;
+ (n < cmds->nr && best == (intptr_t)cmds->items[n].util);
+ n++);
+
+ if (autocorrect.mode != AUTOCORRECT_HINT && n == 1 &&
+ AUTOCORRECT_SIMILAR_ENOUGH(best)) {
+ fprintf_ln(stderr,
+ _("WARNING: You called a subcommand named '%s', which does not exist."),
+ cmd);
+
+ autocorrect_confirm(&autocorrect, cmds->items[0].string);
+ return cmds->items[0].string;
+ }
+
+ if (AUTOCORRECT_SIMILAR_ENOUGH(best)) {
+ error(_("'%s' is not a subcommand."), cmd);
+
+ fprintf_ln(stderr,
+ Q_("\nThe most similar subcommand is",
+ "\nThe most similar subcommands are",
+ n));
+
+ for (unsigned int i = 0; i < n; i++)
+ fprintf(stderr, "\t%s\n", cmds->items[i].string);
+
+ exit(129);
+ }
+
+ 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;
@@ -642,8 +729,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)
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ca55ea8228c3..2a2fef1e17dc 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -632,8 +632,9 @@ test_expect_success 'subcommand - unknown subcommand shows error and usage' '
test_expect_success 'subcommand - subcommands cannot be abbreviated' '
test_expect_code 129 test-tool parse-subcommand cmd subcmd-o 2>err &&
- grep "^error: unknown subcommand: \`subcmd-o$SQ$" err &&
- grep ^usage: err
+ grep "^The most similar subcommands are$" err &&
+ grep "subcmd-one$" err &&
+ grep "subcmd-two$" err
'
test_expect_success 'subcommand - no negated subcommands' '
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 4700beacc182..7174b9932829 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -37,8 +37,8 @@ test_systemd_analyze_verify () {
test_expect_success 'help text' '
test_expect_code 129 git maintenance -h >actual &&
test_grep "usage: git maintenance <subcommand>" actual &&
- test_expect_code 129 git maintenance barf 2>err &&
- test_grep "unknown subcommand: \`barf'\''" err &&
+ test_expect_code 129 git maintenance abarf 2>err &&
+ test_grep "unknown subcommand: \`abarf'\''" err &&
test_grep "usage: git maintenance" err &&
test_expect_code 129 git maintenance 2>err &&
test_grep "error: need a subcommand" err &&
--
2.54.0
[PATCH v6 08/10] parseopt: enable subcommand autocorrection for git-remote and git-notes
Add PARSE_OPT_SUBCOMMAND_AUTOCORRECT to enable autocorrection for subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL.
Apply this to 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. Both builtins either resolve to a single subcommand or take no subcommand at all, meaning any unknown argument encountered by the parser 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(-)
Show changes to 4 files +17 −22
builtin/notes.c, builtin/remote.c, parse-options.c, parse-options.h
diff --git a/builtin/notes.c b/builtin/notes.c
index 9af602bdd7b4..f9bf350df4e8 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_AUTOCORRECT);
+ if (!fn)
fn = list;
- }
if (override_notes_ref) {
struct strbuf sb = STRBUF_INIT;
diff --git a/builtin/remote.c b/builtin/remote.c
index de989ea3ba96..6a78ab8f4cd2 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1953,15 +1953,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_AUTOCORRECT);
- 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 4370d9c623e4..d60e7bd3c977 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -720,14 +720,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_AUTOCORRECT)) {
+ /*
+ * 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..e5fd4da4055b 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_AUTOCORRECT = 1 << 8,
};
enum parse_opt_option_flags {
--
2.54.0
[PATCH v6 09/10] parseopt: add tests for subcommand autocorrection
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 | 58 +++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+)
create mode 100755 t/t9004-autocorrect-subcommand.sh
Show changes to 2 files +59 −0
t/meson.build, t/t9004-autocorrect-subcommand.sh
diff --git a/t/meson.build b/t/meson.build
index 7528e5cda5fe..871cd7ab0a08 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -980,6 +980,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..09d281bd105b
--- /dev/null
+++ b/t/t9004-autocorrect-subcommand.sh
@@ -0,0 +1,58 @@
+#!/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 show hint only' '
+ test_must_fail git worktree lsit 2>actual &&
+ test_grep "most similar subcommand" actual
+'
+
+test_expect_success "'never' disables autocorrection" '
+ test_config help.autocorrect never &&
+
+ 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
+do
+ test_expect_success "'$mode' disables autocorrection and shows hints" "
+ test_config help.autocorrect $mode &&
+
+ test_must_fail git worktree lsit 2>actual &&
+ test_grep 'most similar subcommand' actual
+ "
+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.54.0
Re: [PATCH v6 00/10] parseopt: add subcommand autocorrection
Jiamu Sun <39@barroit.sh> writes:
Show 27 quoted lines
> 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_AUTOCORRECT 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 string literals are also combined so the full text is easier to
> grep for.
>
> Changes in v6:
> - Adjust existing tests to fit subcommand autocorrection behavior
> - Change the similar subcommand hint exit code to 129
I've been carrying the following fix on top of these series since Apr 23 when the topic was merged to 'seen'. Can you fix these up at the source, so that we can move forward with this topic?
Thanks.
Show changes to parse-options.c +3 −2
diff --git a/parse-options.c b/parse-options.c
index a1258134df..1a5feab5dd 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -647,7 +647,8 @@ static const char *autocorrect_subcommand(const char *cmd,
struct string_list *cmds)
{
struct autocorrect autocorrect = { 0 };
- unsigned int n = 0, best = 0;
+ unsigned int n = 0;
+ int best = 0;
struct string_list_item *cand;
autocorrect_resolve(&autocorrect);
@@ -657,7 +658,7 @@ static const char *autocorrect_subcommand(const char *cmd,
for_each_string_list_item(cand, cmds) {
if (starts_with(cand->string, cmd)) {
- cand->util = 0;
+ cand->util = NULL;
} else {
int edit = levenshtein(cmd, cand->string,
0, 2, 1, 3) + 1;
--
2.54.0-170-g88022b8681
Re: [PATCH v6 00/10] parseopt: add subcommand autocorrection
On Mon, May 11, 2026 at 12:03:12PM +0900, Junio C Hamano wrote:
Show 5 quoted lines
> I've been carrying the following fix on top of these series since
> Apr 23 when the topic was merged to 'seen'. Can you fix these up at
> the source, so that we can move forward with this topic?
>
> Thanks.
Sorry for the delay. This email didn't reach my inbox.
By the time I saw this fix, I had already sent v6. Should I resend v6 with this fix squashed in, or bump to v7?
--
Jiamu Sun <39@barroit.sh>
<sunjiamu@outlook.com>Re: [PATCH v6 00/10] parseopt: add subcommand autocorrection
Jiamu Sun <39@barroit.sh> writes:
Show 11 quoted lines
> On Mon, May 11, 2026 at 12:03:12PM +0900, Junio C Hamano wrote:
>> I've been carrying the following fix on top of these series since
>> Apr 23 when the topic was merged to 'seen'. Can you fix these up at
>> the source, so that we can move forward with this topic?
>>
>> Thanks.
>
> Sorry for the delay. This email didn't reach my inbox.
>
> By the time I saw this fix, I had already sent v6. Should I resend v6
> with this fix squashed in, or bump to v7?
Sorry for the delay; this exchange fell through the cracks, and then I no longer recall exactly what the "fix" was. If your v6 still lacks the "fix" I gave (sorry but I do not remember what it was, and I am away from my desk), then please do incorporate and send an updated one as v7. Hopefully it would be the final edition, unless there are other issues outstanding.
Thanks.