From patchwork Wed Mar 6 11:31:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13583958 Received: from fhigh4-smtp.messagingengine.com (fhigh4-smtp.messagingengine.com [103.168.172.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9F0B0763FC for ; Wed, 6 Mar 2024 11:31:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709724715; cv=none; b=sq+LZqohl+49dg7zI1Ub/Oao6V6PBvrgtO9wJFKuHsyg0Xlgi5I7fNMABAV7AgjIgAaSoWgLEY7tBu/ZtHB1v6CuKn9nPTmJjubPLZqW/29mA7702HNB3Kjwoud/82S70Z/S16fkkKLPRId+8C3hx3sh5PwZy2U+SqpAY1g2rHQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709724715; c=relaxed/simple; bh=WslLQBtU0zyG6UecKpzYQ5W1L7PXL7MfUdwvsslWFgo=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=czu8G85d7S0C5ho2zXhhl/g2ZgOvtvG/pQRvgfE/iau4bNIZ9TGgJBzen5lEcGKmORUSMQWClZgNAJOMnJEch8pmIFggqELAf2nhABfgpwCzZkFtN65tV/seRJDXf5CST3N6dnN36sY0tacQQj/OeGCIg5i5gWEDvzL/Iz7wc7E= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=s/4ja3/J; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=bG/srSqU; arc=none smtp.client-ip=103.168.172.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="s/4ja3/J"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="bG/srSqU" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.nyi.internal (Postfix) with ESMTP id AE50111400B3 for ; Wed, 6 Mar 2024 06:31:52 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Wed, 06 Mar 2024 06:31:52 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm1; t=1709724712; x=1709811112; bh=tvRA2uqLRs 1GolxBY3XEM7xmQT8gJUNn9j++HcHTSVo=; b=s/4ja3/J/Ff/AgYGAsYlzft1GK DfkCCL8dy8G1bFoa5jMC1WFOYOj5bScQpdreOECB8O39/gYL+WxB9Zts8xoFyYoC InWdMDVr88H3GIE/ojLMchAKs8LXRXAJ33JZarVEKvVBy6uRL12zdzaRiRjpwwAm ma0QXtWd/vOLs+qvSvPbEB8Y4ZGowy8opse5SI+Dowk46mrIlOpo/PmyOUb7ypW0 Be8+RNHeke62LkOu5Rfqsb5ENbmxphkpVSHhrknJI87Tf9YE0/6pq+cpcCqYhHT/ U4JUSMJA28kfbpQ1wCp2E5LH4cNPwa1CfzcerYVWQlp/9EenQDuerhi+qC9w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1709724712; x=1709811112; bh=tvRA2uqLRs1GolxBY3XEM7xmQT8g JUNn9j++HcHTSVo=; b=bG/srSqUAd8CjjfvNwYQ1lVmVf0CwcG6sjb3V6HKCIGE ofqxsefZwC2oUQMEn4yWCPwD2h/TYGo0zJwRH6K9NcY5Fo+4E/kAhjbjEn7aIplr EzwPqprhITq9lQIiZmzdlrLpzqNzryYZkk5gi8Vc8kwm/3bgYcP6P4xHUJlr0RyY 7IH42PSdQ6eGXTXXQx1x89fpbsMj0csw13Kvflz8E6ItUBbQ8eH0Kjql7O/HuzRX KzWxyBdbPo9nuRijXKTb5xXklY6QdwGctHACjacLWAc62GbYUhXvY7t8lfYuCx5j wGHoWv2PMzK0Vq5A/3vGVzbYslOy9+FIURhSS9pilA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledriedugddviecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtderre dttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhk shdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtdfhvd ejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedunecurfgrrhgr mhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Wed, 6 Mar 2024 06:31:52 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 7c68d29d (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Wed, 6 Mar 2024 11:27:23 +0000 (UTC) Date: Wed, 6 Mar 2024 12:31:50 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 5/8] builtin/config: track subcommands by action Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Part of `cmd_config()` is a rather unwieldy switch statement that invokes the correct subcommand function based on which action has been requested by the user. Now that we have converted actions to be tracked via a `OPT_CMDMODE()`, we know that the `actions` variable will only ever have at most one bit set. This allows us to convert the variable to use an `enum` instead, and thus to create an array that maps from this newly introduced `enum` to the corresponding subcommand function. Refactor the code to do so. Besides allowing us to get rid of the giant switch statement, this refactoring will also make it easier to introduce proper subcommands to git-config(1). Signed-off-by: Patrick Steinhardt --- builtin/config.c | 207 +++++++++++++++++++++++------------------------ 1 file changed, 99 insertions(+), 108 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index a6ab9b8204..0d58397ef5 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -20,6 +20,26 @@ static const char *const builtin_config_usage[] = { NULL }; +enum config_action { + ACTION_NONE, + ACTION_GET, + ACTION_GET_ALL, + ACTION_GET_REGEXP, + ACTION_REPLACE_ALL, + ACTION_ADD, + ACTION_UNSET, + ACTION_UNSET_ALL, + ACTION_RENAME_SECTION, + ACTION_REMOVE_SECTION, + ACTION_LIST, + ACTION_EDIT, + ACTION_SET, + ACTION_SET_ALL, + ACTION_GET_COLOR, + ACTION_GET_COLORBOOL, + ACTION_GET_URLMATCH, +}; + static char *key; static regex_t *key_regexp; static const char *value_pattern; @@ -33,10 +53,12 @@ static char delim = '='; static char key_delim = ' '; static char term = '\n'; +static parse_opt_subcommand_fn *subcommand; +static enum config_action action = ACTION_NONE; static int use_global_config, use_system_config, use_local_config; static int use_worktree_config; static struct git_config_source given_config_source; -static int actions, type; +static int type; static char *default_value; static int end_nul; static int respect_includes_opt = -1; @@ -46,30 +68,6 @@ static int show_scope; static int fixed_value; static int config_flags; -#define ACTION_GET (1<<0) -#define ACTION_GET_ALL (1<<1) -#define ACTION_GET_REGEXP (1<<2) -#define ACTION_REPLACE_ALL (1<<3) -#define ACTION_ADD (1<<4) -#define ACTION_UNSET (1<<5) -#define ACTION_UNSET_ALL (1<<6) -#define ACTION_RENAME_SECTION (1<<7) -#define ACTION_REMOVE_SECTION (1<<8) -#define ACTION_LIST (1<<9) -#define ACTION_EDIT (1<<10) -#define ACTION_SET (1<<11) -#define ACTION_SET_ALL (1<<12) -#define ACTION_GET_COLOR (1<<13) -#define ACTION_GET_COLORBOOL (1<<14) -#define ACTION_GET_URLMATCH (1<<15) - -/* - * The actions "ACTION_LIST | ACTION_GET_*" which may produce more than - * one line of output and which should therefore be paged. - */ -#define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \ - ACTION_GET_REGEXP | ACTION_GET_URLMATCH) - #define TYPE_BOOL 1 #define TYPE_INT 2 #define TYPE_BOOL_OR_INT 3 @@ -842,6 +840,25 @@ static int cmd_config_get_colorbool(int argc, const char **argv, const char *pre return get_colorbool(argv[0], argc == 2); } +static parse_opt_subcommand_fn *subcommands_by_action[] = { + [ACTION_LIST] = cmd_config_list, + [ACTION_EDIT] = cmd_config_edit, + [ACTION_SET] = cmd_config_set, + [ACTION_SET_ALL] = cmd_config_set_all, + [ACTION_ADD] = cmd_config_add, + [ACTION_REPLACE_ALL] = cmd_config_replace_all, + [ACTION_GET] = cmd_config_get, + [ACTION_GET_ALL] = cmd_config_get_all, + [ACTION_GET_REGEXP] = cmd_config_get_regexp, + [ACTION_GET_URLMATCH] = cmd_config_get_urlmatch, + [ACTION_UNSET] = cmd_config_unset, + [ACTION_UNSET_ALL] = cmd_config_unset_all, + [ACTION_RENAME_SECTION] = cmd_config_rename_section, + [ACTION_REMOVE_SECTION] = cmd_config_remove_section, + [ACTION_GET_COLOR] = cmd_config_get_color, + [ACTION_GET_COLORBOOL] = cmd_config_get_colorbool, +}; + static struct option builtin_config_options[] = { OPT_GROUP(N_("Config file location")), OPT_BOOL(0, "global", &use_global_config, N_("use global config file")), @@ -851,20 +868,20 @@ static struct option builtin_config_options[] = { OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")), OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")), OPT_GROUP(N_("Action")), - OPT_CMDMODE(0, "get", &actions, N_("get value: name [value-pattern]"), ACTION_GET), - OPT_CMDMODE(0, "get-all", &actions, N_("get all values: key [value-pattern]"), ACTION_GET_ALL), - OPT_CMDMODE(0, "get-regexp", &actions, N_("get values for regexp: name-regex [value-pattern]"), ACTION_GET_REGEXP), - OPT_CMDMODE(0, "get-urlmatch", &actions, N_("get value specific for the URL: section[.var] URL"), ACTION_GET_URLMATCH), - OPT_CMDMODE(0, "replace-all", &actions, N_("replace all matching variables: name value [value-pattern]"), ACTION_REPLACE_ALL), - OPT_CMDMODE(0, "add", &actions, N_("add a new variable: name value"), ACTION_ADD), - OPT_CMDMODE(0, "unset", &actions, N_("remove a variable: name [value-pattern]"), ACTION_UNSET), - OPT_CMDMODE(0, "unset-all", &actions, N_("remove all matches: name [value-pattern]"), ACTION_UNSET_ALL), - OPT_CMDMODE(0, "rename-section", &actions, N_("rename section: old-name new-name"), ACTION_RENAME_SECTION), - OPT_CMDMODE(0, "remove-section", &actions, N_("remove a section: name"), ACTION_REMOVE_SECTION), - OPT_CMDMODE('l', "list", &actions, N_("list all"), ACTION_LIST), - OPT_CMDMODE('e', "edit", &actions, N_("open an editor"), ACTION_EDIT), - OPT_CMDMODE(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR), - OPT_CMDMODE(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL), + OPT_CMDMODE(0, "get", &action, N_("get value: name [value-pattern]"), ACTION_GET), + OPT_CMDMODE(0, "get-all", &action, N_("get all values: key [value-pattern]"), ACTION_GET_ALL), + OPT_CMDMODE(0, "get-regexp", &action, N_("get values for regexp: name-regex [value-pattern]"), ACTION_GET_REGEXP), + OPT_CMDMODE(0, "get-urlmatch", &action, N_("get value specific for the URL: section[.var] URL"), ACTION_GET_URLMATCH), + OPT_CMDMODE(0, "replace-all", &action, N_("replace all matching variables: name value [value-pattern]"), ACTION_REPLACE_ALL), + OPT_CMDMODE(0, "add", &action, N_("add a new variable: name value"), ACTION_ADD), + OPT_CMDMODE(0, "unset", &action, N_("remove a variable: name [value-pattern]"), ACTION_UNSET), + OPT_CMDMODE(0, "unset-all", &action, N_("remove all matches: name [value-pattern]"), ACTION_UNSET_ALL), + OPT_CMDMODE(0, "rename-section", &action, N_("rename section: old-name new-name"), ACTION_RENAME_SECTION), + OPT_CMDMODE(0, "remove-section", &action, N_("remove a section: name"), ACTION_REMOVE_SECTION), + OPT_CMDMODE('l', "list", &action, N_("list all"), ACTION_LIST), + OPT_CMDMODE('e', "edit", &action, N_("open an editor"), ACTION_EDIT), + OPT_CMDMODE(0, "get-color", &action, N_("find the color configured: slot [default]"), ACTION_GET_COLOR), + OPT_CMDMODE(0, "get-colorbool", &action, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL), OPT_GROUP(N_("Type")), OPT_CALLBACK('t', "type", &type, N_("type"), N_("value is given this type"), option_parse_type), OPT_CALLBACK_VALUE(0, "bool", &type, N_("value is \"true\" or \"false\""), TYPE_BOOL), @@ -976,33 +993,43 @@ int cmd_config(int argc, const char **argv, const char *prefix) key_delim = '\n'; } - if ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && type) { - error(_("--get-color and variable type are incoherent")); - usage_builtin_config(); - } - - if (actions == 0) + if (action == ACTION_NONE) { switch (argc) { - case 1: actions = ACTION_GET; break; - case 2: actions = ACTION_SET; break; - case 3: actions = ACTION_SET_ALL; break; + case 1: action = ACTION_GET; break; + case 2: action = ACTION_SET; break; + case 3: action = ACTION_SET_ALL; break; default: usage_builtin_config(); } + } + if (action <= ACTION_NONE || action >= ARRAY_SIZE(subcommands_by_action)) + BUG("invalid action %d", action); + subcommand = subcommands_by_action[action]; + + if (type && (subcommand == cmd_config_get_color || + subcommand == cmd_config_get_colorbool)) { + error(_("--get-color and variable type are incoherent")); + usage_builtin_config(); + } + if (omit_values && - !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) { + subcommand != cmd_config_list && + subcommand != cmd_config_get_regexp) { error(_("--name-only is only applicable to --list or --get-regexp")); usage_builtin_config(); } - if (show_origin && !(actions & - (ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST))) { + if (show_origin && + subcommand != cmd_config_get && + subcommand != cmd_config_get_all && + subcommand != cmd_config_get_regexp && + subcommand != cmd_config_list) { error(_("--show-origin is only applicable to --get, --get-all, " "--get-regexp, and --list")); usage_builtin_config(); } - if (default_value && !(actions & ACTION_GET)) { + if (default_value && subcommand != cmd_config_get) { error(_("--default is only applicable to --get")); usage_builtin_config(); } @@ -1011,28 +1038,19 @@ int cmd_config(int argc, const char **argv, const char *prefix) if (fixed_value) { int allowed_usage = 0; - switch (actions) { - /* git config --get */ - case ACTION_GET: - /* git config --get-all */ - case ACTION_GET_ALL: - /* git config --get-regexp */ - case ACTION_GET_REGEXP: - /* git config --unset */ - case ACTION_UNSET: - /* git config --unset-all */ - case ACTION_UNSET_ALL: + if (subcommand == cmd_config_get || + subcommand == cmd_config_get_all || + subcommand == cmd_config_get_regexp || + subcommand == cmd_config_unset || + subcommand == cmd_config_unset_all) { + /* git config -- */ allowed_usage = argc > 1 && !!argv[1]; - break; - - /* git config */ - case ACTION_SET_ALL: - /* git config --replace-all */ - case ACTION_REPLACE_ALL: + } else if (subcommand == cmd_config_set_all || + subcommand == cmd_config_replace_all) { + /* git config -- */ allowed_usage = argc > 2 && !!argv[2]; - break; - - /* other options don't allow --fixed-value */ + } else { + /* other options don't allow --fixed-value */ } if (!allowed_usage) { @@ -1043,42 +1061,15 @@ int cmd_config(int argc, const char **argv, const char *prefix) config_flags |= CONFIG_FLAGS_FIXED_VALUE; } - if (actions & PAGING_ACTIONS) + /* + * The actions "ACTION_LIST | ACTION_GET_*" which may produce more than + * one line of output and which should therefore be paged. + */ + if (subcommand == cmd_config_list || + subcommand == cmd_config_get_all || + subcommand == cmd_config_get_regexp || + subcommand == cmd_config_get_urlmatch) setup_auto_pager("config", 1); - if (actions == ACTION_LIST) { - 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); - } - - BUG("invalid action"); + return subcommand(argc, argv, prefix); }