{"thread":{"id":"8713","subject":"[PATCH] config: add support for --bool and --int while setting values","startedAt":"2007-06-25T14:00:24Z","lastAt":"2007-06-27T02:18:27Z","messageCount":5,"participants":["Frank Lichtenheld","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"45766","messageId":"1182780024442-git-send-email-frank@lichtenheld.de","threadId":"8713","inReplyTo":null,"subject":"[PATCH] config: add support for --bool and --int while setting values","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-06-25T14:00:24Z","receivedAt":"2007-06-25T14:00:24Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"\nSigned-off-by: Frank Lichtenheld <frank@lichtenheld.de>\n---\n Documentation/git-config.txt    |    9 +++---\n builtin-config.c                |   54 +++++++++++++++++++++++++++++---------\n t/t1300-repo-config.sh          |   48 +++++++++++++++++++++++++++++++++-\n t/t9400-git-cvsserver-server.sh |    4 +-\n 4 files changed, 94 insertions(+), 21 deletions(-)\n\n There are probably more elegant ways to do this...\n But it seems to work.\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex f2c6717..ddb1dea 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -9,9 +9,9 @@ git-config - Get and set repository or global options\n SYNOPSIS\n --------\n [verse]\n-'git-config' [--system | --global] name [value [value_regex]]\n-'git-config' [--system | --global] --add name value\n-'git-config' [--system | --global] --replace-all name [value [value_regex]]\n+'git-config' [--system | --global] [type] name [value [value_regex]]\n+'git-config' [--system | --global] [type] --add name value\n+'git-config' [--system | --global] [type] --replace-all name [value [value_regex]]\n 'git-config' [--system | --global] [type] --get name [value_regex]\n 'git-config' [--system | --global] [type] --get-all name [value_regex]\n 'git-config' [--system | --global] --unset name [value_regex]\n@@ -36,8 +36,7 @@ prepend a single exclamation mark in front (see also <<EXAMPLES>>).\n The type specifier can be either '--int' or '--bool', which will make\n 'git-config' ensure that the variable(s) are of the given type and\n convert the value to the canonical form (simple decimal number for int,\n-a \"true\" or \"false\" string for bool).  Type specifiers currently only\n-take effect for reading operations.  If no type specifier is passed,\n+a \"true\" or \"false\" string for bool).  If no type specifier is passed,\n no checks or transformations are performed on the value.\n \n This command will fail if:\ndiff --git a/builtin-config.c b/builtin-config.c\nindex b2515f7..3aa4645 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -131,9 +131,32 @@ free_strings:\n \treturn ret;\n }\n \n+char* normalize_value(const char* key, const char* value)\n+{\n+\tchar* normalized;\n+\n+\tif (!value)\n+\t\treturn NULL;\n+\n+\tif (type == T_RAW)\n+\t\tnormalized = xstrdup(value);\n+\telse {\n+\t\tnormalized = xmalloc(64);\n+\t\tif (type == T_INT)\n+\t\t\tsprintf(normalized, \"%d\",\n+\t\t\t\tgit_config_int(key, value?value:\"\"));\n+\t\telse if (type == T_BOOL)\n+\t\t\tsprintf(normalized, \"%s\",\n+\t\t\t\tgit_config_bool(key, value) ? \"true\" : \"false\");\n+\t}\n+\n+\treturn normalized;\n+}\n+\n int cmd_config(int argc, const char **argv, const char *prefix)\n {\n \tint nongit = 0;\n+\tchar* value;\n \tsetup_git_directory_gently(&nongit);\n \n \twhile (1 < argc) {\n@@ -205,9 +228,10 @@ int cmd_config(int argc, const char **argv, const char *prefix)\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-\n-\t\t\treturn git_config_set(argv[1], argv[2]);\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}\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@@ -223,17 +247,21 @@ int cmd_config(int argc, const char **argv, const char *prefix)\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\treturn git_config_set_multivar(argv[2], argv[3], \"^$\", 0);\n-\t\telse if (!strcmp(argv[1], \"--replace-all\"))\n-\n-\t\t\treturn git_config_set_multivar(argv[2], argv[3], NULL, 1);\n-\t\telse\n-\n-\t\t\treturn git_config_set_multivar(argv[1], argv[2], argv[3], 0);\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\treturn git_config_set_multivar(argv[2], argv[3], argv[4], 1);\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\tusage(git_config_set_usage);\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 7731fa7..4234d83 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -465,11 +465,57 @@ test_expect_success bool '\n         done &&\n \tcmp expect result'\n \n-test_expect_failure 'invalid bool' '\n+test_expect_failure 'invalid bool (--get)' '\n \n \tgit-config bool.nobool foobar &&\n \tgit-config --bool --get bool.nobool'\n \n+test_expect_failure 'invalid bool (set)' '\n+\n+\tgit-config --bool bool.nobool foobar'\n+\n+rm .git/config\n+\n+cat > expect <<\\EOF\n+[bool]\n+\ttrue1 = true\n+\ttrue2 = true\n+\ttrue3 = true\n+\ttrue4 = true\n+\tfalse1 = false\n+\tfalse2 = false\n+\tfalse3 = false\n+\tfalse4 = false\n+EOF\n+\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.true3 YeS &&\n+\tgit-config --bool bool.true4 true &&\n+\tgit-config --bool bool.false1 000 &&\n+\tgit-config --bool bool.false2 \"\" &&\n+\tgit-config --bool bool.false3 nO &&\n+\tgit-config --bool bool.false4 FALSE &&\n+\tcmp expect .git/config'\n+\n+rm .git/config\n+\n+cat > expect <<\\EOF\n+[int]\n+\tval1 = 1\n+\tval2 = -1\n+\tval3 = 5242880\n+EOF\n+\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.val3 5m &&\n+\tcmp expect .git/config'\n+\n rm .git/config\n \n git-config quote.leading \" test\"\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex 0331770..641303e 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -38,7 +38,7 @@ echo >empty &&\n   git commit -q -m \"First Commit\" &&\n   git clone -q --local --bare \"$WORKDIR/.git\" \"$SERVERDIR\" >/dev/null 2>&1 &&\n   GIT_DIR=\"$SERVERDIR\" git config --bool gitcvs.enabled true &&\n-  GIT_DIR=\"$SERVERDIR\" git config --bool gitcvs.logfile \"$SERVERDIR/gitcvs.log\" ||\n+  GIT_DIR=\"$SERVERDIR\" git config gitcvs.logfile \"$SERVERDIR/gitcvs.log\" ||\n   exit 1\n \n # note that cvs doesn't accept absolute pathnames\n@@ -255,7 +255,7 @@ rm -fr \"$SERVERDIR\"\n cd \"$WORKDIR\" &&\n git clone -q --local --bare \"$WORKDIR/.git\" \"$SERVERDIR\" >/dev/null 2>&1 &&\n GIT_DIR=\"$SERVERDIR\" git config --bool gitcvs.enabled true &&\n-GIT_DIR=\"$SERVERDIR\" git config --bool gitcvs.logfile \"$SERVERDIR/gitcvs.log\" ||\n+GIT_DIR=\"$SERVERDIR\" git config gitcvs.logfile \"$SERVERDIR/gitcvs.log\" ||\n exit 1\n \n test_expect_success 'cvs update (create new file)' \\\n-- \n1.5.2.1\n"},{"id":"45772","messageId":"467FCBEA.906B14@eudaptics.com","threadId":"8713","inReplyTo":"1182780024442-git-send-email-frank@lichtenheld.de","subject":"Re: [PATCH] config: add support for --bool and --int while setting values","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-06-25T14:06:34Z","receivedAt":"2007-06-25T14:06:34Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Frank Lichtenheld wrote:\n> \n> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>\n\nPlease excuse if I'm missing the big picture, but why do we need this\nchange?\n\n-- Hannes\n"},{"id":"45777","messageId":"20070625161401.GW19725@planck.djpig.de","threadId":"8713","inReplyTo":"467FCBEA.906B14@eudaptics.com","subject":"Re: [PATCH] config: add support for --bool and --int while setting values","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-06-25T16:14:01Z","receivedAt":"2007-06-25T16:14:01Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Mon, Jun 25, 2007 at 04:06:34PM +0200, Johannes Sixt wrote:\n> Frank Lichtenheld wrote:\n> > \n> > Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>\n> \n> Please excuse if I'm missing the big picture, but why do we need this\n> change?\n\n- Of course the user or script calling git-config can do the\n  normalization and error checking, if they want to. But I would\n  prefer to have it available in git-config.\n- I would prefer that these options wouldn't be silently ignored,\n  because that can be confusing (at least it is documented now, but\n  still). So we should either using them or error out. I prefer the former.\n\nSomething that I forgot to mention in the previous mail:\nOne real problem with the patch is that it expands the k,m,g suffixes\nfor integer values. It probably shouldn't do that.\n\nGruesse,\n-- \nFrank Lichtenheld <frank@lichtenheld.de>\nwww: http://www.djpig.de/\n"},{"id":"45879","messageId":"7vbqf2ta9f.fsf@assigned-by-dhcp.pobox.com","threadId":"8713","inReplyTo":"20070625161401.GW19725@planck.djpig.de","subject":"Re: [PATCH] config: add support for --bool and --int while setting values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-27T02:13:32Z","receivedAt":"2007-06-27T02:13:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frank Lichtenheld <frank@lichtenheld.de> writes:\n\n> On Mon, Jun 25, 2007 at 04:06:34PM +0200, Johannes Sixt wrote:\n>> Frank Lichtenheld wrote:\n>> > \n>> > Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>\n>> \n>> Please excuse if I'm missing the big picture, but why do we need this\n>> change?\n>\n> - Of course the user or script calling git-config can do the\n>   normalization and error checking, if they want to. But I would\n>   prefer to have it available in git-config.\n> - I would prefer that these options wouldn't be silently ignored,\n>   because that can be confusing (at least it is documented now, but\n>   still). So we should either using them or error out. I prefer the former.\n>\n> Something that I forgot to mention in the previous mail:\n> One real problem with the patch is that it expands the k,m,g suffixes\n> for integer values. It probably shouldn't do that.\n\nHow about doing something like this, then?\n\ngit_config_int() knows that a missing value is a nonsense and\nbarfs on such an input, so (value ? value : \"\") is redundant\nhere.  Besides, you check value == NULL much earlier in this\nfunction.\n\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 9973f94..33b60ec 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -148,10 +148,11 @@ char* normalize_value(const char* key, const char* value)\n \tif (type == T_RAW)\n \t\tnormalized = xstrdup(value);\n \telse {\n-\t\tnormalized = xmalloc(64);\n-\t\tif (type == T_INT)\n-\t\t\tsprintf(normalized, \"%d\",\n-\t\t\t\tgit_config_int(key, value?value:\"\"));\n+\t\tnormalized = xmalloc(64 + strlen(value));\n+\t\tif (type == T_INT) {\n+\t\t\tint v = git_config_int(key, value);\n+\t\t\tsprintf(normalized, \"%d # %s\", v, value);\n+\t\t}\n \t\telse if (type == T_BOOL)\n \t\t\tsprintf(normalized, \"%s\",\n \t\t\t\tgit_config_bool(key, value) ? \"true\" : \"false\");\n"},{"id":"45884","messageId":"7vk5tqrvgs.fsf@assigned-by-dhcp.pobox.com","threadId":"8713","inReplyTo":"7vbqf2ta9f.fsf@assigned-by-dhcp.pobox.com","subject":"Re: [PATCH] config: add support for --bool and --int while setting values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-27T02:18:27Z","receivedAt":"2007-06-27T02:18:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Frank Lichtenheld <frank@lichtenheld.de> writes:\n>\n>> Something that I forgot to mention in the previous mail:\n>> One real problem with the patch is that it expands the k,m,g suffixes\n>> for integer values. It probably shouldn't do that.\n>\n> How about doing something like this, then?\n\nNah, that wouldn't work, as normalize_value's return value is\nparsed and quoted.  Grumble....\n"}]}