{"thread":{"id":"17931","subject":"[PATCH B v4 2/5] git config: don't allow multiple config file locations","startedAt":"2009-02-21T00:49:24Z","lastAt":"2009-03-07T09:47:54Z","messageCount":13,"participants":["Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":5},"messages":[{"id":"105678","messageId":"1235177369-2727-1-git-send-email-felipe.contreras@gmail.com","threadId":"17931","inReplyTo":null,"subject":"[PATCH B v4 0/5] git config: reorganization","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-21T00:49:24Z","receivedAt":"2009-02-21T00:49:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This patch series is the first of a two parts reorganization for more\nmaintainability and user friendliness.\n\nThe main change is the switch to use parseopt which provides a nicer help usage\ndialog. The rest of the changes take advantage of the initial reorganization to\ndo more strict option parsing.\n\nFelipe Contreras (5):\n  git config: reorganize to use parseopt\n  git config: don't allow multiple config file locations\n  git config: don't allow multiple variable types\n  git config: don't allow extra arguments for -e or -l.\n  git config: don't allow --get-color* and variable type\n\n builtin-config.c |  385 +++++++++++++++++++++++++++++++-----------------------\n 1 files changed, 223 insertions(+), 162 deletions(-)\n"},{"id":"105683","messageId":"1235177369-2727-2-git-send-email-felipe.contreras@gmail.com","threadId":"17931","inReplyTo":"1235177369-2727-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH B v4 1/5] git config: reorganize to use parseopt","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-21T00:49:25Z","receivedAt":"2009-02-21T00:49:25Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This patch has benefited from comments by Johannes\nSchindelin and Junio C Hamano.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c |  349 ++++++++++++++++++++++++++++++------------------------\n 1 files changed, 195 insertions(+), 154 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 0836880..302846e 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -1,9 +1,12 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n #include \"color.h\"\n+#include \"parse-options.h\"\n \n-static const char git_config_set_usage[] =\n-\"git config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color var [default] | --get-colorbool name [stdout-is-tty] | --edit | -e ]\";\n+static const char *const builtin_config_usage[] = {\n+\t\"git config [options]\",\n+\tNULL\n+};\n \n static char *key;\n static regex_t *key_regexp;\n@@ -18,6 +21,63 @@ static char key_delim = ' ';\n static char term = '\\n';\n static enum { T_RAW, T_INT, T_BOOL, T_BOOL_OR_INT } type = T_RAW;\n \n+static int use_global_config, use_system_config;\n+static const char *given_config_file;\n+static int actions;\n+static const char *get_color_slot, *get_colorbool_slot;\n+static int end_null;\n+\n+#define ACTION_GET (1<<0)\n+#define ACTION_GET_ALL (1<<1)\n+#define ACTION_GET_REGEXP (1<<2)\n+#define ACTION_REPLACE_ALL (1<<3)\n+#define ACTION_ADD (1<<4)\n+#define ACTION_UNSET (1<<5)\n+#define ACTION_UNSET_ALL (1<<6)\n+#define ACTION_RENAME_SECTION (1<<7)\n+#define ACTION_REMOVE_SECTION (1<<8)\n+#define ACTION_LIST (1<<9)\n+#define ACTION_EDIT (1<<10)\n+#define ACTION_SET (1<<11)\n+#define ACTION_SET_ALL (1<<12)\n+#define ACTION_GET_COLOR (1<<13)\n+#define ACTION_GET_COLORBOOL (1<<14)\n+\n+static struct option builtin_config_options[] = {\n+\tOPT_GROUP(\"Config file location\"),\n+\tOPT_BOOLEAN(0, \"global\", &use_global_config, \"use global config file\"),\n+\tOPT_BOOLEAN(0, \"system\", &use_system_config, \"use system config file\"),\n+\tOPT_STRING('f', \"file\", &given_config_file, \"FILE\", \"use given config file\"),\n+\tOPT_GROUP(\"Action\"),\n+\tOPT_BIT(0, \"get\", &actions, \"get value: name [value-regex]\", ACTION_GET),\n+\tOPT_BIT(0, \"get-all\", &actions, \"get all values: key [value-regex]\", ACTION_GET_ALL),\n+\tOPT_BIT(0, \"get-regexp\", &actions, \"get values for regexp: name-regex [value-regex]\", ACTION_GET_REGEXP),\n+\tOPT_BIT(0, \"replace-all\", &actions, \"replace all matching variables: name [value [value_regex]\", ACTION_REPLACE_ALL),\n+\tOPT_BIT(0, \"add\", &actions, \"adds a new variable: name value\", ACTION_ADD),\n+\tOPT_BIT(0, \"unset\", &actions, \"removes a variable: name [value-regex]\", ACTION_UNSET),\n+\tOPT_BIT(0, \"unset-all\", &actions, \"removes all matches: name [value-regex]\", ACTION_UNSET_ALL),\n+\tOPT_BIT(0, \"rename-section\", &actions, \"rename section: old-name new-name\", ACTION_RENAME_SECTION),\n+\tOPT_BIT(0, \"remove-section\", &actions, \"remove a section: name\", ACTION_REMOVE_SECTION),\n+\tOPT_BIT('l', \"list\", &actions, \"list all\", ACTION_LIST),\n+\tOPT_BIT('e', \"edit\", &actions, \"opens an editor\", ACTION_EDIT),\n+\tOPT_STRING(0, \"get-color\", &get_color_slot, \"slot\", \"find the color configured: [default]\"),\n+\tOPT_STRING(0, \"get-colorbool\", &get_colorbool_slot, \"slot\", \"find the color setting: [stdout-is-tty]\"),\n+\tOPT_GROUP(\"Type\"),\n+\tOPT_SET_INT(0, \"bool\", &type, \"value is \\\"true\\\" or \\\"false\\\"\", T_BOOL),\n+\tOPT_SET_INT(0, \"int\", &type, \"value is decimal number\", T_INT),\n+\tOPT_SET_INT(0, \"bool-or-int\", &type, NULL, T_BOOL_OR_INT),\n+\tOPT_GROUP(\"Other\"),\n+\tOPT_BOOLEAN('z', \"null\", &end_null, \"terminate values with NUL byte\"),\n+\tOPT_END(),\n+};\n+\n+static void check_argc(int argc, int min, int max) {\n+\tif (argc >= min && argc <= max)\n+\t\treturn;\n+\terror(\"wrong number of arguments\");\n+\tusage_with_options(builtin_config_usage, builtin_config_options);\n+}\n+\n static int show_all_config(const char *key_, const char *value_, void *cb)\n {\n \tif (value_)\n@@ -252,162 +312,143 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \n \tconfig_exclusive_filename = getenv(CONFIG_ENVIRONMENT);\n \n-\twhile (1 < argc) {\n-\t\tif (!strcmp(argv[1], \"--int\"))\n-\t\t\ttype = T_INT;\n-\t\telse if (!strcmp(argv[1], \"--bool\"))\n-\t\t\ttype = T_BOOL;\n-\t\telse if (!strcmp(argv[1], \"--bool-or-int\"))\n-\t\t\ttype = T_BOOL_OR_INT;\n-\t\telse if (!strcmp(argv[1], \"--list\") || !strcmp(argv[1], \"-l\")) {\n-\t\t\tif (argc != 2)\n-\t\t\t\tusage(git_config_set_usage);\n-\t\t\tif (git_config(show_all_config, NULL) < 0) {\n-\t\t\t\tif (config_exclusive_filename)\n-\t\t\t\t\tdie(\"unable to read config file %s: %s\",\n-\t\t\t\t\t    config_exclusive_filename, strerror(errno));\n-\t\t\t\telse\n-\t\t\t\t\tdie(\"error processing config file(s)\");\n-\t\t\t}\n-\t\t\treturn 0;\n-\t\t}\n-\t\telse if (!strcmp(argv[1], \"--global\")) {\n-\t\t\tchar *home = getenv(\"HOME\");\n-\t\t\tif (home) {\n-\t\t\t\tchar *user_config = xstrdup(mkpath(\"%s/.gitconfig\", home));\n-\t\t\t\tconfig_exclusive_filename = user_config;\n-\t\t\t} else {\n-\t\t\t\tdie(\"$HOME not set\");\n-\t\t\t}\n-\t\t}\n-\t\telse if (!strcmp(argv[1], \"--system\"))\n-\t\t\tconfig_exclusive_filename = git_etc_gitconfig();\n-\t\telse if (!strcmp(argv[1], \"--file\") || !strcmp(argv[1], \"-f\")) {\n-\t\t\tif (argc < 3)\n-\t\t\t\tusage(git_config_set_usage);\n-\t\t\tif (!is_absolute_path(argv[2]) && prefix)\n-\t\t\t\tconfig_exclusive_filename = prefix_filename(prefix,\n-\t\t\t\t\t\t\t\t\t    strlen(prefix),\n-\t\t\t\t\t\t\t\t\t    argv[2]);\n-\t\t\telse\n-\t\t\t\tconfig_exclusive_filename = argv[2];\n-\t\t\targc--;\n-\t\t\targv++;\n-\t\t}\n-\t\telse if (!strcmp(argv[1], \"--null\") || !strcmp(argv[1], \"-z\")) {\n-\t\t\tterm = '\\0';\n-\t\t\tdelim = '\\n';\n-\t\t\tkey_delim = '\\n';\n-\t\t}\n-\t\telse if (!strcmp(argv[1], \"--rename-section\")) {\n-\t\t\tint ret;\n-\t\t\tif (argc != 4)\n-\t\t\t\tusage(git_config_set_usage);\n-\t\t\tret = git_config_rename_section(argv[2], argv[3]);\n-\t\t\tif (ret < 0)\n-\t\t\t\treturn ret;\n-\t\t\tif (ret == 0) {\n-\t\t\t\tfprintf(stderr, \"No such section!\\n\");\n-\t\t\t\treturn 1;\n-\t\t\t}\n-\t\t\treturn 0;\n-\t\t}\n-\t\telse if (!strcmp(argv[1], \"--remove-section\")) {\n-\t\t\tint ret;\n-\t\t\tif (argc != 3)\n-\t\t\t\tusage(git_config_set_usage);\n-\t\t\tret = git_config_rename_section(argv[2], NULL);\n-\t\t\tif (ret < 0)\n-\t\t\t\treturn ret;\n-\t\t\tif (ret == 0) {\n-\t\t\t\tfprintf(stderr, \"No such section!\\n\");\n-\t\t\t\treturn 1;\n-\t\t\t}\n-\t\t\treturn 0;\n-\t\t} else if (!strcmp(argv[1], \"--get-color\")) {\n-\t\t\tif (argc > 4 || argc < 3)\n-\t\t\t\tusage(git_config_set_usage);\n-\t\t\tget_color_slot = argv[2];\n-\t\t\tget_color(argv[3]);\n-\t\t\treturn 0;\n-\t\t} else if (!strcmp(argv[1], \"--get-colorbool\")) {\n-\t\t\tif (argc == 4)\n-\t\t\t\tstdout_is_tty = git_config_bool(\"command line\", argv[3]);\n-\t\t\telse if (argc == 3)\n-\t\t\t\tstdout_is_tty = isatty(1);\n-\t\t\telse\n-\t\t\t\tusage(git_config_set_usage);\n-\t\t\tget_colorbool_slot = argv[2];\n-\t\t\treturn get_colorbool(argc != 3);\n-\t\t} else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n-\t\t\tif (argc != 2)\n-\t\t\t\tusage(git_config_set_usage);\n-\t\t\tgit_config(git_default_config, NULL);\n-\t\t\tlaunch_editor(config_exclusive_filename ?\n-\t\t\t\t      config_exclusive_filename : git_path(\"config\"),\n-\t\t\t\t      NULL, NULL);\n-\t\t\treturn 0;\n-\t\t} else\n-\t\t\tbreak;\n-\t\targc--;\n-\t\targv++;\n-\t}\n-\n-\tswitch (argc) {\n-\tcase 2:\n-\t\treturn get_value(argv[1], NULL);\n-\tcase 3:\n-\t\tif (!strcmp(argv[1], \"--unset\"))\n-\t\t\treturn git_config_set(argv[2], NULL);\n-\t\telse if (!strcmp(argv[1], \"--unset-all\"))\n-\t\t\treturn git_config_set_multivar(argv[2], NULL, NULL, 1);\n-\t\telse if (!strcmp(argv[1], \"--get\"))\n-\t\t\treturn get_value(argv[2], NULL);\n-\t\telse if (!strcmp(argv[1], \"--get-all\")) {\n-\t\t\tdo_all = 1;\n-\t\t\treturn get_value(argv[2], NULL);\n-\t\t} else if (!strcmp(argv[1], \"--get-regexp\")) {\n-\t\t\tshow_keys = 1;\n-\t\t\tuse_key_regexp = 1;\n-\t\t\tdo_all = 1;\n-\t\t\treturn get_value(argv[2], NULL);\n+\targc = parse_options(argc, argv, builtin_config_options, builtin_config_usage,\n+\t\t\t     PARSE_OPT_STOP_AT_NON_OPTION);\n+\n+\tif (use_global_config) {\n+\t\tchar *home = getenv(\"HOME\");\n+\t\tif (home) {\n+\t\t\tchar *user_config = xstrdup(mkpath(\"%s/.gitconfig\", home));\n+\t\t\tconfig_exclusive_filename = user_config;\n \t\t} else {\n-\t\t\tvalue = normalize_value(argv[1], argv[2]);\n-\t\t\treturn git_config_set(argv[1], value);\n+\t\t\tdie(\"$HOME not set\");\n \t\t}\n-\tcase 4:\n-\t\tif (!strcmp(argv[1], \"--unset\"))\n-\t\t\treturn git_config_set_multivar(argv[2], NULL, argv[3], 0);\n-\t\telse if (!strcmp(argv[1], \"--unset-all\"))\n-\t\t\treturn git_config_set_multivar(argv[2], NULL, argv[3], 1);\n-\t\telse if (!strcmp(argv[1], \"--get\"))\n-\t\t\treturn get_value(argv[2], argv[3]);\n-\t\telse if (!strcmp(argv[1], \"--get-all\")) {\n-\t\t\tdo_all = 1;\n-\t\t\treturn get_value(argv[2], argv[3]);\n-\t\t} else if (!strcmp(argv[1], \"--get-regexp\")) {\n-\t\t\tshow_keys = 1;\n-\t\t\tuse_key_regexp = 1;\n-\t\t\tdo_all = 1;\n-\t\t\treturn get_value(argv[2], argv[3]);\n-\t\t} else if (!strcmp(argv[1], \"--add\")) {\n-\t\t\tvalue = normalize_value(argv[2], argv[3]);\n-\t\t\treturn git_config_set_multivar(argv[2], value, \"^$\", 0);\n-\t\t} else if (!strcmp(argv[1], \"--replace-all\")) {\n-\t\t\tvalue = normalize_value(argv[2], argv[3]);\n-\t\t\treturn git_config_set_multivar(argv[2], value, NULL, 1);\n-\t\t} else {\n-\t\t\tvalue = normalize_value(argv[1], argv[2]);\n-\t\t\treturn git_config_set_multivar(argv[1], value, argv[3], 0);\n+\t}\n+\telse if (use_system_config)\n+\t\tconfig_exclusive_filename = git_etc_gitconfig();\n+\telse if (given_config_file) {\n+\t\tif (!is_absolute_path(given_config_file) && prefix)\n+\t\t\tconfig_exclusive_filename = prefix_filename(prefix,\n+\t\t\t\t\t\t\t\t    strlen(prefix),\n+\t\t\t\t\t\t\t\t    argv[2]);\n+\t\telse\n+\t\t\tconfig_exclusive_filename = given_config_file;\n+\t}\n+\n+\tif (end_null) {\n+\t\tterm = '\\0';\n+\t\tdelim = '\\n';\n+\t\tkey_delim = '\\n';\n+\t}\n+\n+\tif (get_color_slot)\n+\t    actions |= ACTION_GET_COLOR;\n+\tif (get_colorbool_slot)\n+\t    actions |= ACTION_GET_COLORBOOL;\n+\n+\tif (HAS_MULTI_BITS(actions)) {\n+\t\terror(\"only one action at a time.\");\n+\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n+\t}\n+\tif (actions == 0)\n+\t\tswitch (argc) {\n+\t\tcase 1: actions = ACTION_GET; break;\n+\t\tcase 2: actions = ACTION_SET; break;\n+\t\tcase 3: actions = ACTION_SET_ALL; break;\n+\t\tdefault:\n+\t\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t\t}\n-\tcase 5:\n-\t\tif (!strcmp(argv[1], \"--replace-all\")) {\n-\t\t\tvalue = normalize_value(argv[2], argv[3]);\n-\t\t\treturn git_config_set_multivar(argv[2], value, argv[4], 1);\n+\n+\tif (actions == ACTION_LIST) {\n+\t\tif (git_config(show_all_config, NULL) < 0) {\n+\t\t\tif (config_exclusive_filename)\n+\t\t\t\tdie(\"unable to read config file %s: %s\",\n+\t\t\t\t    config_exclusive_filename, strerror(errno));\n+\t\t\telse\n+\t\t\t\tdie(\"error processing config file(s)\");\n \t\t}\n-\tcase 1:\n-\tdefault:\n-\t\tusage(git_config_set_usage);\n \t}\n+\telse if (actions == ACTION_EDIT) {\n+\t\tgit_config(git_default_config, NULL);\n+\t\tlaunch_editor(config_exclusive_filename ?\n+\t\t\t      config_exclusive_filename : git_path(\"config\"),\n+\t\t\t      NULL, NULL);\n+\t}\n+\telse if (actions == ACTION_SET) {\n+\t\tcheck_argc(argc, 2, 2);\n+\t\tvalue = normalize_value(argv[0], argv[1]);\n+\t\treturn git_config_set(argv[0], value);\n+\t}\n+\telse if (actions == ACTION_SET_ALL) {\n+\t\tcheck_argc(argc, 2, 3);\n+\t\tvalue = normalize_value(argv[0], argv[1]);\n+\t\treturn git_config_set_multivar(argv[0], value, argv[2], 0);\n+\t}\n+\telse if (actions == ACTION_ADD) {\n+\t\tcheck_argc(argc, 2, 2);\n+\t\tvalue = normalize_value(argv[0], argv[1]);\n+\t\treturn git_config_set_multivar(argv[0], value, \"^$\", 0);\n+\t}\n+\telse if (actions == ACTION_REPLACE_ALL) {\n+\t\tcheck_argc(argc, 2, 3);\n+\t\tvalue = normalize_value(argv[0], argv[1]);\n+\t\treturn git_config_set_multivar(argv[0], value, argv[2], 1);\n+\t}\n+\telse if (actions == ACTION_GET) {\n+\t\tcheck_argc(argc, 1, 2);\n+\t\treturn get_value(argv[0], argv[1]);\n+\t}\n+\telse if (actions == ACTION_GET_ALL) {\n+\t\tdo_all = 1;\n+\t\tcheck_argc(argc, 1, 2);\n+\t\treturn get_value(argv[0], argv[1]);\n+\t}\n+\telse if (actions == ACTION_GET_REGEXP) {\n+\t\tshow_keys = 1;\n+\t\tuse_key_regexp = 1;\n+\t\tdo_all = 1;\n+\t\tcheck_argc(argc, 1, 2);\n+\t\treturn get_value(argv[0], argv[1]);\n+\t}\n+\telse if (actions == ACTION_UNSET) {\n+\t\tcheck_argc(argc, 1, 2);\n+\t\tif (argc == 2)\n+\t\t\treturn git_config_set_multivar(argv[0], NULL, argv[1], 0);\n+\t\telse\n+\t\t\treturn git_config_set(argv[0], NULL);\n+\t}\n+\telse if (actions == ACTION_UNSET_ALL) {\n+\t\tcheck_argc(argc, 1, 2);\n+\t\treturn git_config_set_multivar(argv[0], NULL, argv[1], 1);\n+\t}\n+\telse if (actions == ACTION_RENAME_SECTION) {\n+\t\tint ret;\n+\t\tcheck_argc(argc, 2, 2);\n+\t\tret = git_config_rename_section(argv[0], argv[1]);\n+\t\tif (ret < 0)\n+\t\t\treturn ret;\n+\t\tif (ret == 0)\n+\t\t\tdie(\"No such section!\");\n+\t}\n+\telse if (actions == ACTION_REMOVE_SECTION) {\n+\t\tint ret;\n+\t\tcheck_argc(argc, 1, 1);\n+\t\tret = git_config_rename_section(argv[0], NULL);\n+\t\tif (ret < 0)\n+\t\t\treturn ret;\n+\t\tif (ret == 0)\n+\t\t\tdie(\"No such section!\");\n+\t}\n+\telse if (actions == ACTION_GET_COLOR) {\n+\t\tget_color(argv[0]);\n+\t}\n+\telse if (actions == ACTION_GET_COLORBOOL) {\n+\t\tif (argc == 1)\n+\t\t\tstdout_is_tty = git_config_bool(\"command line\", argv[0]);\n+\t\telse if (argc == 0)\n+\t\t\tstdout_is_tty = isatty(1);\n+\t\treturn get_colorbool(argc != 0);\n+\t}\n+\n \treturn 0;\n }\n-- \n1.6.1.3\n"},{"id":"105676","messageId":"1235177369-2727-3-git-send-email-felipe.contreras@gmail.com","threadId":"17931","inReplyTo":"1235177369-2727-2-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH B v4 2/5] git config: don't allow multiple config file locations","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-21T00:49:26Z","receivedAt":"2009-02-21T00:49:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Either --global, --system, or --file can be used, but not any\ncombination.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 302846e..876eb0e 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -315,6 +315,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \targc = parse_options(argc, argv, builtin_config_options, builtin_config_usage,\n \t\t\t     PARSE_OPT_STOP_AT_NON_OPTION);\n \n+\tif (use_global_config + use_system_config + !!given_config_file > 1) {\n+\t\terror(\"only one config file at a time.\");\n+\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n+\t}\n+\n \tif (use_global_config) {\n \t\tchar *home = getenv(\"HOME\");\n \t\tif (home) {\n-- \n1.6.1.3\n"},{"id":"105680","messageId":"1235177369-2727-4-git-send-email-felipe.contreras@gmail.com","threadId":"17931","inReplyTo":"1235177369-2727-3-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH B v4 3/5] git config: don't allow multiple variable types","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-21T00:49:27Z","receivedAt":"2009-02-21T00:49:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Only --bool, --int, or --bool-or-int can be used, but not any\ncombination of them.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c |   32 ++++++++++++++++++++------------\n 1 files changed, 20 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 876eb0e..c2a30f2 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -19,11 +19,10 @@ static int seen;\n static char delim = '=';\n static char key_delim = ' ';\n static char term = '\\n';\n-static enum { T_RAW, T_INT, T_BOOL, T_BOOL_OR_INT } type = T_RAW;\n \n static int use_global_config, use_system_config;\n static const char *given_config_file;\n-static int actions;\n+static int actions, types;\n static const char *get_color_slot, *get_colorbool_slot;\n static int end_null;\n \n@@ -43,6 +42,10 @@ static int end_null;\n #define ACTION_GET_COLOR (1<<13)\n #define ACTION_GET_COLORBOOL (1<<14)\n \n+#define TYPE_BOOL (1<<0)\n+#define TYPE_INT (1<<1)\n+#define TYPE_BOOL_OR_INT (1<<2)\n+\n static struct option builtin_config_options[] = {\n \tOPT_GROUP(\"Config file location\"),\n \tOPT_BOOLEAN(0, \"global\", &use_global_config, \"use global config file\"),\n@@ -63,9 +66,9 @@ static struct option builtin_config_options[] = {\n \tOPT_STRING(0, \"get-color\", &get_color_slot, \"slot\", \"find the color configured: [default]\"),\n \tOPT_STRING(0, \"get-colorbool\", &get_colorbool_slot, \"slot\", \"find the color setting: [stdout-is-tty]\"),\n \tOPT_GROUP(\"Type\"),\n-\tOPT_SET_INT(0, \"bool\", &type, \"value is \\\"true\\\" or \\\"false\\\"\", T_BOOL),\n-\tOPT_SET_INT(0, \"int\", &type, \"value is decimal number\", T_INT),\n-\tOPT_SET_INT(0, \"bool-or-int\", &type, NULL, T_BOOL_OR_INT),\n+\tOPT_BIT(0, \"bool\", &types, \"value is \\\"true\\\" or \\\"false\\\"\", TYPE_BOOL),\n+\tOPT_BIT(0, \"int\", &types, \"value is decimal number\", TYPE_INT),\n+\tOPT_BIT(0, \"bool-or-int\", &types, NULL, TYPE_BOOL_OR_INT),\n \tOPT_GROUP(\"Other\"),\n \tOPT_BOOLEAN('z', \"null\", &end_null, \"terminate values with NUL byte\"),\n \tOPT_END(),\n@@ -109,11 +112,11 @@ static int show_config(const char *key_, const char *value_, void *cb)\n \t}\n \tif (seen && !do_all)\n \t\tdup_error = 1;\n-\tif (type == T_INT)\n+\tif (types == TYPE_INT)\n \t\tsprintf(value, \"%d\", git_config_int(key_, value_?value_:\"\"));\n-\telse if (type == T_BOOL)\n+\telse if (types == TYPE_BOOL)\n \t\tvptr = git_config_bool(key_, value_) ? \"true\" : \"false\";\n-\telse if (type == T_BOOL_OR_INT) {\n+\telse if (types == TYPE_BOOL_OR_INT) {\n \t\tint is_bool, v;\n \t\tv = git_config_bool_or_int(key_, value_, &is_bool);\n \t\tif (is_bool)\n@@ -212,18 +215,18 @@ static char *normalize_value(const char *key, const char *value)\n \tif (!value)\n \t\treturn NULL;\n \n-\tif (type == T_RAW)\n+\tif (types == 0)\n \t\tnormalized = xstrdup(value);\n \telse {\n \t\tnormalized = xmalloc(64);\n-\t\tif (type == T_INT) {\n+\t\tif (types == TYPE_INT) {\n \t\t\tint v = git_config_int(key, value);\n \t\t\tsprintf(normalized, \"%d\", v);\n \t\t}\n-\t\telse if (type == T_BOOL)\n+\t\telse if (types == TYPE_BOOL)\n \t\t\tsprintf(normalized, \"%s\",\n \t\t\t\tgit_config_bool(key, value) ? \"true\" : \"false\");\n-\t\telse if (type == T_BOOL_OR_INT) {\n+\t\telse if (types == TYPE_BOOL_OR_INT) {\n \t\t\tint is_bool, v;\n \t\t\tv = git_config_bool_or_int(key, value, &is_bool);\n \t\t\tif (!is_bool)\n@@ -346,6 +349,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \t\tkey_delim = '\\n';\n \t}\n \n+\tif (HAS_MULTI_BITS(types)) {\n+\t\terror(\"only one type at a time.\");\n+\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n+\t}\n+\n \tif (get_color_slot)\n \t    actions |= ACTION_GET_COLOR;\n \tif (get_colorbool_slot)\n-- \n1.6.1.3\n"},{"id":"105677","messageId":"1235177369-2727-5-git-send-email-felipe.contreras@gmail.com","threadId":"17931","inReplyTo":"1235177369-2727-4-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH B v4 4/5] git config: don't allow extra arguments for -e or -l.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-21T00:49:28Z","receivedAt":"2009-02-21T00:49:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"As suggested by Johannes Schindelin.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex c2a30f2..8045926 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -373,6 +373,7 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \t\t}\n \n \tif (actions == ACTION_LIST) {\n+\t\tcheck_argc(argc, 0, 0);\n \t\tif (git_config(show_all_config, NULL) < 0) {\n \t\t\tif (config_exclusive_filename)\n \t\t\t\tdie(\"unable to read config file %s: %s\",\n@@ -382,6 +383,7 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \t\t}\n \t}\n \telse if (actions == ACTION_EDIT) {\n+\t\tcheck_argc(argc, 0, 0);\n \t\tgit_config(git_default_config, NULL);\n \t\tlaunch_editor(config_exclusive_filename ?\n \t\t\t      config_exclusive_filename : git_path(\"config\"),\n-- \n1.6.1.3\n"},{"id":"105679","messageId":"1235177369-2727-6-git-send-email-felipe.contreras@gmail.com","threadId":"17931","inReplyTo":"1235177369-2727-5-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH B v4 5/5] git config: don't allow --get-color* and variable type","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-21T00:49:29Z","receivedAt":"2009-02-21T00:49:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Doing so would be incoherent since --get-color would pick a color slot\nand ignore the variable type option (e.g. --bool), and the type would\nrequire a variable name.\n\nSuggested by Junio C Hamano.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 8045926..9930568 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -359,6 +359,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \tif (get_colorbool_slot)\n \t    actions |= ACTION_GET_COLORBOOL;\n \n+\tif ((get_color_slot || get_colorbool_slot) && types) {\n+\t\terror(\"--get-color and variable type are incoherent\");\n+\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n+\t}\n+\n \tif (HAS_MULTI_BITS(actions)) {\n \t\terror(\"only one action at a time.\");\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n-- \n1.6.1.3\n"},{"id":"105681","messageId":"94a0d4530902201653p694bcad6wd79579536f285571@mail.gmail.com","threadId":"17931","inReplyTo":"1235177369-2727-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH B v4 0/5] git config: reorganization","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-21T00:53:16Z","receivedAt":"2009-02-21T00:53:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Feb 21, 2009 at 2:49 AM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> This patch series is the first of a two parts reorganization for more\n> maintainability and user friendliness.\n\nErr, the second.\n\n-- \nFelipe Contreras\n"},{"id":"105775","messageId":"7vskm62z9w.fsf@gitster.siamese.dyndns.org","threadId":"17931","inReplyTo":"1235177369-2727-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH B v4 1/5] git config: reorganize to use parseopt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-22T16:48:43Z","receivedAt":"2009-02-22T16:48:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> +\telse if (actions == ACTION_GET_COLORBOOL) {\n> +\t\tif (argc == 1)\n> +\t\t\tstdout_is_tty = git_config_bool(\"command line\", argv[0]);\n> +\t\telse if (argc == 0)\n> +\t\t\tstdout_is_tty = isatty(1);\n> +\t\treturn get_colorbool(argc != 0);\n> +\t}\n\nI see you fixed this from the last series...\n"},{"id":"105779","messageId":"7vmyce2z9q.fsf@gitster.siamese.dyndns.org","threadId":"17931","inReplyTo":"1235177369-2727-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH B v4 5/5] git config: don't allow --get-color* and variable type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-22T16:48:49Z","receivedAt":"2009-02-22T16:48:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Doing so would be incoherent since --get-color would pick a color slot\n> and ignore the variable type option (e.g. --bool), and the type would\n> require a variable name.\n>\n> Suggested by Junio C Hamano.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  builtin-config.c |    5 +++++\n>  1 files changed, 5 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-config.c b/builtin-config.c\n> index 8045926..9930568 100644\n> --- a/builtin-config.c\n> +++ b/builtin-config.c\n> @@ -359,6 +359,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n>  \tif (get_colorbool_slot)\n>  \t    actions |= ACTION_GET_COLORBOOL;\n>  \n> +\tif ((get_color_slot || get_colorbool_slot) && types) {\n> +\t\terror(\"--get-color and variable type are incoherent\");\n> +\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n> +\t}\n> +\n\nI do not think I suggested anything like this, so I'd decline to take\ncredit for this patch.\n\nStrictly speaking, \"--bool --get-colorbool diff.color 1\" shouldn't error\nout, don't you think?  And it certainly shouldn't say \"--get-color\".\n"},{"id":"105785","messageId":"94a0d4530902220912r94e00d6ha7592bc397baa57c@mail.gmail.com","threadId":"17931","inReplyTo":"7vmyce2z9q.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH B v4 5/5] git config: don't allow --get-color* and variable type","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-22T17:12:07Z","receivedAt":"2009-02-22T17:12:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Feb 22, 2009 at 6:48 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> Doing so would be incoherent since --get-color would pick a color slot\n>> and ignore the variable type option (e.g. --bool), and the type would\n>> require a variable name.\n>>\n>> Suggested by Junio C Hamano.\n>>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  builtin-config.c |    5 +++++\n>>  1 files changed, 5 insertions(+), 0 deletions(-)\n>>\n>> diff --git a/builtin-config.c b/builtin-config.c\n>> index 8045926..9930568 100644\n>> --- a/builtin-config.c\n>> +++ b/builtin-config.c\n>> @@ -359,6 +359,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n>>       if (get_colorbool_slot)\n>>           actions |= ACTION_GET_COLORBOOL;\n>>\n>> +     if ((get_color_slot || get_colorbool_slot) && types) {\n>> +             error(\"--get-color and variable type are incoherent\");\n>> +             usage_with_options(builtin_config_usage, builtin_config_options);\n>> +     }\n>> +\n>\n> I do not think I suggested anything like this, so I'd decline to take\n> credit for this patch.\n>\n> Strictly speaking, \"--bool --get-colorbool diff.color 1\" shouldn't error\n> out, don't you think?  And it certainly shouldn't say \"--get-color\".\n\nHuh? I misinterpreted:\nI see \"git config --bool --get-color diff.color.whitespace\" is still\nallowed, which you might want to tighten further.\n\nI don't really understand the --get-color* options, so please drop the patch.\n\n-- \nFelipe Contreras\n"},{"id":"105788","messageId":"94a0d4530902220919t729915dleed932b010ee76b@mail.gmail.com","threadId":"17931","inReplyTo":"7vskm62z9w.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH B v4 1/5] git config: reorganize to use parseopt","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-22T17:19:07Z","receivedAt":"2009-02-22T17:19:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Feb 22, 2009 at 6:48 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> +     else if (actions == ACTION_GET_COLORBOOL) {\n>> +             if (argc == 1)\n>> +                     stdout_is_tty = git_config_bool(\"command line\", argv[0]);\n>> +             else if (argc == 0)\n>> +                     stdout_is_tty = isatty(1);\n>> +             return get_colorbool(argc != 0);\n>> +     }\n>\n> I see you fixed this from the last series...\n\nYes, argc != 1 was wrong. Now I've made sure all the tests pass.\n\n-- \nFelipe Contreras\n"},{"id":"105792","messageId":"7vmycez86y.fsf@gitster.siamese.dyndns.org","threadId":"17931","inReplyTo":"94a0d4530902220912r94e00d6ha7592bc397baa57c@mail.gmail.com","subject":"Re: [PATCH B v4 5/5] git config: don't allow --get-color* and variable type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-22T17:35:01Z","receivedAt":"2009-02-22T17:35:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>>> diff --git a/builtin-config.c b/builtin-config.c\n>>> index 8045926..9930568 100644\n>>> --- a/builtin-config.c\n>>> +++ b/builtin-config.c\n>>> @@ -359,6 +359,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n>>>       if (get_colorbool_slot)\n>>>           actions |= ACTION_GET_COLORBOOL;\n>>>\n>>> +     if ((get_color_slot || get_colorbool_slot) && types) {\n>>> +             error(\"--get-color and variable type are incoherent\");\n>>> +             usage_with_options(builtin_config_usage, builtin_config_options);\n>>> +     }\n>>> +\n>>\n>> I do not think I suggested anything like this, so I'd decline to take\n>> credit for this patch.\n>>\n>> Strictly speaking, \"--bool --get-colorbool diff.color 1\" shouldn't error\n>> out, don't you think?  And it certainly shouldn't say \"--get-color\".\n>\n> Huh? I misinterpreted:\n> I see \"git config --bool --get-color diff.color.whitespace\" is still\n> allowed, which you might want to tighten further.\n\n\"--get-color\" gets, escape sequence to throw at the terminal to get the\ncolor configured.  It does not make sense to ask for bool (or int) for\nthat action.\n\n\"--get-colorbool\" asks if it is appropriate to use such escape sequence\n(e.g. when the output is tty and config says \"auto\", you would get \"please\nuse color\").  In other words, its type is always bool, so using it as\n--int does not make sense but we cannot really say using it with --bool is\nnonsense.\n\nThe two use formats with and without is_tty are meant to be used in\nscripts this way:\n\n\t# without $is_tty\n\tif git config --get-colorbool color.diff\n        then\n        \tuse_color=true\n\telse\n        \tuse_color=false\n\tfi\n\n        # with $is_tty\n        if test -t 1\n        then\n        \tis_tty=1\n\telse\n        \tis_tty=0\n\tfi\n        use_color=$(git config --get-colorbool color.diff $is_tty)\n\nIn the latter form, output from the command is captured by $() and does\nnot see if the stdout of the calling script is a tty, hence the script\nfeeds that information to the command as an extra parameter.\n"},{"id":"107281","messageId":"94a0d4530903070147v1128e482n9522d60292d206a0@mail.gmail.com","threadId":"17931","inReplyTo":"7vmycez86y.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH B v4 5/5] git config: don't allow --get-color* and variable type","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-03-07T09:47:54Z","receivedAt":"2009-03-07T09:47:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Feb 22, 2009 at 7:35 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>>>> diff --git a/builtin-config.c b/builtin-config.c\n>>>> index 8045926..9930568 100644\n>>>> --- a/builtin-config.c\n>>>> +++ b/builtin-config.c\n>>>> @@ -359,6 +359,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n>>>>       if (get_colorbool_slot)\n>>>>           actions |= ACTION_GET_COLORBOOL;\n>>>>\n>>>> +     if ((get_color_slot || get_colorbool_slot) && types) {\n>>>> +             error(\"--get-color and variable type are incoherent\");\n>>>> +             usage_with_options(builtin_config_usage, builtin_config_options);\n>>>> +     }\n>>>> +\n>>>\n>>> I do not think I suggested anything like this, so I'd decline to take\n>>> credit for this patch.\n>>>\n>>> Strictly speaking, \"--bool --get-colorbool diff.color 1\" shouldn't error\n>>> out, don't you think?  And it certainly shouldn't say \"--get-color\".\n>>\n>> Huh? I misinterpreted:\n>> I see \"git config --bool --get-color diff.color.whitespace\" is still\n>> allowed, which you might want to tighten further.\n>\n> \"--get-color\" gets, escape sequence to throw at the terminal to get the\n> color configured.  It does not make sense to ask for bool (or int) for\n> that action.\n>\n> \"--get-colorbool\" asks if it is appropriate to use such escape sequence\n> (e.g. when the output is tty and config says \"auto\", you would get \"please\n> use color\").  In other words, its type is always bool, so using it as\n> --int does not make sense but we cannot really say using it with --bool is\n> nonsense.\n\nNow I get it, but why would somebody want to do '--bool\n--get-colorbool'? That is redundant.\n\nI think --bool should be used only for boolean variables, not color ones.\n\nPlease let me know if the original patch is ok and I'll resend it with\n'--get-color*'.\n\n-- \nFelipe Contreras\n"}]}