{"thread":{"id":"18308","subject":"[PATCH] config: --replace-all with one argument exits properly with a better message.","startedAt":"2009-03-14T02:42:32Z","lastAt":"2009-03-16T15:25:46Z","messageCount":7,"participants":["Carlos Rica","Junio C Hamano","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"107992","messageId":"1236998552.9952.2.camel@luis-desktop","threadId":"18308","inReplyTo":null,"subject":"[PATCH] config: --replace-all with one argument exits properly with a better message.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2009-03-14T02:42:32Z","receivedAt":"2009-03-14T02:42:32Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"'config --replace-all ONE_ARG' was being treated as 'config NAME VALUE',\nshowing the error \"key does not contain a section: --replace-all\".\n\nNow it exits before with an error message asking for the missing value.\nDocumentation is updated and a new test is added to ensure that\nconfiguration remains the same when no value is provided.\n\nSigned-off-by: Carlos Rica \n---\n Documentation/git-config.txt | 2 +-\n builtin-config.c | 2 ++\n t/t1300-repo-config.sh | 9 ++++++++-\n 3 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 82ce89e..7131ee3 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n [verse]\n 'git config' [] [type] [-z|--null] name [value [value_regex]]\n 'git config' [] [type] --add name value\n-'git config' [] [type] --replace-all name [value [value_regex]]\n+'git config' [] [type] --replace-all name value [value_regex]\n 'git config' [] [type] [-z|--null] --get name [value_regex]\n 'git config' [] [type] [-z|--null] --get-all name [value_regex]\n 'git config' [] [type] [-z|--null] --get-regexp name_regex [value_regex]\ndiff --git a/builtin-config.c b/builtin-config.c\nindex d52a057..005b6ea 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -386,6 +386,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\treturn git_config_set_multivar(argv[2], NULL, NULL, 1);\n \t\telse if (!strcmp(argv[1], \"--get\"))\n \t\t\treturn get_value(argv[2], NULL);\n+\t\telse if (!strcmp(argv[1], \"--replace-all\"))\n+\t\t\treturn error(\"missing value for --replace-all\");\n \t\telse if (!strcmp(argv[1], \"--get-all\")) {\n \t\t\tdo_all = 1;\n \t\t\treturn get_value(argv[2], NULL);\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 3c06842..9c81e04 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -118,7 +118,14 @@ EOF\n \n test_expect_success 'multiple unset is correct' 'cmp .git/config expect'\n \n-mv .git/config2 .git/config\n+cp .git/config2 .git/config\n+\n+test_expect_success '--replace-all missing value' '\n+\ttest_must_fail git config --replace-all beta.haha &&\n+\ttest_cmp .git/config2 .git/config\n+'\n+\n+rm .git/config2\n \n test_expect_success '--replace-all' \\\n \t'git config --replace-all beta.haha gamma'\n-- \n1.5.4.3\n"},{"id":"108021","messageId":"7vtz5vakrp.fsf@gitster.siamese.dyndns.org","threadId":"18308","inReplyTo":"1236998552.9952.2.camel@luis-desktop","subject":"Re: [PATCH] config: --replace-all with one argument exits properly with a better message.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-14T20:53:46Z","receivedAt":"2009-03-14T20:53:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Rica writes:\n\n> 'config --replace-all ONE_ARG' was being treated as 'config NAME VALUE',\n> showing the error \"key does not contain a section: --replace-all\".\n\nHmm, I am getting \"error: wrong number of arguments\" followed by the long\nand somewhat annoying \"usage\" from the parseopt table dump.\n\nAhh, that is because I am running the version from 'next', which contains\nthe fc/parseopt-config topic that rewrites the option parser and error\nchecking almost completely.\n\nCan you work with Felipe to see if this is still needed, or needs to be\nfixed in a different way? It could be that your tests may already pass\nover there on 'next'. I didn't check.\n"},{"id":"108027","messageId":"94a0d4530903141434w2fb8aa28we087465482a12e41@mail.gmail.com","threadId":"18308","inReplyTo":"7vtz5vakrp.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] config: --replace-all with one argument exits properly with a better message.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-03-14T21:34:55Z","receivedAt":"2009-03-14T21:34:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Mar 14, 2009 at 10:53 PM, Junio C Hamano wrote:\n> Carlos Rica writes:\n>\n>> 'config --replace-all ONE_ARG' was being treated as 'config NAME VALUE',\n>> showing the error \"key does not contain a section: --replace-all\".\n>\n> Hmm, I am getting \"error: wrong number of arguments\" followed by the long\n> and somewhat annoying \"usage\" from the parseopt table dump.\n\nIf you find it annoying why don't you remove the usage?\n\n> Ahh, that is because I am running the version from 'next', which contains\n> the fc/parseopt-config topic that rewrites the option parser and error\n> checking almost completely.\n>\n> Can you work with Felipe to see if this is still needed, or needs to be\n> fixed in a different way?  It could be that your tests may already pass\n> over there on 'next'.  I didn't check.\n\nThe new code is already checking correctly that --replace-all needs at\nleast two arguments. However, the \"usage\" is incorrect and of course\nthe test will come in handy.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"108030","messageId":"7vab7na6wb.fsf@gitster.siamese.dyndns.org","threadId":"18308","inReplyTo":"94a0d4530903141434w2fb8aa28we087465482a12e41@mail.gmail.com","subject":"Re: [PATCH] config: --replace-all with one argument exits properly with a better message.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-15T01:53:24Z","receivedAt":"2009-03-15T01:53:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Sat, Mar 14, 2009 at 10:53 PM, Junio C Hamano wrote:\n>> Carlos Rica writes:\n>>\n>>> 'config --replace-all ONE_ARG' was being treated as 'config NAME VALUE',\n>>> showing the error \"key does not contain a section: --replace-all\".\n>>\n>> Hmm, I am getting \"error: wrong number of arguments\" followed by the long\n>> and somewhat annoying \"usage\" from the parseopt table dump.\n>\n> If you find it annoying why don't you remove the usage?\n\nBecause the primary target audience of the help text is not me?\n\n>> Can you work with Felipe to see if this is still needed, or needs to be\n>> fixed in a different way?  It could be that your tests may already pass\n>> over there on 'next'.  I didn't check.\n>\n> The new code is already checking correctly that --replace-all needs at\n> least two arguments. However, the \"usage\" is incorrect and of course\n> the test will come in handy.\n\nSo perhaps you can pick a part of it and send in an update to your\nparseoptification series? I think the series is ready for 'master'\nsometime next week if not sooner.\n"},{"id":"108038","messageId":"94a0d4530903150326u34a0715v38269417e2785db8@mail.gmail.com","threadId":"18308","inReplyTo":"7vab7na6wb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] config: --replace-all with one argument exits properly with a better message.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-03-15T10:26:30Z","receivedAt":"2009-03-15T10:26:30Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Mar 15, 2009 at 3:53 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Sat, Mar 14, 2009 at 10:53 PM, Junio C Hamano wrote:\n>>> Carlos Rica writes:\n>>>\n>>>> 'config --replace-all ONE_ARG' was being treated as 'config NAME VALUE',\n>>>> showing the error \"key does not contain a section: --replace-all\".\n>>>\n>>> Hmm, I am getting \"error: wrong number of arguments\" followed by the long\n>>> and somewhat annoying \"usage\" from the parseopt table dump.\n>>\n>> If you find it annoying why don't you remove the usage?\n>\n> Because the primary target audience of the help text is not me?\n\nOk. I don't think it makes a big difference to leave it on or off.\nPeople not familiar with 'git config' might find it handy, but I admit\nthat I also find it a bit annoying, mainly because the error message\ngets lost in the noise.\n\n>>> Can you work with Felipe to see if this is still needed, or needs to be\n>>> fixed in a different way?  It could be that your tests may already pass\n>>> over there on 'next'.  I didn't check.\n>>\n>> The new code is already checking correctly that --replace-all needs at\n>> least two arguments. However, the \"usage\" is incorrect and of course\n>> the test will come in handy.\n>\n> So perhaps you can pick a part of it and send in an update to your\n> parseoptification series?  I think the series is ready for 'master'\n> sometime next week if not sooner.\n\nOr maybe Carlos can beat me to do it since it seems he is interested.\nOtherwise yeah, I'll do it.\n\n-- \nFelipe Contreras\n"},{"id":"108085","messageId":"1b46aba20903160741y64598f92gda5cfe9c8dd31586@mail.gmail.com","threadId":"18308","inReplyTo":"94a0d4530903150326u34a0715v38269417e2785db8@mail.gmail.com","subject":"Re: [PATCH] config: --replace-all with one argument exits properly with a better message.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2009-03-16T14:41:35Z","receivedAt":"2009-03-16T14:41:35Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"Hi Felipe, I didn't know that you were writing the parse options for\nconfig. I tried it a year ago and I leave it unfinished because (if I\nremember correctly) options like -4, -5, -6... and those:\nhttp://thread.gmane.org/gmane.comp.version-control.git/78480\n\nOn Sun, Mar 15, 2009 at 11:26 AM, Felipe Contreras\n wrote:\n> On Sun, Mar 15, 2009 at 3:53 AM, Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>>\n>>> On Sat, Mar 14, 2009 at 10:53 PM, Junio C Hamano wrote:\n>>>> Carlos Rica writes:\n>>>>\n>>>>> 'config --replace-all ONE_ARG' was being treated as 'config NAME VALUE',\n>>>>> showing the error \"key does not contain a section: --replace-all\".\n>>>>\n>>>> Hmm, I am getting \"error: wrong number of arguments\" followed by the long\n>>>> and somewhat annoying \"usage\" from the parseopt table dump.\n>>>\n>>> If you find it annoying why don't you remove the usage?\n>>\n>> Because the primary target audience of the help text is not me?\n>\n> Ok. I don't think it makes a big difference to leave it on or off.\n> People not familiar with 'git config' might find it handy, but I admit\n> that I also find it a bit annoying, mainly because the error message\n> gets lost in the noise.\n>\n>>>> Can you work with Felipe to see if this is still needed, or needs to be\n>>>> fixed in a different way? It could be that your tests may already pass\n>>>> over there on 'next'. I didn't check.\n>>>\n>>> The new code is already checking correctly that --replace-all needs at\n>>> least two arguments. However, the \"usage\" is incorrect and of course\n>>> the test will come in handy.\n>>\n>> So perhaps you can pick a part of it and send in an update to your\n>> parseoptification series? I think the series is ready for 'master'\n>> sometime next week if not sooner.\n>\n> Or maybe Carlos can beat me to do it since it seems he is interested.\n> Otherwise yeah, I'll do it.\n\nOf course, I'm looking at your code in \"pu\" to see how could apply this.\n"},{"id":"108091","messageId":"94a0d4530903160825h2e4fae8fy9fd53271a2944d40@mail.gmail.com","threadId":"18308","inReplyTo":"1b46aba20903160741y64598f92gda5cfe9c8dd31586@mail.gmail.com","subject":"Re: [PATCH] config: --replace-all with one argument exits properly with a better message.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-03-16T15:25:46Z","receivedAt":"2009-03-16T15:25:46Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Mar 16, 2009 at 4:41 PM, Carlos Rica wrote:\n> Hi Felipe, I didn't know that you were writing the parse options for\n> config. I tried it a year ago and I leave it unfinished because (if I\n> remember correctly) options like -4, -5, -6... and those:\n> http://thread.gmane.org/gmane.comp.version-control.git/78480\n\nI found the same issue, but Johannes suggested to use\nPARSE_OPT_STOP_AT_NON_OPTION :)\n\n-- \nFelipe Contreras\n"}]}