{"thread":{"id":"2551","subject":"[PATCH] Fix git_config_set() for mean cases","startedAt":"2005-11-16T09:47:37Z","lastAt":"2005-11-17T21:10:15Z","messageCount":7,"participants":["Johannes Schindelin","Junio C Hamano","H. Peter Anvin","Matthias Urlichs","Randal L. Schwartz"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"12008","messageId":"Pine.LNX.4.63.0511161045310.16596@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2551","inReplyTo":null,"subject":"[PATCH] Fix git_config_set() for mean cases","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-11-16T09:47:37Z","receivedAt":"2005-11-16T09:47:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThere were problems when the keys´ case didn´t match, and also when the\nsection was in the same line as the key.\n\nThis patch also adds a test case, so you see that it works now.\n\nSigned-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n\n---\n\n\tI shoulda started by writing those tests. This patch is on top\n\tof my earlier patch, which worked only in the nice cases. It \n\tnow needs git-config-set, though.\n\n config.c              |   42 ++++++++++++++----\n t/t1300-config-set.sh |  112 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 144 insertions(+), 10 deletions(-)\n create mode 100644 t/t1300-config-set.sh\n\napplies-to: ae3c1e28a1df80086033825ac68fdf4c6c835eee\ne326b690c85a648ffcbbc63e69333a7100a07892\ndiff --git a/config.c b/config.c\nindex 3b883b2..9315351 100644\n--- a/config.c\n+++ b/config.c\n@@ -255,7 +255,7 @@ int git_config(config_fn_t fn)\n \n static struct {\n \tint baselen;\n-\tconst char* key;\n+\tchar* key;\n \tconst char* value;\n \toff_t offset;\n \tenum { START, SECTION_SEEN, SECTION_END_SEEN, KEY_SEEN } state;\n@@ -270,10 +270,10 @@ static int store_aux(const char* key, co\n \t\t\tbreak;\n \t\tcase SECTION_SEEN:\n \t\t\tif (strncmp(key, store.key, store.baselen+1)) {\n-\t\t\t\tstore.offset = ftell(config_file);\n \t\t\t\tstore.state = SECTION_END_SEEN;\n \t\t\t\tbreak;\n-\t\t\t}\n+\t\t\t} else\n+\t\t\t\tstore.offset = ftell(config_file);\n \t\tcase SECTION_END_SEEN:\n \t\tcase START:\n \t\t\tif (!strcmp(key, store.key)) {\n@@ -326,18 +326,21 @@ int git_config_set(const char* key, cons\n \t\treturn 2;\n \t}\n \n+\tstore.key = strdup(key);\n \tfor (i = 0; key[i]; i++)\n \t\tif (i != store.baselen && !isalpha(key[i])) {\n \t\t\tfprintf(stderr, \"invalid key: %s\\n\", key);\n+\t\t\tfree(store.key);\n \t\t\treturn 1;\n-\t\t}\n+\t\t} else\n+\t\t\tstore.key[i] = tolower(key[i]);\n \n-\tstore.key = key;\n \tstore.value = value;\n \n \tfd = open(lock_file, O_WRONLY | O_CREAT | O_EXCL, 0666);\n \tif (fd < 0) {\n \t\tfprintf(stderr, \"could not lock config file\\n\");\n+\t\tfree(store.key);\n \t\treturn -1;\n \t}\n \n@@ -349,6 +352,9 @@ int git_config_set(const char* key, cons\n \t\t\t\"#\\n\"\n \t\t\t\"\\n\";\n \n+\t\tfree(store.key);\n+\t\tstore.key = (char*)key;\n+\n \t\twrite(fd, contents, sizeof(contents)-1);\n \t\tstore_write_section(fd);\n \t\tstore_write_pair(fd);\n@@ -362,25 +368,41 @@ int git_config_set(const char* key, cons\n \n \t\tif (git_config(store_aux)) {\n \t\t\tfprintf(stderr, \"invalid config file\\n\");\n+\t\t\tfree(store.key);\n \t\t\treturn 3;\n \t\t}\n \n+\t\tfree(store.key);\n+\t\tstore.key = (char*)key;\n+\n \t\tin_fd = open(config_file, O_RDONLY, 0666);\n \t\tcontents = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, in_fd, 0);\n \t\tclose(in_fd);\n \n \t\tif (store.offset == 0)\n-\t\t\toffset = st.st_size;\n-\t\telse {\n+\t\t\tstore.offset = offset = st.st_size;\n+\t\telse if (store.state != KEY_SEEN) {\n+\t\t\toffset = store.offset;\n+\t\t} else {\n+\t\t\tint equal_offset = st.st_size,\n+\t\t\t\tbracket_offset = st.st_size;\n \t\t\tfor (offset = store.offset-2; offset > 0 \n-\t\t\t\t\t&& contents[offset] != '\\n'; offset--);\n-\t\t\toffset++;\n+\t\t\t\t\t&& contents[offset] != '\\n'; offset--)\n+\t\t\t\tswitch (contents[offset]) {\n+\t\t\t\t\tcase '=': equal_offset = offset; break;\n+\t\t\t\t\tcase ']': bracket_offset = offset; break;\n+\t\t\t\t}\n+\t\t\tif (bracket_offset < equal_offset) {\n+\t\t\t\tcontents[bracket_offset+1] = '\\n';\n+\t\t\t\toffset = bracket_offset+2;\n+\t\t\t} else\n+\t\t\t\toffset++;\n \t\t}\n \t\twrite(fd, contents, offset);\n \t\tif (store.state == START)\n \t\t\tstore_write_section(fd);\n \t\tstore_write_pair(fd);\n-\t\tif (store.offset > offset)\n+\t\tif (store.offset < st.st_size)\n \t\t\twrite(fd, contents + store.offset,\n \t\t\t\tst.st_size - store.offset);\n \t\t\ndiff --git a/t/t1300-config-set.sh b/t/t1300-config-set.sh\nnew file mode 100644\nindex 0000000..48d4ff1\n--- /dev/null\n+++ b/t/t1300-config-set.sh\n@@ -0,0 +1,112 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2005 Johannes Schindelin\n+#\n+\n+test_description='Test git-config-set in different settings'\n+\n+. ./test-lib.sh\n+\n+test -f .git/config && rm .git/config\n+\n+git-config-set core.penguin \"little blue\"\n+\n+cat > expect.1 << EOF\n+#\n+# This is the config file\n+#\n+\n+[core]\n+\tpenguin = little blue\n+EOF\n+\n+test_expect_success 'initial' 'cmp .git/config expect.1'\n+\n+git-config-set Core.Movie BadPhysics\n+\n+cat > expect.2 << EOF\n+#\n+# This is the config file\n+#\n+\n+[core]\n+\tpenguin = little blue\n+\tMovie = BadPhysics\n+EOF\n+\n+test_expect_success 'mixed case' 'cmp .git/config expect.2'\n+\n+git-config-set Cores.WhatEver Second\n+\n+cat > expect.3 << EOF\n+#\n+# This is the config file\n+#\n+\n+[core]\n+\tpenguin = little blue\n+\tMovie = BadPhysics\n+[Cores]\n+\tWhatEver = Second\n+EOF\n+\n+test_expect_success 'similar section' 'cmp .git/config expect.3'\n+\n+git-config-set CORE.UPPERCASE true\n+\n+cat > expect.4 << EOF\n+#\n+# This is the config file\n+#\n+\n+[core]\n+\tpenguin = little blue\n+\tMovie = BadPhysics\n+\tUPPERCASE = true\n+[Cores]\n+\tWhatEver = Second\n+EOF\n+\n+test_expect_success 'similar section' 'cmp .git/config expect.4'\n+\n+cat > .git/config << EOF\n+[beta] ; silly comment # another comment\n+noIndent= sillyValue ; ´nother silly comment\n+\n+# empty line\n+\t\t; comment\n+\t\thaha   =\"beta\" # last silly comment\n+[nextSection] noNewline = ouch\n+EOF\n+\n+git-config-set beta.haha alpha\n+\n+cat > expect.5 << EOF\n+[beta] ; silly comment # another comment\n+noIndent= sillyValue ; ´nother silly comment\n+\n+# empty line\n+\t\t; comment\n+\thaha = alpha\n+[nextSection] noNewline = ouch\n+EOF\n+\n+test_expect_success 'really mean test' 'cmp .git/config expect.5'\n+\n+git-config-set nextsection.nonewline wow\n+\n+cat > expect.6 << EOF\n+[beta] ; silly comment # another comment\n+noIndent= sillyValue ; ´nother silly comment\n+\n+# empty line\n+\t\t; comment\n+\thaha = alpha\n+[nextSection]\n+\tnonewline = wow\n+EOF\n+\n+test_expect_success 'really really mean test' 'cmp .git/config expect.6'\n+\n+test_done\n+\n---\n0.99.9.GIT"},{"id":"12041","messageId":"7vhdac738c.fsf@assigned-by-dhcp.cox.net","threadId":"2551","inReplyTo":"Pine.LNX.4.63.0511161045310.16596@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Fix git_config_set() for mean cases","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-16T20:21:55Z","receivedAt":"2005-11-16T20:21:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> There were problems when the keys´ case didn´t match, and also when the\n> section was in the same line as the key.\n>\n> This patch also adds a test case, so you see that it works now.\n>\n> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n\nWhy do you spell apostrophe with 0xb4 not ASCII 0x27?  Not that\nit matters because I'll apply it with -u flag to git-am to\nconvert it to UTF-8 in the log message, but I am just curious.\n"},{"id":"12046","messageId":"437B9BF4.6020803@zytor.com","threadId":"2551","inReplyTo":"7vhdac738c.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix git_config_set() for mean cases","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-11-16T20:52:04Z","receivedAt":"2005-11-16T20:52:04Z","isPatch":true,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Junio C Hamano wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> \n>>There were problems when the keys´ case didn´t match, and also when the\n>>section was in the same line as the key.\n>>\n>>This patch also adds a test case, so you see that it works now.\n>>\n>>Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> \n> \n> Why do you spell apostrophe with 0xb4 not ASCII 0x27?  Not that\n> it matters because I'll apply it with -u flag to git-am to\n> convert it to UTF-8 in the log message, but I am just curious.\n> \n\nNot to mention the fact that it's just plain wrong -- U+00B4 is accute \naccent, not apostrophe.\n\n\t-hpa\n"},{"id":"12057","messageId":"Pine.LNX.4.63.0511170013470.9284@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2551","inReplyTo":"7vhdac738c.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix git_config_set() for mean cases","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-11-16T23:19:21Z","receivedAt":"2005-11-16T23:19:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 16 Nov 2005, Junio C Hamano wrote:\n\n> Why do you spell apostrophe with 0xb4 not ASCII 0x27?  Not that\n> it matters because I'll apply it with -u flag to git-am to\n> convert it to UTF-8 in the log message, but I am just curious.\n\nEfficiency. I often do the patches on my iBook w/ German keyboard. The \napostrophe 0xb4 (upward accent) is just beside my back space key, i.e. in \na favorite location for my right little finger.\n\nOTOH, 0x27 (the real apostrophe) is just beside the Enter key, which is \nthe only key Apple made too small. Way too small. So I try to avoid that \nkey, since I hit it by accident already too often. Furthermore, on the \nGerman keyboard you have to hold Shift down for the apostrophe, while you \ndo not for 0xb4.\n\nBesides, the character is there, so why not use it? ;-)\n\nCiao,\nDscho\n"},{"id":"12061","messageId":"pan.2005.11.16.23.26.26.638493@smurf.noris.de","threadId":"2551","inReplyTo":"7vhdac738c.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix git_config_set() for mean cases","fromName":"Matthias Urlichs","fromEmail":"smurf@smurf.noris.de","sentAt":"2005-11-16T23:26:28Z","receivedAt":"2005-11-16T23:26:28Z","isPatch":true,"sender":{"key":"matthias@urlichs.de","avatar":"https://gravatar.com/avatar/2708905af227313eba6f2b2ae0f7d0259b5ac5d71baef58fe5a13c699ce0bbf0?d=mp&s=160"},"body":"Hi, Junio C Hamano wrote:\n\n> Why do you spell apostrophe with 0xb4 not ASCII 0x27?\n\nBecause on German keyboards these two characters are right next to each\nother, I'd suspect...\n\n-- \nMatthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de\nDisclaimer: The quote was selected randomly. Really. | http://smurf.noris.de\n - -\n\"A horrible little boy came up to me and said, `You know in your book\nThe Martian Chronicles?'  I said, `Yes?'  He said, `You know where you\ntalk about Deimos rising in the East?'  I said, `Yes?'  He said `No.'\n-- So I hit him.\"\n\t\t-- attributed to Ray Bradbury\n"},{"id":"12072","messageId":"86br0kjbln.fsf@blue.stonehenge.com","threadId":"2551","inReplyTo":"Pine.LNX.4.63.0511170013470.9284@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Fix git_config_set() for mean cases","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2005-11-17T01:40:20Z","receivedAt":"2005-11-17T01:40:20Z","isPatch":true,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":">>>>> \"Johannes\" == Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\nJohannes> Besides, the character is there, so why not use it? ;-)\n\nIt reminds me of when people typed \"l\" instead of \"1\" because typewriters\ndidn't have a \"1\".\n\nIn that manner, it's just plain wrong.\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>\nPerl/Unix/security consulting, Technical writing, Comedy, etc. etc.\nSee PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!\n"},{"id":"12142","messageId":"Pine.LNX.4.63.0511172200490.18177@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2551","inReplyTo":"7v1x1ful7j.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix git_config_set() for mean cases","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-11-17T21:10:15Z","receivedAt":"2005-11-17T21:10:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\n[taking the discussion back to public]\n\nOn Thu, 17 Nov 2005, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > Also, I felt that a function like git_config_set() was lacking for too \n> > long from git. Maybe git-init-db does not need that flag, but I guess that \n> > we'd want to write config variables comfortably from C at some stage.\n> \n> Yes.  But I hesitate to throw in an interface that dismisses the\n> multivalued key problem without thinking things through first.\n\nOkay, I gave it some thought. See below for my points.\n\n> BTW, I think this discussion should be thrown back to the\n> public.  It went private by accident.\n\nHereby done.\n\n> Mind rehashing the discussion so far in the open?\n> \n> The main points I can think of are:\n> \n>  - configuration file format and semantics cleanups (methinks\n>    it is premature to talk about implementation and interface\n>    before discussing this).\n> \n>    - allowing sectionless variables seems to be a bug.  disallow\n>      it.\n\n Agree.\n\n>    - section names are alnum (i.e. they can start with a digit)\n>      while variable names are alpha+alnum.  is this what we\n>      want?  otherwise how should the rule be changed?\n\nIt seems natural to make variables start with alpha, since people are so \nused to it. It would not hurt the parsing to make them possibly start with \na digit, but I think it's okay to let it be as it is.\n\n>    - multivalued variables.  it is nice to have such, and is\n>      easier to read by humans (methinks).  it is not how other\n>      systems do .ini format, and the tools can live with single\n>      value and parse it into list of values (methinks that is\n>      what you are saying).  which way are we going?  does the\n>      order matter in such list valued variable in either case?\n\nTo recap:\n\n        [proxy]\n                command=\"ssh\" for \"ssh://kernel.org/\"\n                command=\"proxy-command\" for kernel.org\n                command=\"myprotocol-command\" for \"my://\"\n\nIt should be possible to set only the command for kernel.org while leaving \nthe others. I actually implemented a function\n\n\tgit_config_set_multivar(const char* name, const char* value,\n\t\tconst char* value_regex);\n\nwhich you can call like this:\n\n\tgit_config_set_multivar(\n\t\t\"proxy.command\",\n\t\t\"\\\"rsh\\\" for kernel.org\",\n\t\t\"for kernel.org$\");\n\nto change just the second command. You can use that function also to \nunset (i.e. remove) an entry:\n\n\tgit_config_set_multivar(\n\t\t\"proxy.command\",\n\t\tNULL,\n\t\t\"for \\\"ssh://kernel.org\\\"$\");\n\n>  - having a way for scripts to query and update the config file\n>    programatically is a good thing, both from C level and script\n>    level (i do not think there is a disagreement between us on\n>    this point).\n\nThere is no disagreement.\n\n>    - _if_ we do multivalued variables, what is the interface to\n>      append, replace, and remove one of the values?  (we may be\n>      able to get away by only supporting \"replace the whole\n>      thing\", in which case the program can do git_config_get()\n>      to grab the list, manipulate the list and feed the updated\n>      list to git_config_set()).\n\nSee above. For clarities sake, I left \"git_config_set(name,value)\" as \nshortcut to \"git_config_set_multivar(name,value,NULL)\", i.e. calling the \nmultivar version with value_regex==NULL actually reverts to singlevar \nversion.\n\nI'll send out the patches in a few minutes (have to get them into shape \nfirst).\n\nCiao,\nDscho\n"}]}