{"thread":{"id":"17837","subject":"[PATCH v2 1/8] config: Trivial rename in preparation for parseopt.","startedAt":"2009-02-17T00:54:47Z","lastAt":"2009-02-17T13:59:28Z","messageCount":24,"participants":["Felipe Contreras","Junio C Hamano","Gerrit Pape","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":8},"messages":[{"id":"105040","messageId":"1234832094-15541-1-git-send-email-felipe.contreras@gmail.com","threadId":"17837","inReplyTo":null,"subject":"[PATCH v2 1/8] config: Trivial rename in preparation for parseopt.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T00:54:47Z","receivedAt":"2009-02-17T00:54:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Essensially this replaces 'file' with 'prefix' in the cases where the\nvariable is used as a prefix, which is consistent with other git\ncommands.\n\nWhen using the --list option general errors where not properly reported,\nonly errors related with the 'file'. Now they are reported, and 'file'\nis irrelevant.\n\nThat reduces the rest of 'file' usage to nothing, therefore now only\n'prefix' remains.\n\nSuggested by Johannes Schindelin.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c |   27 +++++++++++++--------------\n 1 files changed, 13 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 6937eaf..da754e0 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -178,6 +178,7 @@ static char *normalize_value(const char *key, const char *value)\n \n static int get_color_found;\n static const char *get_color_slot;\n+static const char *get_colorbool_slot;\n static char parsed_color[COLOR_MAXLEN];\n \n static int git_get_color_config(const char *var, const char *value, void *cb)\n@@ -231,7 +232,7 @@ static int get_diff_color_found;\n static int git_get_colorbool_config(const char *var, const char *value,\n \t\tvoid *cb)\n {\n-\tif (!strcmp(var, get_color_slot)) {\n+\tif (!strcmp(var, get_colorbool_slot)) {\n \t\tget_colorbool_found =\n \t\t\tgit_config_colorbool(var, value, stdout_is_tty);\n \t}\n@@ -263,11 +264,11 @@ static int get_colorbool(int argc, const char **argv)\n \t\tusage(git_config_set_usage);\n \tget_colorbool_found = -1;\n \tget_diff_color_found = -1;\n-\tget_color_slot = argv[0];\n+\tget_colorbool_slot = argv[0];\n \tgit_config(git_get_colorbool_config, NULL);\n \n \tif (get_colorbool_found < 0) {\n-\t\tif (!strcmp(get_color_slot, \"color.diff\"))\n+\t\tif (!strcmp(get_colorbool_slot, \"color.diff\"))\n \t\t\tget_colorbool_found = get_diff_color_found;\n \t\tif (get_colorbool_found < 0)\n \t\t\tget_colorbool_found = git_use_color_default;\n@@ -281,11 +282,11 @@ static int get_colorbool(int argc, const char **argv)\n \t}\n }\n \n-int cmd_config(int argc, const char **argv, const char *prefix)\n+int cmd_config(int argc, const char **argv, const char *unused_prefix)\n {\n \tint nongit;\n \tchar* value;\n-\tconst char *file = setup_git_directory_gently(&nongit);\n+\tconst char *prefix = setup_git_directory_gently(&nongit);\n \n \tconfig_exclusive_filename = getenv(CONFIG_ENVIRONMENT);\n \n@@ -299,10 +300,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\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\t\tfile && errno)\n-\t\t\t\tdie(\"unable to read config file %s: %s\", file,\n-\t\t\t\t    strerror(errno));\n+\t\t\tif (git_config(show_all_config, NULL) < 0)\n+\t\t\t\tdie(\"error processing config file(s)\");\n \t\t\treturn 0;\n \t\t}\n \t\telse if (!strcmp(argv[1], \"--global\")) {\n@@ -319,12 +318,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\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\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\tfile = argv[2];\n-\t\t\tconfig_exclusive_filename = file;\n+\t\t\t\tconfig_exclusive_filename = argv[2];\n \t\t\targc--;\n \t\t\targv++;\n \t\t}\n-- \n1.6.1.3\n"},{"id":"105042","messageId":"1234832094-15541-2-git-send-email-felipe.contreras@gmail.com","threadId":"17837","inReplyTo":"1234832094-15541-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 2/8] config: Reorganize get_color*.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T00:54:48Z","receivedAt":"2009-02-17T00:54:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"In preparation for parseopt.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c |   63 +++++++++++++++--------------------------------------\n 1 files changed, 18 insertions(+), 45 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex da754e0..8fd6171 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -180,7 +180,6 @@ static int get_color_found;\n static const char *get_color_slot;\n static const char *get_colorbool_slot;\n static char parsed_color[COLOR_MAXLEN];\n-\n static int git_get_color_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, get_color_slot)) {\n@@ -192,29 +191,8 @@ static int git_get_color_config(const char *var, const char *value, void *cb)\n \treturn 0;\n }\n \n-static int get_color(int argc, const char **argv)\n+static void get_color(const char *def_color)\n {\n-\t/*\n-\t * grab the color setting for the given slot from the configuration,\n-\t * or parse the default value if missing, and return ANSI color\n-\t * escape sequence.\n-\t *\n-\t * e.g.\n-\t * git config --get-color color.diff.whitespace \"blue reverse\"\n-\t */\n-\tconst char *def_color = NULL;\n-\n-\tswitch (argc) {\n-\tdefault:\n-\t\tusage(git_config_set_usage);\n-\tcase 2:\n-\t\tdef_color = argv[1];\n-\t\t/* fallthru */\n-\tcase 1:\n-\t\tget_color_slot = argv[0];\n-\t\tbreak;\n-\t}\n-\n \tget_color_found = 0;\n \tparsed_color[0] = '\\0';\n \tgit_config(git_get_color_config, NULL);\n@@ -223,7 +201,6 @@ static int get_color(int argc, const char **argv)\n \t\tcolor_parse(def_color, \"command line\", parsed_color);\n \n \tfputs(parsed_color, stdout);\n-\treturn 0;\n }\n \n static int stdout_is_tty;\n@@ -247,24 +224,10 @@ static int git_get_colorbool_config(const char *var, const char *value,\n \treturn 0;\n }\n \n-static int get_colorbool(int argc, const char **argv)\n+static int get_colorbool(int print)\n {\n-\t/*\n-\t * git config --get-colorbool <slot> [<stdout-is-tty>]\n-\t *\n-\t * returns \"true\" or \"false\" depending on how <slot>\n-\t * is configured.\n-\t */\n-\n-\tif (argc == 2)\n-\t\tstdout_is_tty = git_config_bool(\"command line\", argv[1]);\n-\telse if (argc == 1)\n-\t\tstdout_is_tty = isatty(1);\n-\telse\n-\t\tusage(git_config_set_usage);\n \tget_colorbool_found = -1;\n \tget_diff_color_found = -1;\n-\tget_colorbool_slot = argv[0];\n \tgit_config(git_get_colorbool_config, NULL);\n \n \tif (get_colorbool_found < 0) {\n@@ -274,12 +237,11 @@ static int get_colorbool(int argc, const char **argv)\n \t\t\tget_colorbool_found = git_use_color_default;\n \t}\n \n-\tif (argc == 1) {\n-\t\treturn get_colorbool_found ? 0 : 1;\n-\t} else {\n+\tif (print) {\n \t\tprintf(\"%s\\n\", get_colorbool_found ? \"true\" : \"false\");\n \t\treturn 0;\n-\t}\n+\t} else\n+\t\treturn get_colorbool_found ? 0 : 1;\n }\n \n int cmd_config(int argc, const char **argv, const char *unused_prefix)\n@@ -358,9 +320,20 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\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\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\treturn get_colorbool(argc-2, argv+2);\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\tconst char *config_filename;\n \t\t\tif (argc != 2)\n-- \n1.6.1.3\n"},{"id":"105047","messageId":"1234832094-15541-3-git-send-email-felipe.contreras@gmail.com","threadId":"17837","inReplyTo":"1234832094-15541-2-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 3/8] config: Use parseopt.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T00:54:49Z","receivedAt":"2009-02-17T00:54:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Reorganizing the code to use parseopt as suggested by Johannes\nSchindelin.\n\nThis 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 |  325 +++++++++++++++++++++++++++++-------------------------\n 1 files changed, 173 insertions(+), 152 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 8fd6171..084222a 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,59 @@ 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+\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),\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_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_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(\"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,160 +308,125 @@ 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\tdie(\"error processing config file(s)\");\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\tconst char *config_filename;\n-\t\t\tif (argc != 2)\n-\t\t\t\tusage(git_config_set_usage);\n-\t\t\tif (config_exclusive_filename)\n-\t\t\t\tconfig_filename = config_exclusive_filename;\n-\t\t\telse\n-\t\t\t\tconfig_filename = git_path(\"config\");\n-\t\t\tgit_config(git_default_config, NULL);\n-\t\t\tlaunch_editor(config_filename, NULL, NULL);\n-\t\t\treturn 0;\n-\t\t} else\n-\t\t\tbreak;\n-\t\targc--;\n-\t\targv++;\n-\t}\n+\targc = parse_options(argc, argv, builtin_config_options, builtin_config_usage, 0);\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+\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\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}\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 (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_ADD; break;\n+\t\tcase 3: actions |= ACTION_REPLACE_ALL; break;\n+\t\tdefault:\n+\t\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t\t}\n-\tcase 1:\n-\tdefault:\n-\t\tusage(git_config_set_usage);\n+\n+\tif (actions & ACTION_LIST) {\n+\t\tif (git_config(show_all_config, NULL) < 0)\n+\t\t\tdie(\"error processing config file(s)\");\n+\t}\n+\telse if (actions & ACTION_EDIT) {\n+\t\tconst char *config_filename;\n+\t\tif (config_exclusive_filename)\n+\t\t\tconfig_filename = config_exclusive_filename;\n+\t\telse\n+\t\t\tconfig_filename = git_path(\"config\");\n+\t\tgit_config(git_default_config, NULL);\n+\t\tlaunch_editor(config_filename, NULL, NULL);\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 (get_color_slot) {\n+\t\tget_color(argv[0]);\n+\t}\n+\telse if (get_colorbool_slot) {\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 != 1);\n+\t}\n+\n \treturn 0;\n }\n-- \n1.6.1.3\n"},{"id":"105044","messageId":"1234832094-15541-4-git-send-email-felipe.contreras@gmail.com","threadId":"17837","inReplyTo":"1234832094-15541-3-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 4/8] config: Disallow multiple variable types.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T00:54:50Z","receivedAt":"2009-02-17T00:54:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"So now only either --bool, --int, or --bool-or-int can be used, but not\nany combination 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 084222a..30de93e 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@@ -39,6 +38,10 @@ static int end_null;\n #define ACTION_LIST (1<<9)\n #define ACTION_EDIT (1<<10)\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@@ -57,9 +60,9 @@ static struct option builtin_config_options[] = {\n \tOPT_BIT('l', \"list\", &actions, \"list all\", ACTION_LIST),\n \tOPT_BIT('e', \"edit\", &actions, \"opens an editor\", ACTION_EDIT),\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_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(\"Other\"),\n@@ -105,11 +108,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@@ -208,18 +211,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@@ -336,6 +339,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 (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":"105048","messageId":"1234832094-15541-5-git-send-email-felipe.contreras@gmail.com","threadId":"17837","inReplyTo":"1234832094-15541-4-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 5/8] config: Disallow multiple config file locations.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T00:54:51Z","receivedAt":"2009-02-17T00:54:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Either --global, --system, or --file should 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 30de93e..cdd8a12 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -313,6 +313,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \n \targc = parse_options(argc, argv, builtin_config_options, builtin_config_usage, 0);\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":"105045","messageId":"1234832094-15541-6-git-send-email-felipe.contreras@gmail.com","threadId":"17837","inReplyTo":"1234832094-15541-5-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 6/8] config: Don't allow extra arguments for -e or -l.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T00:54:52Z","receivedAt":"2009-02-17T00:54:52Z","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 cdd8a12..5891a4e 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -363,10 +363,12 @@ 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\tdie(\"error processing config file(s)\");\n \t}\n \telse if (actions & ACTION_EDIT) {\n+\t\tcheck_argc(argc, 0, 0);\n \t\tconst char *config_filename;\n \t\tif (config_exclusive_filename)\n \t\t\tconfig_filename = config_exclusive_filename;\n-- \n1.6.1.3\n"},{"id":"105046","messageId":"1234832094-15541-7-git-send-email-felipe.contreras@gmail.com","threadId":"17837","inReplyTo":"1234832094-15541-6-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 7/8] config: Codestyle cleanups.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T00:54:53Z","receivedAt":"2009-02-17T00:54:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 5891a4e..ff9e029 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -86,7 +86,7 @@ static int show_all_config(const char *key_, const char *value_, void *cb)\n \treturn 0;\n }\n \n-static int show_config(const char* key_, const char* value_, void *cb)\n+static int show_config(const char *key_, const char *value_, void *cb)\n {\n \tchar value[256];\n \tconst char *vptr = value;\n@@ -133,7 +133,7 @@ static int show_config(const char* key_, const char* value_, void *cb)\n \treturn 0;\n }\n \n-static int get_value(const char* key_, const char* regex_)\n+static int get_value(const char *key_, const char *regex_)\n {\n \tint ret = -1;\n \tchar *tl;\n@@ -306,7 +306,7 @@ static int get_colorbool(int print)\n int cmd_config(int argc, const char **argv, const char *unused_prefix)\n {\n \tint nongit;\n-\tchar* value;\n+\tchar *value;\n \tconst char *prefix = setup_git_directory_gently(&nongit);\n \n \tconfig_exclusive_filename = getenv(CONFIG_ENVIRONMENT);\n-- \n1.6.1.3\n"},{"id":"105043","messageId":"1234832094-15541-8-git-send-email-felipe.contreras@gmail.com","threadId":"17837","inReplyTo":"1234832094-15541-7-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 8/8] config: Cleanup editor action.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T00:54:54Z","receivedAt":"2009-02-17T00:54:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Copy-paste from Johannes Schindelin.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c |    9 +++------\n 1 files changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex ff9e029..bc97854 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -369,13 +369,10 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \t}\n \telse if (actions & ACTION_EDIT) {\n \t\tcheck_argc(argc, 0, 0);\n-\t\tconst char *config_filename;\n-\t\tif (config_exclusive_filename)\n-\t\t\tconfig_filename = config_exclusive_filename;\n-\t\telse\n-\t\t\tconfig_filename = git_path(\"config\");\n \t\tgit_config(git_default_config, NULL);\n-\t\tlaunch_editor(config_filename, NULL, 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_ADD) {\n \t\tcheck_argc(argc, 2, 2);\n-- \n1.6.1.3\n"},{"id":"105051","messageId":"94a0d4530902161700n734b1ee0p22672757b978b850@mail.gmail.com","threadId":"17837","inReplyTo":"1234832094-15541-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 2/8] config: Reorganize get_color*.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T01:00:20Z","receivedAt":"2009-02-17T01:00:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Feb 17, 2009 at 2:54 AM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> In preparation for parseopt.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n\n<snip/>\n\n> @@ -274,12 +237,11 @@ static int get_colorbool(int argc, const char **argv)\n>                        get_colorbool_found = git_use_color_default;\n>        }\n>\n> -       if (argc == 1) {\n> -               return get_colorbool_found ? 0 : 1;\n> -       } else {\n> +       if (print) {\n>                printf(\"%s\\n\", get_colorbool_found ? \"true\" : \"false\");\n>                return 0;\n> -       }\n> +       } else\n> +               return get_colorbool_found ? 0 : 1;\n>  }\n\n<snip/>\n\n>                } else if (!strcmp(argv[1], \"--get-colorbool\")) {\n> -                       return get_colorbool(argc-2, argv+2);\n> +                       if (argc == 4)\n> +                               stdout_is_tty = git_config_bool(\"command line\", argv[3]);\n> +                       else if (argc == 3)\n> +                               stdout_is_tty = isatty(1);\n> +                       else\n> +                               usage(git_config_set_usage);\n> +                       get_colorbool_slot = argv[2];\n> +                       return get_colorbool(argc == 3);\n\nAgh, crap, that should be argc != 3. Anyway, the next patch does it properly.\n\n-- \nFelipe Contreras\n"},{"id":"105057","messageId":"7v3aedet0j.fsf@gitster.siamese.dyndns.org","threadId":"17837","inReplyTo":"1234832094-15541-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 1/8] config: Trivial rename in preparation for parseopt.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-17T01:45:00Z","receivedAt":"2009-02-17T01:45:00Z","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> When using the --list option general errors where not properly reported,\n> only errors related with the 'file'. Now they are reported, and 'file'\n> is irrelevant.\n> ...\n> @@ -299,10 +300,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\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\t\tfile && errno)\n> -\t\t\t\tdie(\"unable to read config file %s: %s\", file,\n> -\t\t\t\t    strerror(errno));\n> +\t\t\tif (git_config(show_all_config, NULL) < 0)\n> +\t\t\t\tdie(\"error processing config file(s)\");\n\nDoes the author of 93a56c2 (git-config: print error message if the config\nfile cannot be read, 2007-10-12) have any comment on this change (cc:ed)?\n"},{"id":"105060","messageId":"7vbpt1by1y.fsf@gitster.siamese.dyndns.org","threadId":"17837","inReplyTo":"1234832094-15541-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 3/8] config: Use parseopt.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-17T02:24:25Z","receivedAt":"2009-02-17T02:24:25Z","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> +\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\nMy initial reaction was:\n\n\tCan we easily say \"--get and --getall are mutually incompatible\"?\n\nand knowing that it would take much more code, the second reaction was:\n\n\tDoes the user know what we mean by \"action\"?\n\nSince the answer to this is \"Yes, the usage message comes from parseopt\nand there is a clear categorization\", I think the message is good enough.\n\nWhat happens when the user says \"config --get --get-colorbool user.name\"?\nIs it an error?  Is it diagnosed as an error?\n\nIt probably is easy to fix it by defining two bits of fake actions and do:\n\n\tif (get_color_slot)\n        \tactions |= ACTION_GET_COLOR;\n\tif (get_colorbool_slot)\n        \tactions |= ACTION_GET_COLORBOOL;\n\nimmediately before this HAS_MULTI_BITS check.\n\nI know I suggested these to are type-like, but I realize that these two\nare better categorized as actions tied to a specific type (color), as you\nhad in your earlier round.\n\n> +\tif (actions == 0)\n> +\t\tswitch (argc) {\n> +\t\tcase 1: actions |= ACTION_GET; break;\n> +\t\tcase 2: actions |= ACTION_ADD; break;\n> +\t\tcase 3: actions |= ACTION_REPLACE_ALL; break;\n\nStraight assignment, not ORing-in please.  It wastes a few seconds from\nthe reader wondering what other bits in the variable \"actions\" are used\nfor things other than ACTION_* (the answer is none).\n\nSimilarly, later conditions:\n\n> +\tif (actions & ACTION_LIST) {\n\nwould read better if they used equality == checks.\n"},{"id":"105061","messageId":"7v63j9by1s.fsf@gitster.siamese.dyndns.org","threadId":"17837","inReplyTo":"1234832094-15541-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 2/8] config: Reorganize get_color*.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-17T02:24:31Z","receivedAt":"2009-02-17T02:24:31Z","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> In preparation for parseopt.\n\nI like this patch, because it clarifies what get_colorbool() and\nget_color() functions are meant to do by moving the boundary of\nresponsibility between the two callers and these two functions.\n\nThe above log message does not do justice to the patch text.\n\n>  builtin-config.c |   63 +++++++++++++++--------------------------------------\n>  1 files changed, 18 insertions(+), 45 deletions(-)\n\nI like a patch that results in code reduction, so I got quite interested\nin seeing what you did.  But there was no magic --- you lost a lot of\ncomments on what each function is supposed to do.\n\nThey are all described in the documentation, and removal of these comments\nthat can go stale is probably a good thing, but you could have avoided\ndissapointing me who expected a magic by mentioning the removal of the\ncomments (and why it is a good idea) upfront ;-)\n"},{"id":"105062","messageId":"7vy6w5ajgs.fsf@gitster.siamese.dyndns.org","threadId":"17837","inReplyTo":"1234832094-15541-4-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 4/8] config: Disallow multiple variable types.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-17T02:24:51Z","receivedAt":"2009-02-17T02:24:51Z","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> +\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\nYou would be able to catch \"config --bool --get-color color.diff.whitespace\"\nas an error by doing something similar to the \"fake actions\" for get-color*\nas I suggested in my review on [3/8].\n\nI did not find anything objectionable in the remainder of the series.\n\nThanks.\n"},{"id":"105063","messageId":"7vskmdaj9z.fsf@gitster.siamese.dyndns.org","threadId":"17837","inReplyTo":"1234832094-15541-8-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 8/8] config: Cleanup editor action.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-17T02:28:56Z","receivedAt":"2009-02-17T02:28:56Z","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> Copy-paste from Johannes Schindelin.\n\nSorry, I do not understand.  Are you saying:\n\n (1) This duplicated code was introduced by copy-and-paste by Dscho and I\n     am cleaning up his mess?\n\nor\n\n (2) This change came from Dscho and I am just sending it by copy-pasting\n     what he did?\n\nIf the latter, then this is not a good way to describe what happened.  You\nwould use in-body From: header to give attribution, and more importantly,\nyou need to say what you would usually say in the commit log message (what\nit does, why the original is bad, etc.)\n"},{"id":"105064","messageId":"94a0d4530902161842s1d1d9fech3786cce1f1a1135d@mail.gmail.com","threadId":"17837","inReplyTo":"7v3aedet0j.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/8] config: Trivial rename in preparation for parseopt.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T02:42:38Z","receivedAt":"2009-02-17T02:42:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Feb 17, 2009 at 3:45 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> When using the --list option general errors where not properly reported,\n>> only errors related with the 'file'. Now they are reported, and 'file'\n>> is irrelevant.\n>> ...\n>> @@ -299,10 +300,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n>>               else if (!strcmp(argv[1], \"--list\") || !strcmp(argv[1], \"-l\")) {\n>>                       if (argc != 2)\n>>                               usage(git_config_set_usage);\n>> -                     if (git_config(show_all_config, NULL) < 0 &&\n>> -                                     file && errno)\n>> -                             die(\"unable to read config file %s: %s\", file,\n>> -                                 strerror(errno));\n>> +                     if (git_config(show_all_config, NULL) < 0)\n>> +                             die(\"error processing config file(s)\");\n>\n> Does the author of 93a56c2 (git-config: print error message if the config\n> file cannot be read, 2007-10-12) have any comment on this change (cc:ed)?\n\nI looked at the debian bug report[1], the guy complains about 2 things:\n\n1) git-config --file fails silently if the filename isn't absolute\n\nThis is still fixed.\n\n2) git-config -l --file doesn't do what you might expect and list the\ncontents of the specified file. Instead it ignores the --file option since it\ncame after the -l. Nice way to shoot oneself in the foot.\n\nThis is now fixed after the parseopt patch.\n\nAlso, before this patch 'git config --global -l' would fail silently\nif there isn't any ~/.gitconfig. Now at least git reports \"error\nprocessing config file(s)\".\n\nA more ideal solution would be:\n\nif (config_exclusive_filename)\n\tdie(\"unable to read config file %s: %s\",\n\t    config_exclusive_filename, strerror(errno));\nelse\n\tdie(\"error processing config file(s)\");\n\nSo, if a file is specified with --file, --global, or --system, then\nthe correct error would be reported.\n\nI digged a bit more and it turns out if there's parse error\ngit_config() will die immediately, and the only time it will return a\nnegative value is when the config file(s) are not present, at which\npoint there will be an errno set, and when config_exclusive_filename\nis specified that means the errno will be the one of fopen trying to\nopen that file.\n\nStill, \"error processing config file(s)\" will be reported when no file\nis specified 'git config -l', there isn't any repo config file (cwd is\nnot in a git repo).\n\nEven better I think would be to allow 'git config -l' to work even if\nwe are not in a git repo, and error only when there isn't any config\nfile (repo, system or global).\n\nThis is how it would look like:\n\n int git_config(config_fn_t fn, void *data)\n {\n-       int ret = 0;\n+       int ret = 0, found = 0;\n        char *repo_config = NULL;\n        const char *home = NULL;\n\n        /* Setting $GIT_CONFIG makes git read _only_ the given config file. */\n        if (config_exclusive_filename)\n                return git_config_from_file(fn,\nconfig_exclusive_filename, data);\n-       if (git_config_system() && !access(git_etc_gitconfig(), R_OK))\n+       if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {\n                ret += git_config_from_file(fn, git_etc_gitconfig(),\n                                            data);\n+               found += 1;\n+       }\n\n        home = getenv(\"HOME\");\n        if (git_config_global() && home) {\n                char *user_config = xstrdup(mkpath(\"%s/.gitconfig\", home));\n-               if (!access(user_config, R_OK))\n+               if (!access(user_config, R_OK)) {\n                        ret += git_config_from_file(fn, user_config, data);\n+                       found += 1;\n+               }\n                free(user_config);\n        }\n\n        repo_config = git_pathdup(\"config\");\n-       ret += git_config_from_file(fn, repo_config, data);\n+       if (!access(repo_config, R_OK)) {\n+               ret += git_config_from_file(fn, repo_config, data);\n+               found += 1;\n+       }\n        free(repo_config);\n+       if (found == 0)\n+               error(\"no config file found\");\n        return ret;\n }\n\n[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=445208\n\n-- \nFelipe Contreras\n"},{"id":"105075","messageId":"7vab8laa7p.fsf@gitster.siamese.dyndns.org","threadId":"17837","inReplyTo":"1234832094-15541-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 3/8] config: Use parseopt.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-17T05:44:42Z","receivedAt":"2009-02-17T05:44:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I've queued the entire series on top of fc/config-editor topic and even\nmerged the result in 'pu' once, but I had to reintegrate 'pu' without the\nseries.\n\nBefore this commit, t/t1300-repo-config.sh passes, but this one breaks\nthe test.\n"},{"id":"105107","messageId":"20090217090015.5334.qmail@ea511e23bd6d9a.315fe32.mid.smarden.org","threadId":"17837","inReplyTo":"7v3aedet0j.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/8] config: Trivial rename in preparation for parseopt.","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2009-02-17T09:00:15Z","receivedAt":"2009-02-17T09:00:15Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Mon, Feb 16, 2009 at 05:45:00PM -0800, Junio C Hamano wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n> > When using the --list option general errors where not properly reported,\n> > only errors related with the 'file'. Now they are reported, and 'file'\n> > is irrelevant.\n> > ...\n> > @@ -299,10 +300,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\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\t\tfile && errno)\n> > -\t\t\t\tdie(\"unable to read config file %s: %s\", file,\n> > -\t\t\t\t    strerror(errno));\n> > +\t\t\tif (git_config(show_all_config, NULL) < 0)\n> > +\t\t\t\tdie(\"error processing config file(s)\");\n> \n> Does the author of 93a56c2 (git-config: print error message if the config\n> file cannot be read, 2007-10-12) have any comment on this change (cc:ed)?\n\nHm, we lose some information from the error message when called with\n--file, but it seems to improve on other cases.  The filename doesn't\nmatter that much, but it would be nice to know the reason.  I wouldn't\nobject against this hunk though if that isn't possible.\n\nRegards, Gerrit.\n"},{"id":"105117","messageId":"94a0d4530902170235g4e481e07t39a6157894dff9aa@mail.gmail.com","threadId":"17837","inReplyTo":"7vab8laa7p.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 3/8] config: Use parseopt.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T10:35:07Z","receivedAt":"2009-02-17T10:35:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Feb 17, 2009 at 7:44 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> I've queued the entire series on top of fc/config-editor topic and even\n> merged the result in 'pu' once, but I had to reintegrate 'pu' without the\n> series.\n>\n> Before this commit, t/t1300-repo-config.sh passes, but this one breaks\n> the test.\n\nAh, I didn't know there was a test for that.\n\nI've fixed most the issues but unfortunately parseopt barfs when -1 is\nused as an argument. That should be fixed somehow, otherwise this\npatch will never pass the test.\n\nI'm attaching a patch that makes the test pass (for review), but\nshouldn't be merged.\n\n-- \nFelipe Contreras\n\n\nFrom 22e4d1a472027d2b7f650a99f487fef78ef3b8ad Mon Sep 17 00:00:00 2001\nFrom: Felipe Contreras <felipe.contreras@gmail.com>\nDate: Tue, 17 Feb 2009 12:30:26 +0200\nSubject: [NOMERGE/PATCH] config: Fixes parseopt for t1300-repo-config.\n\nThis is patch isn't meant for merging, just for review.\n\nIdeally parseopt shouldn't fail with -1 as an argument.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin-config.c       |   18 +++++++++++++++---\n t/t1300-repo-config.sh |    8 ++++----\n 2 files changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 084222a..19f274a 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -38,6 +38,8 @@ static int end_null;\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 \n static struct option builtin_config_options[] = {\n \tOPT_GROUP(\"Config file location\"),\n@@ -46,7 +48,7 @@ static struct option builtin_config_options[] = {\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),\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@@ -343,8 +345,8 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \tif (actions == 0)\n \t\tswitch (argc) {\n \t\tcase 1: actions |= ACTION_GET; break;\n-\t\tcase 2: actions |= ACTION_ADD; break;\n-\t\tcase 3: actions |= ACTION_REPLACE_ALL; 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@@ -362,6 +364,16 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)\n \t\tgit_config(git_default_config, NULL);\n \t\tlaunch_editor(config_filename, 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]);\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 11b82f4..be7104d 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -528,7 +528,7 @@ EOF\n test_expect_success bool '\n \n \tgit config bool.true1 01 &&\n-\tgit config bool.true2 -1 &&\n+\tgit config bool.true2 -- -1 &&\n \tgit config bool.true3 YeS &&\n \tgit config bool.true4 true &&\n \tgit config bool.false1 000 &&\n@@ -569,7 +569,7 @@ EOF\n test_expect_success 'set --bool' '\n \n \tgit config --bool bool.true1 01 &&\n-\tgit config --bool bool.true2 -1 &&\n+\tgit config --bool bool.true2 -- -1 &&\n \tgit config --bool bool.true3 YeS &&\n \tgit config --bool bool.true4 true &&\n \tgit config --bool bool.false1 000 &&\n@@ -590,7 +590,7 @@ EOF\n test_expect_success 'set --int' '\n \n \tgit config --int int.val1 01 &&\n-\tgit config --int int.val2 -1 &&\n+\tgit config --int int.val2 -- -1 &&\n \tgit config --int int.val3 5m &&\n \tcmp expect .git/config'\n \n@@ -648,7 +648,7 @@ test_expect_success 'set --bool-or-int' '\n \tgit config --bool-or-int bool.false2 no &&\n \tgit config --bool-or-int int.int1 0 &&\n \tgit config --bool-or-int int.int2 1 &&\n-\tgit config --bool-or-int int.int3 -1 &&\n+\tgit config --bool-or-int int.int3 -- -1 &&\n \ttest_cmp expect .git/config\n '\n \n-- \n1.6.1.3\n\n"},{"id":"105126","messageId":"alpine.DEB.1.00.0902171255220.6185@intel-tinevez-2-302","threadId":"17837","inReplyTo":"94a0d4530902170235g4e481e07t39a6157894dff9aa@mail.gmail.com","subject":"Re: [PATCH v2 3/8] config: Use parseopt.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-17T11:55:38Z","receivedAt":"2009-02-17T11:55:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Feb 2009, Felipe Contreras wrote:\n\n> On Tue, Feb 17, 2009 at 7:44 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> > I've queued the entire series on top of fc/config-editor topic and even\n> > merged the result in 'pu' once, but I had to reintegrate 'pu' without the\n> > series.\n> >\n> > Before this commit, t/t1300-repo-config.sh passes, but this one breaks\n> > the test.\n> \n> Ah, I didn't know there was a test for that.\n> \n> I've fixed most the issues but unfortunately parseopt barfs when -1 is\n> used as an argument. That should be fixed somehow, otherwise this\n> patch will never pass the test.\n\nHave you seen PARSE_OPT_STOP_AT_NON_OPTION?\n\nCiao,\nDscho\n"},{"id":"105127","messageId":"alpine.DEB.1.00.0902171257360.6185@intel-tinevez-2-302","threadId":"17837","inReplyTo":"20090217090015.5334.qmail@ea511e23bd6d9a.315fe32.mid.smarden.org","subject":"Re: [PATCH v2 1/8] config: Trivial rename in preparation for parseopt.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-17T11:58:42Z","receivedAt":"2009-02-17T11:58:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Feb 2009, Gerrit Pape wrote:\n\n> On Mon, Feb 16, 2009 at 05:45:00PM -0800, Junio C Hamano wrote:\n> > Felipe Contreras <felipe.contreras@gmail.com> writes:\n> > > When using the --list option general errors where not properly reported,\n> > > only errors related with the 'file'. Now they are reported, and 'file'\n> > > is irrelevant.\n> > > ...\n> > > @@ -299,10 +300,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\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\t\tfile && errno)\n> > > -\t\t\t\tdie(\"unable to read config file %s: %s\", file,\n> > > -\t\t\t\t    strerror(errno));\n> > > +\t\t\tif (git_config(show_all_config, NULL) < 0)\n> > > +\t\t\t\tdie(\"error processing config file(s)\");\n> > \n> > Does the author of 93a56c2 (git-config: print error message if the config\n> > file cannot be read, 2007-10-12) have any comment on this change (cc:ed)?\n> \n> Hm, we lose some information from the error message when called with\n> --file, but it seems to improve on other cases.  The filename doesn't\n> matter that much, but it would be nice to know the reason.  I wouldn't\n> object against this hunk though if that isn't possible.\n\nThe point is: when _not_ using --file, the output could be wrong \n(mentioning another config file than the one having an issue), or not be \nshown at all -- I haven't checked, but both options to not look good to \nme.\n\nCiao,\nDscho\n"},{"id":"105129","messageId":"alpine.DEB.1.00.0902171300450.6185@intel-tinevez-2-302","threadId":"17837","inReplyTo":"94a0d4530902161842s1d1d9fech3786cce1f1a1135d@mail.gmail.com","subject":"Re: [PATCH v2 1/8] config: Trivial rename in preparation for parseopt.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-17T12:01:47Z","receivedAt":"2009-02-17T12:01:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Feb 2009, Felipe Contreras wrote:\n\n>         free(repo_config);\n> +       if (found == 0)\n> +               error(\"no config file found\");\n>         return ret;\n\nErr, you mean \"return error(..)\"?\n\nBut it might be not an error at all: think of \"cd / && git ls-remote $URL\" \nwithout /etc/gitconfig nor $HOME/.gitconfig.\n\nCiao,\nDscho\n"},{"id":"105133","messageId":"94a0d4530902170521w71127810y188d27faaf1f2b83@mail.gmail.com","threadId":"17837","inReplyTo":"alpine.DEB.1.00.0902171255220.6185@intel-tinevez-2-302","subject":"Re: [PATCH v2 3/8] config: Use parseopt.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T13:21:01Z","receivedAt":"2009-02-17T13:21:01Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Feb 17, 2009 at 1:55 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Tue, 17 Feb 2009, Felipe Contreras wrote:\n>\n>> On Tue, Feb 17, 2009 at 7:44 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> > I've queued the entire series on top of fc/config-editor topic and even\n>> > merged the result in 'pu' once, but I had to reintegrate 'pu' without the\n>> > series.\n>> >\n>> > Before this commit, t/t1300-repo-config.sh passes, but this one breaks\n>> > the test.\n>>\n>> Ah, I didn't know there was a test for that.\n>>\n>> I've fixed most the issues but unfortunately parseopt barfs when -1 is\n>> used as an argument. That should be fixed somehow, otherwise this\n>> patch will never pass the test.\n>\n> Have you seen PARSE_OPT_STOP_AT_NON_OPTION?\n\nThanks, that works... cooking up a new series.\n\n-- \nFelipe Contreras\n"},{"id":"105146","messageId":"94a0d4530902170555l4e3f769wa24513b0ffbdd6a0@mail.gmail.com","threadId":"17837","inReplyTo":"7vbpt1by1y.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 3/8] config: Use parseopt.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T13:55:51Z","receivedAt":"2009-02-17T13:55:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Feb 17, 2009 at 4:24 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> +     if (HAS_MULTI_BITS(actions)) {\n>> +             error(\"only one action at a time.\");\n>> +             usage_with_options(builtin_config_usage, builtin_config_options);\n>\n> My initial reaction was:\n>\n>        Can we easily say \"--get and --getall are mutually incompatible\"?\n>\n> and knowing that it would take much more code, the second reaction was:\n>\n>        Does the user know what we mean by \"action\"?\n>\n> Since the answer to this is \"Yes, the usage message comes from parseopt\n> and there is a clear categorization\", I think the message is good enough.\n>\n> What happens when the user says \"config --get --get-colorbool user.name\"?\n> Is it an error?  Is it diagnosed as an error?\n>\n> It probably is easy to fix it by defining two bits of fake actions and do:\n>\n>        if (get_color_slot)\n>                actions |= ACTION_GET_COLOR;\n>        if (get_colorbool_slot)\n>                actions |= ACTION_GET_COLORBOOL;\n>\n> immediately before this HAS_MULTI_BITS check.\n>\n> I know I suggested these to are type-like, but I realize that these two\n> are better categorized as actions tied to a specific type (color), as you\n> had in your earlier round.\n\nAll right, done.\n\n>> +     if (actions == 0)\n>> +             switch (argc) {\n>> +             case 1: actions |= ACTION_GET; break;\n>> +             case 2: actions |= ACTION_ADD; break;\n>> +             case 3: actions |= ACTION_REPLACE_ALL; break;\n>\n> Straight assignment, not ORing-in please.  It wastes a few seconds from\n> the reader wondering what other bits in the variable \"actions\" are used\n> for things other than ACTION_* (the answer is none).\n>\n> Similarly, later conditions:\n>\n>> +     if (actions & ACTION_LIST) {\n>\n> would read better if they used equality == checks.\n\nCool, I was worried those where not logical to the reader but I\ncouldn't think of a better solution... this one looks much better!\n\nI've also made the same change for the 'types' variable in a later patch.\n\n-- \nFelipe Contreras\n"},{"id":"105147","messageId":"94a0d4530902170559r74d0b9acyea9be124ece0adeb@mail.gmail.com","threadId":"17837","inReplyTo":"alpine.DEB.1.00.0902171300450.6185@intel-tinevez-2-302","subject":"Re: [PATCH v2 1/8] config: Trivial rename in preparation for parseopt.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-17T13:59:28Z","receivedAt":"2009-02-17T13:59:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Feb 17, 2009 at 2:01 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Tue, 17 Feb 2009, Felipe Contreras wrote:\n>\n>>         free(repo_config);\n>> +       if (found == 0)\n>> +               error(\"no config file found\");\n>>         return ret;\n>\n> Err, you mean \"return error(..)\"?\n>\n> But it might be not an error at all: think of \"cd / && git ls-remote $URL\"\n> without /etc/gitconfig nor $HOME/.gitconfig.\n\nYeah, I thought about that when doing the patch. I've removed the\nerror(), now it will only return -1. Almost nobody is checking the\nreturn code anyways.\n\n-- \nFelipe Contreras\n"}]}