login
Search
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-config.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index 6937eaf..afc4393 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -27,7 +27,7 @@ static int show_all_config(const char *key_, const char *value_, void *cb)
return 0;
}
-static int show_config(const char* key_, const char* value_, void *cb)
+static int show_config(const char *key_, const char *value_, void *cb)
{
char value[256];
const char *vptr = value;
@@ -74,7 +74,7 @@ static int show_config(const char* key_, const char* value_, void *cb)
return 0;
}
-static int get_value(const char* key_, const char* regex_)
+static int get_value(const char *key_, const char *regex_)
{
int ret = -1;
char *tl;
@@ -284,7 +284,7 @@ static int get_colorbool(int argc, const char **argv)
int cmd_config(int argc, const char **argv, const char *prefix)
{
int nongit;
- char* value;
+ char *value;
const char *file = setup_git_directory_gently(&nongit);
config_exclusive_filename = getenv(CONFIG_ENVIRONMENT);
--
1.6.1.3
--
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-config.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index afc4393..d52a057 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -363,15 +363,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)
} else if (!strcmp(argv[1], "--get-colorbool")) {
return get_colorbool(argc-2, argv+2);
} else if (!strcmp(argv[1], "--edit") || !strcmp(argv[1], "-e")) {
- const char *config_filename;
if (argc != 2)
usage(git_config_set_usage);
- if (config_exclusive_filename)
- config_filename = config_exclusive_filename;
- else
- config_filename = git_path("config");
git_config(git_default_config, NULL);
- launch_editor(config_filename, NULL, NULL);
+ launch_editor(config_exclusive_filename ?
+ config_exclusive_filename : git_path("config"),
+ NULL, NULL);
return 0;
} else
break;
--
1.6.1.3
--
Currently git_config() returns an error if there is no repo config file
available (cwd is not a git repo); it will correctly parse the system
and global config files, but still return an error.
It doesn't affect anything else since almost nobody is checking for the
return code (with the exception of 'git remote update').
A reorganization in 'git config' would benefit from being able to
properly detect errors in git_config without the noise generated when
cwd is not a git repo.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
config.c | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/config.c b/config.c
index 7dc1b0f..68ce519 100644
--- a/config.c
+++ b/config.c
@@ -649,28 +649,37 @@ int git_config_global(void)
int git_config(config_fn_t fn, void *data)
{
- int ret = 0;
+ int ret = 0, found = 0;
char *repo_config = NULL;
const char *home = NULL;
/* Setting $GIT_CONFIG makes git read _only_ the given config file. */
if (config_exclusive_filename)
return git_config_from_file(fn, config_exclusive_filename, data);
- if (git_config_system() && !access(git_etc_gitconfig(), R_OK))
+ if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
ret += git_config_from_file(fn, git_etc_gitconfig(),
data);
+ found += 1;
+ }
home = getenv("HOME");
if (git_config_global() && home) {
char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
- if (!access(user_config, R_OK))
+ if (!access(user_config, R_OK)) {
ret += git_config_from_file(fn, user_config, data);
+ found += 1;
+ }
free(user_config);
}
repo_config = git_pathdup("config");
- ret += git_config_from_file(fn, repo_config, data);
+ if (!access(repo_config, R_OK)) {
+ ret += git_config_from_file(fn, repo_config, data);
+ found += 1;
+ }
free(repo_config);
+ if (found == 0)
+ return -1;
return ret;
}
--
1.6.1.3
--
Essensially this replaces 'file' with 'prefix' in the cases where the
variable is used as a prefix, which is consistent with other git
commands.
When using the --list option general errors where not properly reported,
only errors related with the 'file'. Now they are reported, and 'file'
is irrelevant.
That reduces the rest of 'file' usage to nothing, therefore now only
'prefix' remains.
Suggested by Johannes Schindelin.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-config.c | 32 ++++++++++++++++++--------------
1 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index d52a057..5074c61 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -178,6 +178,7 @@ static char *normalize_value(const char *key, const char *value)
static int get_color_found;
static const char *get_color_slot;
+static const char *get_colorbool_slot;
static char parsed_color[COLOR_MAXLEN];
static int git_get_color_config(const char *var, const char *value, void *cb)
@@ -231,7 +232,7 @@ static int get_diff_color_found;
static int git_get_colorbool_config(const char *var, const char *value,
void *cb)
{
- if (!strcmp(var, get_color_slot)) {
+ if (!strcmp(var, get_colorbool_slot)) {
get_colorbool_found =
git_config_colorbool(var, value, stdout_is_tty);
}
@@ -263,11 +264,11 @@ static int get_colorbool(int argc, const char **argv)
usage(git_config_set_usage);
get_colorbool_found = -1;
get_diff_color_found = -1;
- get_color_slot = argv[0];
+ get_colorbool_slot = argv[0];
git_config(git_get_colorbool_config, NULL);
if (get_colorbool_found < 0) {
- if (!strcmp(get_color_slot, "color.diff"))
+ if (!strcmp(get_colorbool_slot, "color.diff"))
get_colorbool_found = get_diff_color_found;
if (get_colorbool_found < 0)
get_colorbool_found = git_use_color_default;
@@ -281,11 +282,11 @@ static int get_colorbool(int argc, const char **argv)
}
}
-int cmd_config(int ...In preparation for parseopt.
Also remove some unecessary comments since the usage is described in the
documentation.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-config.c | 63 +++++++++++++++--------------------------------------
1 files changed, 18 insertions(+), 45 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index 5074c61..0836880 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -180,7 +180,6 @@ static int get_color_found;
static const char *get_color_slot;
static const char *get_colorbool_slot;
static char parsed_color[COLOR_MAXLEN];
-
static int git_get_color_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, get_color_slot)) {
@@ -192,29 +191,8 @@ static int git_get_color_config(const char *var, const char *value, void *cb)
return 0;
}
-static int get_color(int argc, const char **argv)
+static void get_color(const char *def_color)
{
- /*
- * grab the color setting for the given slot from the configuration,
- * or parse the default value if missing, and return ANSI color
- * escape sequence.
- *
- * e.g.
- * git config --get-color color.diff.whitespace "blue reverse"
- */
- const char *def_color = NULL;
-
- switch (argc) {
- default:
- usage(git_config_set_usage);
- case 2:
- def_color = argv[1];
- /* fallthru */
- case 1:
- get_color_slot = argv[0];
- break;
- }
-
get_color_found = 0;
parsed_color[0] = '\0';
git_config(git_get_color_config, NULL);
@@ -223,7 +201,6 @@ static int get_color(int argc, const char **argv)
color_parse(def_color, "command line", parsed_color);
fputs(parsed_color, stdout);
- return 0;
}
static int stdout_is_tty;
@@ -247,24 +224,10 @@ static int git_get_colorbool_config(const char *var, const char *value,
return 0;
}
-static int get_colorbool(int argc, const char **argv)
+static int get_colorbool(int print)
{
- /*
- * git config --get-colorbool <slot> [<stdout-is-tty>]
- *
- * ...Reorganizing the code to use parseopt as suggested by Johannes
Schindelin.
This patch has benefited from comments by Johannes
Schindelin and Junio C Hamano.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-config.c | 342 ++++++++++++++++++++++++++++++------------------------
1 files changed, 188 insertions(+), 154 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index 0836880..45b4a9f 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -1,9 +1,12 @@
#include "builtin.h"
#include "cache.h"
#include "color.h"
+#include "parse-options.h"
-static const char git_config_set_usage[] =
-"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 ]";
+static const char *const builtin_config_usage[] = {
+ "git config [options]",
+ NULL
+};
static char *key;
static regex_t *key_regexp;
@@ -18,6 +21,61 @@ static char key_delim = ' ';
static char term = '\n';
static enum { T_RAW, T_INT, T_BOOL, T_BOOL_OR_INT } type = T_RAW;
+static int use_global_config, use_system_config;
+static const char *given_config_file;
+static int actions;
+static const char *get_color_slot, *get_colorbool_slot;
+static int end_null;
+
+#define ACTION_GET (1<<0)
+#define ACTION_GET_ALL (1<<1)
+#define ACTION_GET_REGEXP (1<<2)
+#define ACTION_REPLACE_ALL (1<<3)
+#define ACTION_ADD (1<<4)
+#define ACTION_UNSET (1<<5)
+#define ACTION_UNSET_ALL (1<<6)
+#define ACTION_RENAME_SECTION (1<<7)
+#define ACTION_REMOVE_SECTION (1<<8)
+#define ACTION_LIST (1<<9)
+#define ACTION_EDIT (1<<10)
+#define ACTION_SET (1<<11)
+#define ACTION_SET_ALL (1<<12)
+
+static struct option builtin_config_options[] = {
+ OPT_GROUP("Config file ...From: Junio C Hamano <gitster@pobox.com>
So that --get and --get-colorbool are diagnosed as errors.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-config.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index 45b4a9f..8ee0cd9 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -40,6 +40,8 @@ static int end_null;
#define ACTION_EDIT (1<<10)
#define ACTION_SET (1<<11)
#define ACTION_SET_ALL (1<<12)
+#define ACTION_GET_COLOR (1<<13)
+#define ACTION_GET_COLORBOOL (1<<14)
static struct option builtin_config_options[] = {
OPT_GROUP("Config file location"),
@@ -339,6 +341,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
key_delim = '\n';
}
+ if (get_color_slot)
+ actions |= ACTION_GET_COLOR;
+ if (get_colorbool_slot)
+ actions |= ACTION_GET_COLORBOOL;
+
if (HAS_MULTI_BITS(actions)) {
error("only one action at a time.");
usage_with_options(builtin_config_usage, builtin_config_options);
@@ -432,10 +439,10 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
if (ret == 0)
die("No such section!");
}
- else if (get_color_slot) {
+ else if (actions == ACTION_GET_COLOR) {
get_color(argv[0]);
}
- else if (get_colorbool_slot) {
+ else if (actions == ACTION_GET_COLORBOOL) {
if (argc == 1)
stdout_is_tty = git_config_bool("command line", argv[0]);
else if (argc == 0)
--
1.6.1.3
--
Either --global, --system, or --file should be used, but not any
combination.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-config.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index 8ee0cd9..a674246 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -315,6 +315,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
argc = parse_options(argc, argv, builtin_config_options, builtin_config_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (use_global_config + use_system_config + !!given_config_file > 1) {
+ error("only one config file at a time.");
+ usage_with_options(builtin_config_usage, builtin_config_options);
+ }
+
if (use_global_config) {
char *home = getenv("HOME");
if (home) {
--
1.6.1.3
--
So now only either --bool, --int, or --bool-or-int can be used, but not
any combination of them.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-config.c | 32 ++++++++++++++++++++------------
1 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index a674246..060191c 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -19,11 +19,10 @@ static int seen;
static char delim = '=';
static char key_delim = ' ';
static char term = '\n';
-static enum { T_RAW, T_INT, T_BOOL, T_BOOL_OR_INT } type = T_RAW;
static int use_global_config, use_system_config;
static const char *given_config_file;
-static int actions;
+static int actions, types;
static const char *get_color_slot, *get_colorbool_slot;
static int end_null;
@@ -43,6 +42,10 @@ static int end_null;
#define ACTION_GET_COLOR (1<<13)
#define ACTION_GET_COLORBOOL (1<<14)
+#define TYPE_BOOL (1<<0)
+#define TYPE_INT (1<<1)
+#define TYPE_BOOL_OR_INT (1<<2)
+
static struct option builtin_config_options[] = {
OPT_GROUP("Config file location"),
OPT_BOOLEAN(0, "global", &use_global_config, "use global config file"),
@@ -63,9 +66,9 @@ static struct option builtin_config_options[] = {
OPT_STRING(0, "get-color", &get_color_slot, "slot", "find the color configured: [default]"),
OPT_STRING(0, "get-colorbool", &get_colorbool_slot, "slot", "find the color setting: [stdout-is-tty]"),
OPT_GROUP("Type"),
- OPT_SET_INT(0, "bool", &type, "value is \"true\" or \"false\"", T_BOOL),
- OPT_SET_INT(0, "int", &type, "value is decimal number", T_INT),
- OPT_SET_INT(0, "bool-or-int", &type, NULL, T_BOOL_OR_INT),
+ OPT_BIT(0, "bool", &types, "value is \"true\" or \"false\"", TYPE_BOOL),
+ OPT_BIT(0, "int", &types, "value is decimal number", TYPE_INT),
+ OPT_BIT(0, "bool-or-int", &types, NULL, TYPE_BOOL_OR_INT),
OPT_GROUP("Other"),
OPT_BOOLEAN('z', "null", &end_null, "terminate values with NUL byte"),
OPT_END(),
@@ -109,11 +112,11 @@ ...As suggested by Johannes Schindelin.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-config.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index 060191c..ec7b613 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -373,6 +373,7 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
}
if (actions == ACTION_LIST) {
+ check_argc(argc, 0, 0);
if (git_config(show_all_config, NULL) < 0) {
if (config_exclusive_filename)
die("unable to read config file %s: %s",
@@ -382,6 +383,7 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
}
}
else if (actions == ACTION_EDIT) {
+ check_argc(argc, 0, 0);
git_config(git_default_config, NULL);
launch_editor(config_exclusive_filename ?
config_exclusive_filename : git_path("config"),
--
1.6.1.3
--
Currently it does???
Here is what I get from using the command from master and next:
$ cd /var/tmp
$ ls -d /.git /var/.git /var/tmp/.git /var/tmp/config
ls: /.git: No such file or directory
ls: /var/.git: No such file or directory
ls: /var/tmp/.git: No such file or directory
ls: /var/tmp/config: No such file or directory
$ git config -l >/dev/null ; echo $?
0
$ git config alias.co; echo $?
checkout
0
I have $HOME/.gitconfig (where the alias comes from) and no system wide
configuration file.
Also the patch is mistitled. Whatever you are trying to say about the
current problem which I do not seem to get, and whatever different
behaviour from the current one you are trying to implement (which is not
quite clear from the above log message), it is not about making it more
flexible.
The patch text suggests you are trying to change the function's exit
status so the title would be "git-config: report errors correctly in its
exit status", but it is unspecified in your commit log message what
definition of "correctly" you are using in this patch.
--
This patch doesn't alter the behavior of 'git config', it's for git_config() (the function). At this point cmd_config() will return an exit code of 0 even if there's an error on git_config() when no What I'm trying to do is "Make git_config() don't return an error when there's no repo config file if there are other config files available"... I thought there was no short way to say that =/ -- Felipe Contreras --
git_config(): not having a per-repo config file is not an error Please drop the full-stop at the end of the Subject: line, by the way. --
On Tue, Feb 17, 2009 at 14:52, Felipe Contreras Did I miss 0/10 somehow? If not, it would be nice to have one if you send a long patch series :). That way I can ignore v3 of the series if the cover letter tells me nothing significant (to me) has changed. 1 minutes to write for you, saves 5 minutes for all reviewers :). -- Cheers, Sverre Rabbelier --
Hmm, right. I was thinking on Junio and Johanness that already know the context, so they don't need an introduction, but I forgot about other people that might be interested in giving this a review. In any case, this patch series is a try to re-organize the code of 'git config' and the main change is to use parseopt for better maintainability and option parsing. A few functionality changes are provided, like the fact that now the arguments don't need to be in a specific order: (--global -l vs -l --global), and improvements in error reporting. Otherwise it should work exactly the same as before (except with a much better --help). Cheers. -- Felipe Contreras --
