Re: [PATCH v2 3/8] config: Use parseopt.

[Felipe Contreras <felipe.contreras@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

On Tue, Feb 17, 2009 at 7:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I've queued the entire series on top of fc/config-editor topic and even
> merged the result in 'pu' once, but I had to reintegrate 'pu' without the
> series.
>
> Before this commit, t/t1300-repo-config.sh passes, but this one breaks
> the test.

Ah, I didn't know there was a test for that.

I've fixed most the issues but unfortunately parseopt barfs when -1 is
used as an argument. That should be fixed somehow, otherwise this
patch will never pass the test.

I'm attaching a patch that makes the test pass (for review), but
shouldn't be merged.

-- 
Felipe Contreras

[-- Attachment #2: 0001-config-Fixes-parseopt-for-t1300-repo-config.patch --]
[-- Type: application/octet-stream, Size: 3982 bytes --]

From 22e4d1a472027d2b7f650a99f487fef78ef3b8ad Mon Sep 17 00:00:00 2001
From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Tue, 17 Feb 2009 12:30:26 +0200
Subject: [NOMERGE/PATCH] config: Fixes parseopt for t1300-repo-config.

This is patch isn't meant for merging, just for review.

Ideally parseopt shouldn't fail with -1 as an argument.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin-config.c       |   18 +++++++++++++++---
 t/t1300-repo-config.sh |    8 ++++----
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/builtin-config.c b/builtin-config.c
index 084222a..19f274a 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -38,6 +38,8 @@ static int end_null;
 #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 location"),
@@ -46,7 +48,7 @@ static struct option builtin_config_options[] = {
 	OPT_STRING('f', "file", &given_config_file, "FILE", "use given config file"),
 	OPT_GROUP("Action"),
 	OPT_BIT(0, "get", &actions, "get value: name [value-regex]", ACTION_GET),
-	OPT_BIT(0, "get-all", &actions, "get all values: key [value-regex]", ACTION_GET),
+	OPT_BIT(0, "get-all", &actions, "get all values: key [value-regex]", ACTION_GET_ALL),
 	OPT_BIT(0, "get-regexp", &actions, "get values for regexp: name-regex [value-regex]", ACTION_GET_REGEXP),
 	OPT_BIT(0, "replace-all", &actions, "replace all matching variables: name [value [value_regex]", ACTION_REPLACE_ALL),
 	OPT_BIT(0, "add", &actions, "adds a new variable: name value", ACTION_ADD),
@@ -343,8 +345,8 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
 	if (actions == 0)
 		switch (argc) {
 		case 1: actions |= ACTION_GET; break;
-		case 2: actions |= ACTION_ADD; break;
-		case 3: actions |= ACTION_REPLACE_ALL; break;
+		case 2: actions |= ACTION_SET; break;
+		case 3: actions |= ACTION_SET_ALL; break;
 		default:
 			usage_with_options(builtin_config_usage, builtin_config_options);
 		}
@@ -362,6 +364,16 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
 		git_config(git_default_config, NULL);
 		launch_editor(config_filename, NULL, NULL);
 	}
+	else if (actions & ACTION_SET) {
+		check_argc(argc, 2, 2);
+		value = normalize_value(argv[0], argv[1]);
+		return git_config_set(argv[0], value);
+	}
+	else if (actions & ACTION_SET_ALL) {
+		check_argc(argc, 2, 3);
+		value = normalize_value(argv[0], argv[1]);
+		return git_config_set_multivar(argv[0], value, argv[2], 0);
+	}
 	else if (actions & ACTION_ADD) {
 		check_argc(argc, 2, 2);
 		value = normalize_value(argv[0], argv[1]);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 11b82f4..be7104d 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -528,7 +528,7 @@ EOF
 test_expect_success bool '
 
 	git config bool.true1 01 &&
-	git config bool.true2 -1 &&
+	git config bool.true2 -- -1 &&
 	git config bool.true3 YeS &&
 	git config bool.true4 true &&
 	git config bool.false1 000 &&
@@ -569,7 +569,7 @@ EOF
 test_expect_success 'set --bool' '
 
 	git config --bool bool.true1 01 &&
-	git config --bool bool.true2 -1 &&
+	git config --bool bool.true2 -- -1 &&
 	git config --bool bool.true3 YeS &&
 	git config --bool bool.true4 true &&
 	git config --bool bool.false1 000 &&
@@ -590,7 +590,7 @@ EOF
 test_expect_success 'set --int' '
 
 	git config --int int.val1 01 &&
-	git config --int int.val2 -1 &&
+	git config --int int.val2 -- -1 &&
 	git config --int int.val3 5m &&
 	cmp expect .git/config'
 
@@ -648,7 +648,7 @@ test_expect_success 'set --bool-or-int' '
 	git config --bool-or-int bool.false2 no &&
 	git config --bool-or-int int.int1 0 &&
 	git config --bool-or-int int.int2 1 &&
-	git config --bool-or-int int.int3 -1 &&
+	git config --bool-or-int int.int3 -- -1 &&
 	test_cmp expect .git/config
 '
 
-- 
1.6.1.3