{"thread":{"id":"29814","subject":"[PATCH 1/2] Complain in the tests about git config not failing with, keys without a section","startedAt":"2012-03-02T10:57:34Z","lastAt":"2012-03-06T08:06:24Z","messageCount":6,"participants":["Rune Schjellerup Philosof","Junio C Hamano","Rune Philosof","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"185906","messageId":"4F50A79E.5060206@philosof.dk","threadId":"29814","inReplyTo":null,"subject":"[PATCH 1/2] Complain in the tests about git config not failing with, keys without a section","fromName":"Rune Schjellerup Philosof","fromEmail":"rune@philosof.dk","sentAt":"2012-03-02T10:57:34Z","receivedAt":"2012-03-02T10:57:34Z","isPatch":true,"sender":{"key":"rune@philosof.dk","avatar":null},"body":"From c4fc13a5a1e8c1eab9873232e6e8b7e0523cd6ea Mon Sep 17 00:00:00 2001\nFrom: Rune Philosof \nDate: Fri, 2 Mar 2012 10:42:23 +0100\nSubject: [PATCH 1/2] Complain in the tests about git config not failing with\n keys without a section\n\ngit is supposed to fail when having a key without a section, but does not.\n\nSigned-off-by: Rune Philosof \n---\n t/t1300-repo-config.sh | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 5f249f6..81ccad5 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -928,6 +928,11 @@ test_expect_success 'git -c \"key=value\" support' '\n \ttest_must_fail git -c name=value config core.name\n '\n\n+test_expect_failure 'No section' '\n+\techo \"fail=true\" > failconf &&\n+\ttest_must_fail git config --file failconf --list\n+'\n+\n test_expect_success 'key sanity-checking' '\n \ttest_must_fail git config foo=bar &&\n \ttest_must_fail git config foo=.bar &&\n-- \n1.7.5.4\n\n"},{"id":"185937","messageId":"7v8vjiethh.fsf@alter.siamese.dyndns.org","threadId":"29814","inReplyTo":"4F50A79E.5060206@philosof.dk","subject":"Re: [PATCH 1/2] Complain in the tests about git config not failing with, keys without a section","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-02T18:40:58Z","receivedAt":"2012-03-02T18:40:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rune Schjellerup Philosof writes:\n\n> From c4fc13a5a1e8c1eab9873232e6e8b7e0523cd6ea Mon Sep 17 00:00:00 2001\n> From: Rune Philosof \n> Date: Fri, 2 Mar 2012 10:42:23 +0100\n> Subject: [PATCH 1/2] Complain in the tests about git config not failing with\n> keys without a section\n\nWhat are these lines?\n\n>\n> git is supposed to fail when having a key without a section, but does not.\n\nI do not think anybody said it is supposed to fail in this case.\n\nHistorically, we always allowed single level names in our configuration\nparser, at least for reading, from pretty much day one, I think. The\ndocumentation only talks about two and three level names, because we do\nnot use such a name ourselves in the code, and the behaviour for a single\nlevel name is just \"undefined\", which is very different from \"must fail\".\n"},{"id":"186089","messageId":"4F547261.7000506@philosof.dk","threadId":"29814","inReplyTo":"7v8vjiethh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] Complain in the tests about git config not failing with, keys without a section","fromName":"Rune Philosof","fromEmail":"rune@philosof.dk","sentAt":"2012-03-05T07:59:29Z","receivedAt":"2012-03-05T07:59:29Z","isPatch":true,"sender":{"key":"rune@philosof.dk","avatar":null},"body":"On 02-03-2012 19:40, Junio C Hamano wrote:\n>> git is supposed to fail when having a key without a section, but does not.\n> I do not think anybody said it is supposed to fail in this case.\n>\n> the behaviour for a single level name is just \"undefined\", which is very different from \"must fail\".\n\nQuoting from `git help config`:\nThis command will fail if:\n 3. no section was provided,\n...\nunder SYNTAX\n Each variable must belong to some section, which means that there must \nbe a section header before the first\n setting of a variable.\n\n--\nGreeting\nRune Philosof\n"},{"id":"186096","messageId":"20120305103714.GA30463@sigill.intra.peff.net","threadId":"29814","inReplyTo":"4F547261.7000506@philosof.dk","subject":"Re: [PATCH 1/2] Complain in the tests about git config not failing with, keys without a section","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-05T10:37:14Z","receivedAt":"2012-03-05T10:37:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 05, 2012 at 08:59:29AM +0100, Rune Philosof wrote:\n\n> On 02-03-2012 19:40, Junio C Hamano wrote:\n> >>git is supposed to fail when having a key without a section, but does not.\n> >I do not think anybody said it is supposed to fail in this case.\n> >\n> >the behaviour for a single level name is just \"undefined\", which is very different from \"must fail\".\n> \n> Quoting from `git help config`:\n> This command will fail if:\n> 3. no section was provided,\n\nThe text you are quoting is not about what is in the config file, but\nrather the config name given on the command line (which we would be\ntrying to look up). And we do correctly complain about that:\n\n $ git config foo\n error: key does not contain a section: foo\n\nBut:\n\n> under SYNTAX\n> Each variable must belong to some section, which means that there\n> must be a section header before the first\n> setting of a variable.\n\nYes, everything is supposed to be in a section. Historically we have\nnot complained, but instead just treated it as a single-level variable.\nFor internal git use this never mattered, as git only looked at\nvariables with section names. For \"git config foo\", it also does not\nmatter, since we will notice the lack of section before even doing a\nlookup.\n\nFor \"git config --list\", as you noticed, we include it in the output. I\nsuspect we should simply omit it as cruft. But we could also issue a\nwarning, and/or die.\n\n-Peff\n"},{"id":"186133","messageId":"7veht626ex.fsf@alter.siamese.dyndns.org","threadId":"29814","inReplyTo":"20120305103714.GA30463@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] Complain in the tests about git config not failing with, keys without a section","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-05T19:29:10Z","receivedAt":"2012-03-05T19:29:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> For \"git config --list\", as you noticed, we include it in the output. I\n> suspect we should simply omit it as cruft. But we could also issue a\n> warning, and/or die.\n\nA new warning might be worth if we were to suddenly start omitting\nthem from --list output, but dying won't fly well.\n\nPeople have been happily lived with their config with such lines\nthat do not affect what git does, but a new version of git starts to\n\"die\" on them. For what purpose? Whom is such a change designed to\nhelp?\n"},{"id":"186176","messageId":"20120306080624.GB21199@sigill.intra.peff.net","threadId":"29814","inReplyTo":"7veht626ex.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] Complain in the tests about git config not failing with, keys without a section","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-06T08:06:24Z","receivedAt":"2012-03-06T08:06:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 05, 2012 at 11:29:10AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > For \"git config --list\", as you noticed, we include it in the output. I\n> > suspect we should simply omit it as cruft. But we could also issue a\n> > warning, and/or die.\n> \n> A new warning might be worth if we were to suddenly start omitting\n> them from --list output, but dying won't fly well.\n> \n> People have been happily lived with their config with such lines\n> that do not affect what git does, but a new version of git starts to\n> \"die\" on them. For what purpose? Whom is such a change designed to\n> help?\n\nThe only people you might help are those who would like to be informed\nthat their config file is bogus, and that those entries are being\nignored (which might be an error, for example, if they accidentally\ndeleted a section header). But I don't think it's a huge deal; it\ncertainly is not the first config error one could make that isn't\ndetected (e.g., you could just as easily typo \"core.quotpath\", and git\nwill never say a word).\n\nA warning probably serves to inform such people just as well as dying.\nI am actually just fine with omitting it from --list, too. We can be\nliberal in what we accept from people's config files, and just pretend\nthat bogus things do not exist (just like core.quotpath ends up being\nsilently ignored). You cannot access a section-less variable via \"git\nconfig\" directly, so it is probably better to just act as if it does not\nexist.\n\nFor the record, I am also fine with just leaving it as-is. This seems\nlike such a low priority that until somebody shows up with a patch, I\ncan't bring myself to care too much.\n\n-Peff\n"}]}