Merge branch 'js/parseopt-subcommand-autocorrection' into seen

The parse-options library learned to auto-correct misspelled
subcommand names.

Comments?

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

Author: gitster

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

autocorrect.c

@@ -0,0 +1,89 @@
+#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 autocorrect_mode parse_autocorrect(const char *value)
+{
+	switch (git_parse_maybe_bool_text(value)) {
+	case 1:
+		return AUTOCORRECT_IMMEDIATELY;
+	case 0:
+		return AUTOCORRECT_HINT;
+	default: /* other random text */
+		break;
+	}
+
+	if (!strcmp(value, "prompt"))
+		return AUTOCORRECT_PROMPT;
+	else if (!strcmp(value, "never"))
+		return AUTOCORRECT_NEVER;
+	else if (!strcmp(value, "immediate"))
+		return AUTOCORRECT_IMMEDIATELY;
+	else if (!strcmp(value, "show"))
+		return AUTOCORRECT_HINT;
+	else
+		return AUTOCORRECT_DELAY;
+}
+
+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 0;
+
+	conf->mode = 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 (conf->mode == AUTOCORRECT_DELAY) {
+		conf->delay = git_config_int(var, value, ctx->kvi);
+
+		if (!conf->delay)
+			conf->mode = AUTOCORRECT_HINT;
+		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)
+{
+	if (conf->mode == AUTOCORRECT_IMMEDIATELY) {
+		fprintf_ln(stderr,
+			   _("Continuing under the assumption that you meant '%s'."),
+			   assumed);
+	} else if (conf->mode == 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 if (conf->mode == AUTOCORRECT_DELAY) {
+		fprintf_ln(stderr,
+			   _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
+			   conf->delay / 10.0, assumed);
+		sleep_millisec(conf->delay * 100);
+	}
+}

autocorrect.h

@@ -0,0 +1,36 @@
+#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,
+	AUTOCORRECT_PROMPT,
+	AUTOCORRECT_IMMEDIATELY,
+	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 */

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;

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();
-	}
 }

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,77 +537,30 @@ 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)
-{
-	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)
+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;
 
-	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)) {
 		size_t key_len = strlen(key);
 
 		if (subsection) {
 			/* [alias "name"] command = value */
 			if (!strcmp(key, "command"))
-				add_cmdname(&cfg->aliases, subsection,
+				add_cmdname(aliases, subsection,
 					    subsection_len);
 			else {
 				key = var + strlen("alias.");
 				key_len = strlen(key);
-				add_cmdname(&cfg->aliases, key, key_len);
+				add_cmdname(aliases, key, key_len);
 			}
 		} else {
 			/* alias.name = value */
-			add_cmdname(&cfg->aliases, key, key_len);
+			add_cmdname(aliases, key, key_len);
 		}
 	}
 
@@ -633,38 +587,32 @@ 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?");
 
 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);
-
-	/*
-	 * Disable autocorrection prompt in a non-interactive session
-	 */
-	if ((cfg.autocorrect == AUTOCORRECT_PROMPT) && (!isatty(0) || !isatty(2)))
-		cfg.autocorrect = AUTOCORRECT_NEVER;
+	autocorrect_resolve(&autocorrect);
 
-	if (cfg.autocorrect == 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);
@@ -714,7 +662,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;
@@ -723,49 +671,29 @@ char *help_unknown_cmd(const char *cmd)
 		     n++)
 			; /* still counting */
 	}
-	if (cfg.autocorrect && cfg.autocorrect != AUTOCORRECT_SHOW && n == 1 &&
-	    SIMILAR_ENOUGH(best_similarity)) {
+
+	if (autocorrect.mode != AUTOCORRECT_HINT && n == 1 &&
+	    AUTOCORRECT_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);
+		autocorrect_confirm(&autocorrect, assumed);
+
+		cmdnames_release(&aliases);
 		cmdnames_release(&main_cmds);
 		cmdnames_release(&other_cmds);
 		return assumed;
 	}
 
 	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);

meson.build

@@ -290,6 +290,7 @@ libgit_sources = [
   'archive-zip.c',
   'archive.c',
   'attr.c',
+  'autocorrect.c',
   'base85.c',
   'bisect.c',
   'blame.c',