[PATCH 4/8] builtin/config: move modes into separate functions

From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Subject: [PATCH 4/8] builtin/config: move modes into separate functions
Date: Wed, 6 Mar 2024 12:31:46 +0100	[thread overview]
Message-ID: <b258f31b7d9d7bb104175b7dedc3e13506cea9e9.1709724089.git.ps@pks.im> (raw)
In-Reply-To: <cover.1709724089.git.ps@pks.im>

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

The git-config(1) command has several different modes which cause it to
do different things. The logic for each of these modes is hosted in a
giant switch in `cmd_config()` itself. For one, this is hard to read.
But second, we're about to introduce proper subcommands to git-config(1)
that will require separate functions for each of the modes.

Refactor the code and move each mode into its own function to prepare
for this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/config.c | 410 +++++++++++++++++++++++++++++------------------
 1 file changed, 255 insertions(+), 155 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 8a2d1a5de7..a6ab9b8204 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -44,6 +44,7 @@ static struct config_options config_options;
 static int show_origin;
 static int show_scope;
 static int fixed_value;
+static int config_flags;
 
 #define ACTION_GET (1<<0)
 #define ACTION_GET_ALL (1<<1)
@@ -622,6 +623,225 @@ static char *default_user_config(void)
 	return strbuf_detach(&buf, NULL);
 }
 
+static int cmd_config_list(int argc, const char **argv, const char *prefix)
+{
+	check_argc(argc, 0, 0);
+	if (config_with_options(show_all_config, NULL,
+				&given_config_source, the_repository,
+				&config_options) < 0) {
+		if (given_config_source.file)
+			die_errno(_("unable to read config file '%s'"),
+				  given_config_source.file);
+		else
+			die(_("error processing config file(s)"));
+	}
+
+	return 0;
+}
+
+static int cmd_config_edit(int argc, const char **argv, const char *prefix)
+{
+	char *config_file;
+
+	check_argc(argc, 0, 0);
+	if (!given_config_source.file && !startup_info->have_repository)
+		die(_("not in a git directory"));
+	if (given_config_source.use_stdin)
+		die(_("editing stdin is not supported"));
+	if (given_config_source.blob)
+		die(_("editing blobs is not supported"));
+	git_config(git_default_config, NULL);
+	config_file = given_config_source.file ?
+			xstrdup(given_config_source.file) :
+			git_pathdup("config");
+	if (use_global_config) {
+		int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY, 0666);
+		if (fd >= 0) {
+			char *content = default_user_config();
+			write_str_in_full(fd, content);
+			free(content);
+			close(fd);
+		}
+		else if (errno != EEXIST)
+			die_errno(_("cannot create configuration file %s"), config_file);
+	}
+	launch_editor(config_file, NULL, NULL);
+	free(config_file);
+
+	return 0;
+}
+
+static int cmd_config_set(int argc, const char **argv, const char *prefix)
+{
+	struct key_value_info default_kvi = KVI_INIT;
+	char *value = NULL;
+	int ret;
+
+	check_write();
+	check_argc(argc, 2, 2);
+	value = normalize_value(argv[0], argv[1], &default_kvi);
+	ret = git_config_set_in_file_gently(given_config_source.file, argv[0], value);
+	if (ret == CONFIG_NOTHING_SET)
+		error(_("cannot overwrite multiple values with a single value\n"
+		"       Use a regexp, --add or --replace-all to change %s."), argv[0]);
+
+	free(value);
+	return ret;
+}
+
+static int cmd_config_set_all(int argc, const char **argv, const char *prefix)
+{
+	struct key_value_info default_kvi = KVI_INIT;
+	char *value = NULL;
+	int ret;
+
+	check_write();
+	check_argc(argc, 2, 3);
+	value = normalize_value(argv[0], argv[1], &default_kvi);
+	ret = git_config_set_multivar_in_file_gently(given_config_source.file,
+						     argv[0], value, argv[2],
+						     config_flags);
+
+	free(value);
+	return ret;
+}
+
+static int cmd_config_add(int argc, const char **argv, const char *prefix)
+{
+	struct key_value_info default_kvi = KVI_INIT;
+	char *value = NULL;
+	int ret;
+
+	check_write();
+	check_argc(argc, 2, 2);
+	value = normalize_value(argv[0], argv[1], &default_kvi);
+	ret = git_config_set_multivar_in_file_gently(given_config_source.file,
+						     argv[0], value,
+						     CONFIG_REGEX_NONE,
+						     config_flags);
+
+	free(value);
+	return ret;
+}
+
+static int cmd_config_replace_all(int argc, const char **argv, const char *prefix)
+{
+	struct key_value_info default_kvi = KVI_INIT;
+	char *value = NULL;
+	int ret;
+
+	check_write();
+	check_argc(argc, 2, 3);
+	value = normalize_value(argv[0], argv[1], &default_kvi);
+	ret = git_config_set_multivar_in_file_gently(given_config_source.file,
+						     argv[0], value, argv[2],
+						     config_flags | CONFIG_FLAGS_MULTI_REPLACE);
+
+	free(value);
+	return ret;
+}
+
+static int cmd_config_get(int argc, const char **argv, const char *prefix)
+{
+	check_argc(argc, 1, 2);
+	return get_value(argv[0], argv[1], config_flags);
+}
+
+static int cmd_config_get_all(int argc, const char **argv, const char *prefix)
+{
+	do_all = 1;
+	check_argc(argc, 1, 2);
+	return get_value(argv[0], argv[1], config_flags);
+}
+
+static int cmd_config_get_regexp(int argc, const char **argv, const char *prefix)
+{
+	show_keys = 1;
+	use_key_regexp = 1;
+	do_all = 1;
+	check_argc(argc, 1, 2);
+	return get_value(argv[0], argv[1], config_flags);
+}
+
+static int cmd_config_get_urlmatch(int argc, const char **argv, const char *prefix)
+{
+	check_argc(argc, 2, 2);
+	return get_urlmatch(argv[0], argv[1]);
+}
+
+static int cmd_config_unset(int argc, const char **argv, const char *prefix)
+{
+	check_write();
+	check_argc(argc, 1, 2);
+	if (argc == 2)
+		return git_config_set_multivar_in_file_gently(given_config_source.file,
+							      argv[0], NULL, argv[1],
+							      config_flags);
+	else
+		return git_config_set_in_file_gently(given_config_source.file,
+						     argv[0], NULL);
+}
+
+static int cmd_config_unset_all(int argc, const char **argv, const char *prefix)
+{
+	check_write();
+	check_argc(argc, 1, 2);
+	return git_config_set_multivar_in_file_gently(given_config_source.file,
+						      argv[0], NULL, argv[1],
+						      config_flags | CONFIG_FLAGS_MULTI_REPLACE);
+}
+
+static int cmd_config_rename_section(int argc, const char **argv, const char *prefix)
+{
+	int ret;
+
+	check_write();
+	check_argc(argc, 2, 2);
+	ret = git_config_rename_section_in_file(given_config_source.file,
+						argv[0], argv[1]);
+	if (ret < 0)
+		return ret;
+	else if (!ret)
+		die(_("no such section: %s"), argv[0]);
+	else
+		ret = 0;
+
+	return ret;
+}
+
+static int cmd_config_remove_section(int argc, const char **argv, const char *prefix)
+{
+	int ret;
+
+	check_write();
+	check_argc(argc, 1, 1);
+	ret = git_config_rename_section_in_file(given_config_source.file,
+						argv[0], NULL);
+	if (ret < 0)
+		return ret;
+	else if (!ret)
+		die(_("no such section: %s"), argv[0]);
+	else
+		ret = 0;
+
+	return ret;
+}
+
+static int cmd_config_get_color(int argc, const char **argv, const char *prefix)
+{
+	check_argc(argc, 1, 2);
+	get_color(argv[0], argv[1]);
+	return 0;
+}
+
+static int cmd_config_get_colorbool(int argc, const char **argv, const char *prefix)
+{
+	check_argc(argc, 1, 2);
+	if (argc == 2)
+		color_stdout_is_tty = git_config_bool("command line", argv[1]);
+	return get_colorbool(argv[0], argc == 2);
+}
+
 static struct option builtin_config_options[] = {
 	OPT_GROUP(N_("Config file location")),
 	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
@@ -671,12 +891,6 @@ static NORETURN void usage_builtin_config(void)
 
 int cmd_config(int argc, const char **argv, const char *prefix)
 {
-	int nongit = !startup_info->have_repository;
-	char *value = NULL;
-	int flags = 0;
-	int ret = 0;
-	struct key_value_info default_kvi = KVI_INIT;
-
 	given_config_source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT));
 
 	argc = parse_options(argc, argv, prefix, builtin_config_options,
@@ -690,7 +904,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		usage_builtin_config();
 	}
 
-	if (nongit) {
+	if (!startup_info->have_repository) {
 		if (use_local_config)
 			die(_("--local can only be used inside a git repository"));
 		if (given_config_source.blob)
@@ -751,7 +965,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		config_options.respect_includes = !given_config_source.file;
 	else
 		config_options.respect_includes = respect_includes_opt;
-	if (!nongit) {
+	if (startup_info->have_repository) {
 		config_options.commondir = get_git_common_dir();
 		config_options.git_dir = get_git_dir();
 	}
@@ -826,159 +1040,45 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			usage_builtin_config();
 		}
 
-		flags |= CONFIG_FLAGS_FIXED_VALUE;
+		config_flags |= CONFIG_FLAGS_FIXED_VALUE;
 	}
 
 	if (actions & PAGING_ACTIONS)
 		setup_auto_pager("config", 1);
 
 	if (actions == ACTION_LIST) {
-		check_argc(argc, 0, 0);
-		if (config_with_options(show_all_config, NULL,
-					&given_config_source, the_repository,
-					&config_options) < 0) {
-			if (given_config_source.file)
-				die_errno(_("unable to read config file '%s'"),
-					  given_config_source.file);
-			else
-				die(_("error processing config file(s)"));
-		}
+		return cmd_config_list(argc, argv, prefix);
+	} else if (actions == ACTION_EDIT) {
+		return cmd_config_edit(argc, argv, prefix);
+	} else if (actions == ACTION_SET) {
+		return cmd_config_set(argc, argv, prefix);
+	} else if (actions == ACTION_SET_ALL) {
+		return cmd_config_set_all(argc, argv, prefix);
+	} else if (actions == ACTION_ADD) {
+		return cmd_config_add(argc, argv, prefix);
+	} else if (actions == ACTION_REPLACE_ALL) {
+		return cmd_config_replace_all(argc, argv, prefix);
+	} else if (actions == ACTION_GET) {
+		return cmd_config_get(argc, argv, prefix);
+	} else if (actions == ACTION_GET_ALL) {
+		return cmd_config_get_all(argc, argv, prefix);
+	} else if (actions == ACTION_GET_REGEXP) {
+		return cmd_config_get_regexp(argc, argv, prefix);
+	} else if (actions == ACTION_GET_URLMATCH) {
+		return cmd_config_get_urlmatch(argc, argv, prefix);
+	} else if (actions == ACTION_UNSET) {
+		return cmd_config_unset(argc, argv, prefix);
+	} else if (actions == ACTION_UNSET_ALL) {
+		return cmd_config_unset_all(argc, argv, prefix);
+	} else if (actions == ACTION_RENAME_SECTION) {
+		return cmd_config_rename_section(argc, argv, prefix);
+	} else if (actions == ACTION_REMOVE_SECTION) {
+		return cmd_config_remove_section(argc, argv, prefix);
+	} else if (actions == ACTION_GET_COLOR) {
+		return cmd_config_get_color(argc, argv, prefix);
+	} else if (actions == ACTION_GET_COLORBOOL) {
+		return cmd_config_get_colorbool(argc, argv, prefix);
 	}
-	else if (actions == ACTION_EDIT) {
-		char *config_file;
 
-		check_argc(argc, 0, 0);
-		if (!given_config_source.file && nongit)
-			die(_("not in a git directory"));
-		if (given_config_source.use_stdin)
-			die(_("editing stdin is not supported"));
-		if (given_config_source.blob)
-			die(_("editing blobs is not supported"));
-		git_config(git_default_config, NULL);
-		config_file = given_config_source.file ?
-				xstrdup(given_config_source.file) :
-				git_pathdup("config");
-		if (use_global_config) {
-			int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY, 0666);
-			if (fd >= 0) {
-				char *content = default_user_config();
-				write_str_in_full(fd, content);
-				free(content);
-				close(fd);
-			}
-			else if (errno != EEXIST)
-				die_errno(_("cannot create configuration file %s"), config_file);
-		}
-		launch_editor(config_file, NULL, NULL);
-		free(config_file);
-	}
-	else if (actions == ACTION_SET) {
-		check_write();
-		check_argc(argc, 2, 2);
-		value = normalize_value(argv[0], argv[1], &default_kvi);
-		ret = git_config_set_in_file_gently(given_config_source.file, argv[0], value);
-		if (ret == CONFIG_NOTHING_SET)
-			error(_("cannot overwrite multiple values with a single value\n"
-			"       Use a regexp, --add or --replace-all to change %s."), argv[0]);
-	}
-	else if (actions == ACTION_SET_ALL) {
-		check_write();
-		check_argc(argc, 2, 3);
-		value = normalize_value(argv[0], argv[1], &default_kvi);
-		ret = git_config_set_multivar_in_file_gently(given_config_source.file,
-							     argv[0], value, argv[2],
-							     flags);
-	}
-	else if (actions == ACTION_ADD) {
-		check_write();
-		check_argc(argc, 2, 2);
-		value = normalize_value(argv[0], argv[1], &default_kvi);
-		ret = git_config_set_multivar_in_file_gently(given_config_source.file,
-							     argv[0], value,
-							     CONFIG_REGEX_NONE,
-							     flags);
-	}
-	else if (actions == ACTION_REPLACE_ALL) {
-		check_write();
-		check_argc(argc, 2, 3);
-		value = normalize_value(argv[0], argv[1], &default_kvi);
-		ret = git_config_set_multivar_in_file_gently(given_config_source.file,
-							     argv[0], value, argv[2],
-							     flags | CONFIG_FLAGS_MULTI_REPLACE);
-	}
-	else if (actions == ACTION_GET) {
-		check_argc(argc, 1, 2);
-		return get_value(argv[0], argv[1], flags);
-	}
-	else if (actions == ACTION_GET_ALL) {
-		do_all = 1;
-		check_argc(argc, 1, 2);
-		return get_value(argv[0], argv[1], flags);
-	}
-	else if (actions == ACTION_GET_REGEXP) {
-		show_keys = 1;
-		use_key_regexp = 1;
-		do_all = 1;
-		check_argc(argc, 1, 2);
-		return get_value(argv[0], argv[1], flags);
-	}
-	else if (actions == ACTION_GET_URLMATCH) {
-		check_argc(argc, 2, 2);
-		return get_urlmatch(argv[0], argv[1]);
-	}
-	else if (actions == ACTION_UNSET) {
-		check_write();
-		check_argc(argc, 1, 2);
-		if (argc == 2)
-			return git_config_set_multivar_in_file_gently(given_config_source.file,
-								      argv[0], NULL, argv[1],
-								      flags);
-		else
-			return git_config_set_in_file_gently(given_config_source.file,
-							     argv[0], NULL);
-	}
-	else if (actions == ACTION_UNSET_ALL) {
-		check_write();
-		check_argc(argc, 1, 2);
-		return git_config_set_multivar_in_file_gently(given_config_source.file,
-							      argv[0], NULL, argv[1],
-							      flags | CONFIG_FLAGS_MULTI_REPLACE);
-	}
-	else if (actions == ACTION_RENAME_SECTION) {
-		check_write();
-		check_argc(argc, 2, 2);
-		ret = git_config_rename_section_in_file(given_config_source.file,
-							argv[0], argv[1]);
-		if (ret < 0)
-			return ret;
-		else if (!ret)
-			die(_("no such section: %s"), argv[0]);
-		else
-			ret = 0;
-	}
-	else if (actions == ACTION_REMOVE_SECTION) {
-		check_write();
-		check_argc(argc, 1, 1);
-		ret = git_config_rename_section_in_file(given_config_source.file,
-							argv[0], NULL);
-		if (ret < 0)
-			return ret;
-		else if (!ret)
-			die(_("no such section: %s"), argv[0]);
-		else
-			ret = 0;
-	}
-	else if (actions == ACTION_GET_COLOR) {
-		check_argc(argc, 1, 2);
-		get_color(argv[0], argv[1]);
-	}
-	else if (actions == ACTION_GET_COLORBOOL) {
-		check_argc(argc, 1, 2);
-		if (argc == 2)
-			color_stdout_is_tty = git_config_bool("command line", argv[1]);
-		return get_colorbool(argv[0], argc == 2);
-	}
-
-	free(value);
-	return ret;
+	BUG("invalid action");
 }
-- 
2.44.0


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