{"thread":{"id":"12913","subject":"[PATCH] Make builtin-config.c use parse-options. (FIRST)","startedAt":"2008-03-29T20:06:08Z","lastAt":"2008-03-29T20:06:08Z","messageCount":1,"participants":["Carlos Rica"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"73332","messageId":"47EEA130.2010406@gmail.com","threadId":"12913","inReplyTo":null,"subject":"[PATCH] Make builtin-config.c use parse-options. (FIRST)","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2008-03-29T20:06:08Z","receivedAt":"2008-03-29T20:06:08Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"Signed-off-by: Carlos Rica <jasampler@gmail.com>\n---\n\n   Adding parse-options makes all parameters starting with \"-\" to\n   be treated as options, so setting configure variables with\n   values like \"-2\" or \"-my-option\" needs to add an explicit\n   \"--\" to stop the arguments' parsing before it reach such value.\n\n   In consequence, those tests setting variable values starting\n   with a dash are currently failing with this patch.\n\n   While the \"--\" solution, IMHO, is the right thing to do because\n   this way is easy to discriminate between options and arguments,\n   it has the problem of breaking existing code and habits related\n   to setting configure variables.\n\n   The parse-options use has some advantages over current solution:\n\n   - Code is more readable, so adding or changing behaviour\n     of one option now is easier (git config has too many). A\n     possibly related problem was exposed while writing the patch:\n     \"git-config --replace-all haha\" command is not properly\n     managed because (I think) current code was distributed\n     among many places in the code.\n\n   - Options order is not significative anymore, so those\n     weird boolean options \"default\" and \"stdout-is-tty\"\n     in --get-color and --get-colorboll now could be properly\n     called --default and --stdout-is-tty and added in any\n     place in command line (I think that they are not options\n     already because that reason).\n\n   - Help message now is not only more complete, but makes\n     documentation to be more in parallel with the options\n     provided by the command, because the -h option shows\n     all of them.\n\n   Pierre was developing a way to process those arguments\n   starting with a dash to be able to leave them in the argv\n   array if you want without stop the parsing, so perhaps\n   it could be a solution to support the old behaviour\n   and also the new parse-options, but I'm not sure how\n   this could be done. Any ideas?\n\n builtin-config.c |  271 ++++++++++++++++++++++++++++++------------------------\n 1 files changed, 152 insertions(+), 119 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex c34bc8b..69d2036 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -1,6 +1,24 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n #include \"color.h\"\n+#include \"parse-options.h\"\n+\n+static const char * const git_config_usage[] = {\n+\t\"git-config [file] [type] [-z|--null] name [value [value_regex]]\",\n+\t\"git-config [file] [type] --add name value\",\n+\t\"git-config [file] [type] --replace-all name [value [value_regex]]\",\n+\t\"git-config [file] [type] [-z|--null] --get name [value_regex]\",\n+\t\"git-config [file] [type] [-z|--null] --get-all name [value_regex]\",\n+\t\"git-config [file] [type] [-z|--null] --get-regexp name_regex [value_regex]\",\n+\t\"git-config [file] --unset name [value_regex]\",\n+\t\"git-config [file] --unset-all name [value_regex]\",\n+\t\"git-config [file] --rename-section old_name new_name\",\n+\t\"git-config [file] --remove-section name\",\n+\t\"git-config [file] [-z|--null] -l | --list\",\n+\t\"git-config [file] --get-color name [default]\",\n+\t\"git-config [file] --get-colorbool name [stdout-is-tty]\",\n+\tNULL\n+};\n\n static const char git_config_set_usage[] =\n \"git-config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --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]\";\n@@ -267,139 +285,154 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \tint nongit;\n \tchar* value;\n \tconst char *file = setup_git_directory_gently(&nongit);\n+\tconst char *file_arg = NULL;\n+\tint null = 0, global = 0, system = 0;\n+\tint ret;\n+\tenum { A_DEFAULT, A_LIST, A_RENAME_SECT, A_REMOVE_SECT,\n+\t\tA_ADD, A_REPLACE_ALL, A_UNSET, A_UNSET_ALL, A_GET,\n+\t\tA_GET_ALL, A_GET_REGEXP, A_GET_COLOR, A_GET_COLORBOOL\n+\t} action = A_DEFAULT;\n+\tstruct option options[] = {\n+\t\tOPT_BOOLEAN('z', \"null\", &null, \"secure output format ending values with null\"),\n+\t\tOPT_SET_INT('l', \"list\", &action, \"list all variables\", A_LIST),\n+\t\tOPT_SET_INT(0, \"rename-section\", &action, \"rename the given section to a new name\", A_RENAME_SECT),\n+\t\tOPT_SET_INT(0, \"remove-section\", &action, \"remove the given section from the file\", A_REMOVE_SECT),\n+\t\tOPT_SET_INT(0, \"add\", &action, \"add a new line to the value of the given key\", A_ADD),\n+\t\tOPT_SET_INT(0, \"replace-all\", &action, \"replace all variables matching\", A_REPLACE_ALL),\n+\t\tOPT_SET_INT(0, \"get\", &action, \"show the value for the given key\", A_GET),\n+\t\tOPT_SET_INT(0, \"get-all\", &action, \"show values of a multivalued key\", A_GET_ALL),\n+\t\tOPT_SET_INT(0, \"get-regexp\", &action, \"find keys using a regular expression\", A_GET_REGEXP),\n+\t\tOPT_SET_INT(0, \"unset\", &action, \"remove the given variable\", A_UNSET),\n+\t\tOPT_SET_INT(0, \"unset-all\", &action, \"remove all variables matching\", A_UNSET_ALL),\n+\t\tOPT_SET_INT(0, \"get-color\", &action, \"ANSI color escape sequence of given color\", A_GET_COLOR),\n+\t\tOPT_SET_INT(0, \"get-colorbool\", &action, \"show if given color is set\", A_GET_COLORBOOL),\n+\t\tOPT_GROUP(\"file and type options\"),\n+\t\tOPT_BOOLEAN(0, \"global\", &global, \"use only the user-specific configuration file\"),\n+\t\tOPT_BOOLEAN(0, \"system\", &system, \"use only the system-wide configuration file\"),\n+\t\tOPT_STRING('f', \"file\", &file_arg, \"config-file\", \"use only the given file instead of the others\"),\n+\t\tOPT_SET_INT(0, \"bool\", &type, \"convert values to type boolean\", T_BOOL),\n+\t\tOPT_SET_INT(0, \"int\", &type, \"convert values to type integer\", T_INT),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, options, git_config_usage, 0);\n+\n+\tif (null) {\n+\t\tterm = '\\0';\n+\t\tdelim = '\\n';\n+\t\tkey_delim = '\\n';\n+\t}\n+\n+\tif (global) {\n+\t\tchar *home = getenv(\"HOME\");\n+\t\tif (home) {\n+\t\t\tchar *user_config = xstrdup(mkpath(\"%s/.gitconfig\", home));\n+\t\t\tsetenv(CONFIG_ENVIRONMENT, user_config, 1);\n+\t\t\tfree(user_config);\n+\t\t} else {\n+\t\t\tdie(\"$HOME not set\");\n+\t\t}\n+\t}\n+\telse if (system)\n+\t\tsetenv(CONFIG_ENVIRONMENT, git_etc_gitconfig(), 1);\n+\telse if (file_arg) {\n+\t\tif (!is_absolute_path(file_arg) && file)\n+\t\t\tfile = prefix_filename(file, strlen(file), file_arg);\n+\t\telse\n+\t\t\tfile = file_arg;\n+\t\tsetenv(CONFIG_ENVIRONMENT, file, 1);\n+\t}\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], \"--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) < 0 && file && errno)\n-\t\t\t\tdie(\"unable to read config file %s: %s\", file,\n+\tswitch (action) {\n+\tcase A_LIST:\n+\t\tif (argc)\n+\t\t\tusage(git_config_set_usage);\n+\t\tif (git_config(show_all_config) < 0 && file && errno)\n+\t\t\tdie(\"unable to read config file %s: %s\", file,\n \t\t\t\t    strerror(errno));\n-\t\t\treturn 0;\n+\t\treturn 0;\n+\tcase A_RENAME_SECT:\n+\t\tif (argc != 2)\n+\t\t\tusage(git_config_set_usage);\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\tfprintf(stderr, \"No such section!\\n\");\n+\t\t\treturn 1;\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\tsetenv(CONFIG_ENVIRONMENT, user_config, 1);\n-\t\t\t\tfree(user_config);\n-\t\t\t} else {\n-\t\t\t\tdie(\"$HOME not set\");\n-\t\t\t}\n+\t\treturn 0;\n+\tcase A_REMOVE_SECT:\n+\t\tif (argc != 1)\n+\t\t\tusage(git_config_set_usage);\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\tfprintf(stderr, \"No such section!\\n\");\n+\t\t\treturn 1;\n \t\t}\n-\t\telse if (!strcmp(argv[1], \"--system\"))\n-\t\t\tsetenv(CONFIG_ENVIRONMENT, git_etc_gitconfig(), 1);\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]) && file)\n-\t\t\t\tfile = prefix_filename(file, strlen(file),\n-\t\t\t\t\t\t       argv[2]);\n-\t\t\telse\n-\t\t\t\tfile = argv[2];\n-\t\t\tsetenv(CONFIG_ENVIRONMENT, file, 1);\n-\t\t\targc--;\n-\t\t\targv++;\n+\t\treturn 0;\n+\tcase A_ADD:\n+\t\tif (argc == 2) {\n+\t\t\tvalue = normalize_value(argv[0], argv[1]);\n+\t\t\treturn git_config_set_multivar(argv[0], value, \"^$\", 0);\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\tusage(git_config_set_usage);\n+\tcase A_DEFAULT:\n+\t\tif (argc == 1)\n+\t\t\treturn get_value(argv[0], NULL);\n+\t\telse if (argc == 2) {\n+\t\t\tvalue = normalize_value(argv[0], argv[1]);\n+\t\t\treturn git_config_set(argv[0], value);\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\telse if (argc == 3) {\n+\t\t\tvalue = normalize_value(argv[0], argv[1]);\n+\t\t\treturn git_config_set_multivar(argv[0], value, argv[2], 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\treturn get_color(argc-2, argv+2);\n-\t\t} else if (!strcmp(argv[1], \"--get-colorbool\")) {\n-\t\t\treturn get_colorbool(argc-2, argv+2);\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-\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\tusage(git_config_set_usage);\n+\tcase A_REPLACE_ALL:\n+\t\tif (argc == 2) {\n+\t\t\tvalue = normalize_value(argv[0], argv[1]);\n+\t\t\treturn git_config_set_multivar(argv[0], value, NULL, 1);\n+\t\t}\n+\t\telse if (argc == 3) {\n+\t\t\tvalue = normalize_value(argv[0], argv[1]);\n+\t\t\treturn git_config_set_multivar(argv[0], value, argv[2], 1);\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\tusage(git_config_set_usage);\n+\tcase A_UNSET:\n+\t\tif (argc == 1)\n+\t\t\treturn git_config_set(argv[0], NULL);\n+\t\telse if (argc == 2)\n+\t\t\treturn git_config_set_multivar(argv[0], NULL, argv[1], 0);\n+\t\tusage(git_config_set_usage);\n+\tcase A_UNSET_ALL:\n+\t\tif (argc == 1)\n+\t\t\treturn git_config_set_multivar(argv[0], NULL, NULL, 1);\n+\t\telse if (argc == 2)\n+\t\t\treturn git_config_set_multivar(argv[0], NULL, argv[1], 1);\n+\t\tusage(git_config_set_usage);\n+\tcase A_GET:\n+\tcase A_GET_ALL:\n+\tcase A_GET_REGEXP:\n+\t\tif (action == A_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\telse if (action == A_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\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-\t\t}\n-\tcase 1:\n-\tdefault:\n+\t\tif (argc == 1)\n+\t\t\treturn get_value(argv[0], NULL);\n+\t\telse if (argc == 2)\n+\t\t\treturn get_value(argv[0], argv[1]);\n \t\tusage(git_config_set_usage);\n+\tcase A_GET_COLOR:\n+\t\treturn get_color(argc, argv);\n+\tcase A_GET_COLORBOOL:\n+\t\treturn get_colorbool(argc, argv);\n \t}\n+\tusage(git_config_set_usage);\n \treturn 0;\n }\n-- \n1.5.3.4\n"}]}