usage: clarify --recurse-submodules as a boolean

`git switch`, `git checkout`, `git reset`, and `git read-tree` allow a
user to choose to recurse into submodules. All four of these commands'
short usage seems to indicate that `--recurse-submodules` should take an
argument. In practice, though, all four of these commands parse through
the same callback path:

  option_parse_recurse_submodules_worktree_updater(...) checks for
  set/unset, or passes off to...
  parse_update_recurse_submodules_arg(...), which is a straight handoff
  to...
  parse_update_recurse(...), which only accepts various ways to
  spell Boolean

So ultimately, it can only be true or false (or yes/no/on/off/etc),
unlike `git push --recurse-submodules=<enum>`. A user could provide
`--recurse-submodules=true`, but we don't typically suggest that for
boolean arguments.

Documentation/git-(switch|checkout|reset|read-tree).txt suggests
--[no-]recurse-submodules, too.

In fact, these four commands are the only ones that use this codepath -
so there's not any reason for it to be so meandering. However, because
option_parse_recurse_submodules_worktree_updater() modifies static state
in submodule.c, we still need to get a handle to that static state
through a function call.

To preserve the behavior of --recurse-submodules=true and clarify the
usage string simultaneously, get rid of the OPT_CALLBACK_F in favor of a
simple OPT_BOOL, and call a setter in submodule.c for the static state
instead. Also, remove the now-unused
option_parse_recurse_submodules_worktree_updater(),
parse_update_recurse_submodules_arg(), and parse_update_recurse() calls.

Signed-off-by: Emily Shaffer <nasamuffin@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

7 files changed, 33 insertions, 47 deletions

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 38a8cd6a96..d105403b0e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -80,6 +80,7 @@ struct checkout_opts {
 	int ignore_unmerged;
 	int pathspec_file_nul;
 	char *pathspec_from_file;
+	int recurse_submodules;
 
 	const char *new_branch;
 	const char *new_branch_force;
@@ -1587,9 +1588,8 @@ static struct option *add_common_options(struct checkout_opts *opts,
 {
 	struct option options[] = {
 		OPT__QUIET(&opts->quiet, N_("suppress progress reporting")),
-		OPT_CALLBACK_F(0, "recurse-submodules", NULL,
-			    "checkout", "control recursive updating of submodules",
-			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater),
+		OPT_BOOL(0, "recurse-submodules", &opts->recurse_submodules,
+			 N_("control recursive updating of submodules")),
 		OPT_BOOL(0, "progress", &opts->show_progress, N_("force progress reporting")),
 		OPT_BOOL('m', "merge", &opts->merge, N_("perform a 3-way merge with the new branch")),
 		OPT_STRING(0, "conflict", &opts->conflict_style, N_("style"),
@@ -1597,6 +1597,11 @@ static struct option *add_common_options(struct checkout_opts *opts,
 		OPT_END()
 	};
 	struct option *newopts = parse_options_concat(prevopts, options);
+	/*
+	 * we only want to act on --recurse-submodules if it was set explicitly,
+	 * so put it into unset third state.
+	 */
+	opts->recurse_submodules = -1;
 	free(prevopts);
 	return newopts;
 }
@@ -1677,6 +1682,9 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 	argc = parse_options(argc, argv, prefix, options,
 			     usagestr, parseopt_flags);
 
+	if (opts->recurse_submodules >= 0)
+		set_config_update_recurse_submodules(opts->recurse_submodules);
+
 	if (opts->show_progress < 0) {
 		if (opts->quiet)
 			opts->show_progress = 0;
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 600d4f748f..4d325f7481 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -116,6 +116,7 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
 	struct unpack_trees_options opts;
 	int prefix_set = 0;
 	struct lock_file lock_file = LOCK_INIT;
+	int recurse_submodules = -1;
 	const struct option read_tree_options[] = {
 		OPT__SUPER_PREFIX(&opts.super_prefix),
 		OPT_CALLBACK_F(0, "index-output", NULL, N_("file"),
@@ -149,9 +150,8 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
 			 N_("skip applying sparse checkout filter")),
 		OPT_BOOL(0, "debug-unpack", &opts.internal.debug_unpack,
 			 N_("debug unpack-trees")),
-		OPT_CALLBACK_F(0, "recurse-submodules", NULL,
-			    "checkout", "control recursive updating of submodules",
-			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater),
+		OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
+			 N_("control recursive updating of submodules")),
 		OPT__QUIET(&opts.quiet, N_("suppress feedback messages")),
 		OPT_END()
 	};
@@ -177,6 +177,10 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
 	if (opts.reset)
 		opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED;
 
+	/* only modify the value if set explicitly */
+	if (recurse_submodules >= 0)
+		set_config_update_recurse_submodules(recurse_submodules);
+
 	prepare_repo_settings(the_repository);
 	the_repository->settings.command_requires_full_index = 0;
 
diff --git a/builtin/reset.c b/builtin/reset.c
index 0ed329236c..d67b5ab1e0 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -326,6 +326,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	struct object_id oid;
 	struct pathspec pathspec;
 	int intent_to_add = 0;
+	int recurse_submodules = -1;
 	const struct option options[] = {
 		OPT__QUIET(&quiet, N_("be quiet, only report errors")),
 		OPT_BOOL(0, "no-refresh", &no_refresh,
@@ -339,9 +340,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				N_("reset HEAD, index and working tree"), MERGE),
 		OPT_SET_INT(0, "keep", &reset_type,
 				N_("reset HEAD but keep local changes"), KEEP),
-		OPT_CALLBACK_F(0, "recurse-submodules", NULL,
-			    "reset", "control recursive updating of submodules",
-			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater),
+		OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
+			 N_("control recursive updating of submodules")),
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
@@ -356,6 +356,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
+	/* only modify the value if set explicitly */
+	if (recurse_submodules >= 0)
+		set_config_update_recurse_submodules(recurse_submodules);
+
 	if (pathspec_from_file) {
 		if (patch_mode)
 			die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--patch");
diff --git a/submodule-config.c b/submodule-config.c
index ecf0fcf007..9ef0bdc207 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -338,26 +338,6 @@ int option_fetch_parse_recurse_submodules(const struct option *opt,
 	return 0;
 }
 
-static int parse_update_recurse(const char *opt, const char *arg,
-				int die_on_error)
-{
-	switch (git_parse_maybe_bool(arg)) {
-	case 1:
-		return RECURSE_SUBMODULES_ON;
-	case 0:
-		return RECURSE_SUBMODULES_OFF;
-	default:
-		if (die_on_error)
-			die("bad %s argument: %s", opt, arg);
-		return RECURSE_SUBMODULES_ERROR;
-	}
-}
-
-int parse_update_recurse_submodules_arg(const char *opt, const char *arg)
-{
-	return parse_update_recurse(opt, arg, 1);
-}
-
 static int parse_push_recurse(const char *opt, const char *arg,
 			       int die_on_error)
 {
diff --git a/submodule-config.h b/submodule-config.h
index c2045875bb..fda6ad0162 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -55,7 +55,6 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
 struct option;
 int option_fetch_parse_recurse_submodules(const struct option *opt,
 					  const char *arg, int unset);
-int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
 int parse_push_recurse_submodules_arg(const char *opt, const char *arg);
 void repo_read_gitmodules(struct repository *repo, int skip_if_read);
 void gitmodules_config_oid(const struct object_id *commit_oid);
diff --git a/submodule.c b/submodule.c
index 94644fac0a..8a2a6c8384 100644
--- a/submodule.c
+++ b/submodule.c
@@ -229,21 +229,12 @@ int git_default_submodule_config(const char *var, const char *value,
 	return 0;
 }
 
-int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
-						     const char *arg, int unset)
+void set_config_update_recurse_submodules(int value)
 {
-	if (unset) {
-		config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
-		return 0;
-	}
-	if (arg)
-		config_update_recurse_submodules =
-			parse_update_recurse_submodules_arg(opt->long_name,
-							    arg);
-	else
-		config_update_recurse_submodules = RECURSE_SUBMODULES_ON;
-
-	return 0;
+	if (value < 0 || value > 1)
+		BUG("config_update_recurse_submodules is a boolean");
+	config_update_recurse_submodules = value ? RECURSE_SUBMODULES_ON
+						 : RECURSE_SUBMODULES_OFF;
 }
 
 /*
diff --git a/submodule.h b/submodule.h
index c55a25ca37..9532d4e073 100644
--- a/submodule.h
+++ b/submodule.h
@@ -51,9 +51,9 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *,
 					     const char *path);
 int git_default_submodule_config(const char *var, const char *value, void *cb);
 
-struct option;
-int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
-						     const char *arg, int unset);
+/* Sets static state 'config_update_recurse_submodules'. 'value' must be 0 or 1. */
+void set_config_update_recurse_submodules(int value);
+
 int is_tree_submodule_active(struct repository *repo,
 			     const struct object_id *treeish_name,
 			     const char *path);