@@ -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;
@@ -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;
@@ -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");
@@ -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)
{
@@ -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);
@@ -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;
}
/*
@@ -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);
`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 true or false. So ultimately, it can only be true or false, 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> --- The one thing missing from this patch is tests - especially since the need for a reroll was predicated on not breaking a specific user edge case. But I had some trouble finding a good spot, because it seems these commands weren't explicitly testing the --recurse-submodule flag anywhere. I'm planning to add them sometime this week, but if someone beats me to it, or has a suggestion of where to put them, or if Junio wants to take this patch without them, I won't mind. Changes since v1: - Added translation tag to the usage string at all 3 callsites - Took Junio's advice[1] and got rid of the callback interface entirely, replacing it with a setter to the static state in submodule.c. By the way, it occurred to me to try and get a handle directly to the static int and pass that in to OPT_BOOL instead of this overhead. I gave it a try despite the feeling of poor layering smell, but it turns out we're saved from having to even consider that, because RECURSE_SUBMODULES_ON == 2. So no matter what, we will need to take an extra step to do the conversion from bool (and be careful about setting when it wasn't explicitly provided, because this static also takes its value from the config before the option parse happens). - Emily 1: https://lore.kernel.org/git/xmqqy1n3fat2.fsf%40gitster.g --- Range-diff against v1: 1: c345a44e36 < -: ---------- usage: clarify --recurse-submodules as a boolean -: ---------- > 1: 1ec2a064d2 usage: clarify --recurse-submodules as a boolean builtin/checkout.c | 14 +++++++++++--- builtin/read-tree.c | 10 +++++++--- builtin/reset.c | 10 +++++++--- submodule-config.c | 20 -------------------- submodule-config.h | 1 - submodule.c | 19 +++++-------------- submodule.h | 6 +++--- 7 files changed, 33 insertions(+), 47 deletions(-)