{"thread":{"id":"2582","subject":"[PATCH] Add test case for git-config-set","startedAt":"2005-11-17T21:50:08Z","lastAt":"2005-11-18T10:57:14Z","messageCount":5,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"12151","messageId":"Pine.LNX.4.63.0511172249410.18285@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2582","inReplyTo":null,"subject":"[PATCH] Add test case for git-config-set","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-11-17T21:50:08Z","receivedAt":"2005-11-17T21:50:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\n... includes the mean tests I mentioned on the list.\n\nSigned-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n\n---\n\n t1300-config-set.sh |  180 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 180 insertions(+), 0 deletions(-)\n create mode 100644 t1300-config-set.sh\n\napplies-to: de5d2a42ebd69e3608e0f99f615fbcf55a88c101\n3fa048df0d691d3423ec81f8dba947e2c05fa996\ndiff --git a/t1300-config-set.sh b/t1300-config-set.sh\nnew file mode 100644\nindex 0000000..df89216\n--- /dev/null\n+++ b/t1300-config-set.sh\n@@ -0,0 +1,180 @@\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 << 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'\n+\n+git-config-set Core.Movie BadPhysics\n+\n+cat > expect << 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'\n+\n+git-config-set Cores.WhatEver Second\n+\n+cat > expect << 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'\n+\n+git-config-set CORE.UPPERCASE true\n+\n+cat > expect << 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'\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 << 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'\n+\n+git-config-set nextsection.nonewline wow\n+\n+cat > expect << 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'\n+\n+git-config-set beta.haha\n+\n+cat > expect << EOF\n+[beta] ; silly comment # another comment\n+noIndent= sillyValue ; 'nother silly comment\n+\n+# empty line\n+\t\t; comment\n+[nextSection]\n+\tnonewline = wow\n+EOF\n+\n+test_expect_success 'unset' 'cmp .git/config expect'\n+\n+git-config-set nextsection.NoNewLine \"wow2 for me\" \"for me$\"\n+\n+cat > expect << EOF\n+[beta] ; silly comment # another comment\n+noIndent= sillyValue ; 'nother silly comment\n+\n+# empty line\n+\t\t; comment\n+[nextSection]\n+\tnonewline = wow\n+\tNoNewLine = wow2 for me\n+EOF\n+\n+test_expect_success 'multivar' 'cmp .git/config expect'\n+\n+git-config-set nextsection.nonewline \"wow3\" \"wow$\"\n+\n+cat > expect << EOF\n+[beta] ; silly comment # another comment\n+noIndent= sillyValue ; 'nother silly comment\n+\n+# empty line\n+\t\t; comment\n+[nextSection]\n+\tnonewline = wow3\n+\tNoNewLine = wow2 for me\n+EOF\n+\n+test_expect_success 'multivar replace' 'cmp .git/config expect'\n+\n+test_expect_failure 'ambiguous unset' \\\n+\t'git-config-set --unset nextsection.nonewline'\n+\n+test_expect_failure 'invalid unset' \\\n+\t'git-config-set --unset somesection.nonewline'\n+\n+git-config-set --unset nextsection.nonewline \"wow3$\"\n+\n+cat > expect << EOF\n+[beta] ; silly comment # another comment\n+noIndent= sillyValue ; 'nother silly comment\n+\n+# empty line\n+\t\t; comment\n+[nextSection]\n+\tNoNewLine = wow2 for me\n+EOF\n+\n+test_expect_success 'multivar unset' 'cmp .git/config expect'\n+\n+test_expect_failure 'invalid key' 'git-config-set inval.2key blabla'\n+\n+test_expect_success 'correct key' 'git-config-set 123456.a123 987'\n+\n+test_done\n+\n---\n0.99.9.GIT\n"},{"id":"12193","messageId":"7vd5kypjv2.fsf@assigned-by-dhcp.cox.net","threadId":"2582","inReplyTo":"Pine.LNX.4.63.0511172249410.18285@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add test case for git-config-set","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-18T06:09:53Z","receivedAt":"2005-11-18T06:09:53Z","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> +test_expect_failure 'ambiguous unset' \\\n> +\t'git-config-set --unset nextsection.nonewline'\n\nI am not so sure about this case.  Shouldn't this remove both?\n\nFor example, if a Porcelain wants to force pull.twohead to be\nresolve and nothing else, and it wants to do it unconditionally,\nit would first want to empty whatever multivalue there are\ncurrently, and then insert its own, and I'd imagine the way to\nsay that would be like this:\n\n\tgit-config-set --unset pull.twohead '^'\n        git-config-set pull.twohead resolve\n\nMore simply (I do not think you have a test case for this):\n\n        git-config-set pull.twohead resolve '^'\n\nI think it is the easiest to explain and understand the\nsemantics of config_set_multivalue if it were to first remove all\nexisting key-value for matching ones, and then insert what was\nprovided by the user.\n\nExtending that multivalue example a bit more, I think it is a\nbit cumbersome for a Porcelain to set pull.twohead to recursive\nand then resolve, with your interface.  Even if you had the\nemptying behaviour I suggested above, you would have to say\nsomething awkard like this:\n\n\tgit-config-set --unset pull.twohead '^'\n        git-config-set pull.twohead recursive\n        git-config-set pull.twohead resolve no-such-value-should-be-there\n\nMaybe we could have the shell-level interface like this:\n\n\tgit-config-set [--remove rx] section.key [value...]\n\nWhen --remove rx is specified, the command first removes\nexisting multivalue for the given section.key that match rx, and\nthen insert given value(s); not giving any values amounts to\n--unset.  Not giving --remove rx is the same as giving a regexp\nthat matches all multivalues.  So the simplest:\n\n\tgit-config-set section.key value\n\nbecomes a single-value assignment (insert-or-replace),\n\n\tgit-config-set section.key\n\nconfusingly enough is --unset (we probably would want to require\nan explicit command line noise-word \"--unset\" in this case).\nAnd \"replacing with two values regardless of whatever there are\ncurrently\" naturally becomes:\n\n\tgit-config-set pull.twohead recursive resolve\n\nThe C-level interface would become something like:\n\n\tgit_config_set_multivar(const char *key,\n        \t\t\tconst char *remove_value_regex,\n                                const char **values)\n\nwhere values is a NULL terminated list of values.\n\nBTW, do we want to remove the section after removing the last\nkey and making it empty?\n"},{"id":"12194","messageId":"7vsltuo49q.fsf@assigned-by-dhcp.cox.net","threadId":"2582","inReplyTo":"7vd5kypjv2.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add test case for git-config-set","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-18T06:32:01Z","receivedAt":"2005-11-18T06:32:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> \tgit-config-set section.key\n>\n> confusingly enough is --unset (we probably would want to require\n> an explicit command line noise-word \"--unset\" in this case).\n\nThis is too confusing, so I think we should require --unset.  I\nthink the C-interface I suggested in the previous message can\nstay the same, but from the command line perspective, we should\nhave a bit easier syntax.\n\nA revised suggestion is:\n\n\t;# remove all\n\tgit-config-set --unset section.key\n\n\t;# remove values that match rx and then append zero or more values\n\tgit-config-set --remove rx section.key [value...]\n\n\t;# append one or more values (equivalent to specifying --remove\n        ;# with rx that never matches anything).  To reduce\n        ;# confusion, we always require at least one value here.\n\tgit-config-set section.key value [value...]\n\n\t;# Additionally, purely as a syntax sugar, replace the\n\t;# entire multivalue with one or more values\n\t;# (equivalent to saying --remove '^').\n\tgit-config-set --replace section.key value [value...]\n\n\nI think (aside from \"*-set\" now becomes confusing), showing the\nvalue of the specified key to stdout with\n\n\tgit-config-set section.key\n\nwould be a nice addition to complete the suite; has anybody\nnoticed that git-var is cumbersome to use for this?\n"},{"id":"12212","messageId":"Pine.LNX.4.63.0511181136400.8037@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2582","inReplyTo":"7vd5kypjv2.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add test case for git-config-set","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-11-18T10:48:48Z","receivedAt":"2005-11-18T10:48:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 17 Nov 2005, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > +test_expect_failure 'ambiguous unset' \\\n> > +\t'git-config-set --unset nextsection.nonewline'\n> \n> I am not so sure about this case.  Shouldn't this remove both?\n\nNo. The common case should be a variable with a unique key/value pair. And \nin that case, it should be treated as an error if there are already more \nthan one pair.\n\n> For example, if a Porcelain wants to force pull.twohead to be\n> resolve and nothing else, and it wants to do it unconditionally,\n> it would first want to empty whatever multivalue there are\n> currently, and then insert its own, and I'd imagine the way to\n> say that would be like this:\n> \n> \tgit-config-set --unset pull.twohead '^'\n>         git-config-set pull.twohead resolve\n> \n> More simply (I do not think you have a test case for this):\n> \n>         git-config-set pull.twohead resolve '^'\n\nThis is a slippery slope. Giving a wrong regex by accident may destroy \n*all* your settings. Not what I want.\n\n> I think it is the easiest to explain and understand the\n> semantics of config_set_multivalue if it were to first remove all\n> existing key-value for matching ones, and then insert what was\n> provided by the user.\n\nI'd be very, very careful about that. For sure, I'd want to make the \nremoving of multiple values a single operation, which leaves a backup of \n.git/config behind.\n\n> Extending that multivalue example a bit more, I think it is a\n> bit cumbersome for a Porcelain to set pull.twohead to recursive\n> and then resolve, with your interface.  Even if you had the\n> emptying behaviour I suggested above, you would have to say\n> something awkard like this:\n> \n> \tgit-config-set --unset pull.twohead '^'\n>         git-config-set pull.twohead recursive\n>         git-config-set pull.twohead resolve no-such-value-should-be-there\n\nReally, I don't see the point in making twohead a multivar:\n\n\t[diff]\n\t\ttwohead = resolve recursive blabla\n\nThis looks much nicer to me, and should be easy to parse (even for human \neyes, who do not particularly like to find one value at the top of \n.git/config, and the second on the bottom, which is perfectly possible in \nyour setup).\n\n> Maybe we could have the shell-level interface like this:\n> \n> \tgit-config-set [--remove rx] section.key [value...]\n\nAs I said, I'd be *very* careful to remove multiple values. So, I would \nlike to *require* two steps for this.\n\n> \tgit-config-set section.key\n> \n> confusingly enough is --unset\n\nOkay. That was my original code before introducing multivars, where I did \nnot have --unset. This will go.\n\n> \tgit-config-set pull.twohead recursive resolve\n\nOh yes, yes. Just quote \"recursive resolve\"! Note that it is somewhat \nuncommon to make the behaviour of a variable in an ini file depend on its \nposition... (this is meant as one more point in favor of just one pair).\n\n> The C-level interface would become something like:\n> \n> \tgit_config_set_multivar(const char *key,\n>         \t\t\tconst char *remove_value_regex,\n>                                 const char **values)\n> \n> where values is a NULL terminated list of values.\n\nThe normal use case (for things like \"proxy.command\", which will hopefully \nnot become very common) will be to set/replace/unset exactly one value. \nFor what you want, your proposal might be good, but I deem a unique \nkey/value pair with a space separated list of methods a better solution.\n\n> BTW, do we want to remove the section after removing the last\n> key and making it empty?\n\nNo, because you might have comments in there. I am not willing to go as \nfar as removing something like\n\n\t[diff]\n\t\t;might want to use that someday\n\t\t;twohead = recursive megacool\n\nCiao,\nDscho\n"},{"id":"12214","messageId":"Pine.LNX.4.63.0511181148520.8037@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2582","inReplyTo":"7vsltuo49q.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add test case for git-config-set","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-11-18T10:57:14Z","receivedAt":"2005-11-18T10:57:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 17 Nov 2005, Junio C Hamano wrote:\n\n> Junio C Hamano <junkio@cox.net> writes:\n> \n> > \tgit-config-set section.key\n> >\n> > confusingly enough is --unset (we probably would want to require\n> > an explicit command line noise-word \"--unset\" in this case).\n\nAs mentioned in my previous mail, this will go.\n\n> A revised suggestion is:\n> \n> \t;# remove all\n> \tgit-config-set --unset section.key\n\nNo. Just remove one unique entry. If people insist it is a good idea to \nthrow away all multivars, I'll think about it. But I am not convinced. \nYour use case of a multivar does not fit what I think multivars are useful \nfor.\n\nIMHO, multiple key/value pairs are only sensible when they provide a sort \nof subkey, like Linus did in his example:\n\n\t[proxy]\n\t\tcommand=ssh for kernel.org\n\t\tcommand=rsh for myprivate.machine\n\nSee, there is a subkey, namely \"for kernel.org\", which can nicely be \nexpressed as a regex.\n\nIMOHO, your example\n\n\t[diff]\n\t\ttwohead = resolve\n\t\ttwohead = recursive\n\nwould look much better in this way\n\n\t[diff]\n\t\ttwohead = recursive resolve\n\nSeveral reasons:\n\n\t- it is easier to read\n\t- it does the job\n\t- it is clear from the beginning, which one precedes the other\n\t  (for me it was not at all clear that the last entry wins...)\n\n> \t;# remove values that match rx and then append zero or more values\n> \tgit-config-set --remove rx section.key [value...]\n\nAs I said. *If*, then these must be two operations, for your security. But \nI still don't see a sensible use case of removing all key/value pairs for \na certain key (and possibly a regex for the values).\n\nRather, if you have a sensible setup, you'll want to set/replace/unset \nexactly one entry.\n\n> \t;# append one or more values (equivalent to specifying --remove\n>         ;# with rx that never matches anything).  To reduce\n>         ;# confusion, we always require at least one value here.\n> \tgit-config-set section.key value [value...]\n\nI think it is less error prone to add them one by one, else you have to \ncheck the values for uniqueness, too.\n\n> I think (aside from \"*-set\" now becomes confusing), showing the\n> value of the specified key to stdout with\n> \n> \tgit-config-set section.key\n> \n> would be a nice addition to complete the suite; has anybody\n> noticed that git-var is cumbersome to use for this?\n\nGood point! This will be in my next version.\n\nCiao,\nDscho\n"}]}