{"thread":{"id":"41752","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","startedAt":"2016-03-19T16:49:02Z","lastAt":"2016-03-24T17:22:12Z","messageCount":10,"participants":["Chirayu Desai","Pranit Bauva","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"281236","messageId":"CAJj6+1Fcp+Fjx9N6Mon1A5uP-_npnPL1Acu5-cR_bHVfs3EMWA@mail.gmail.com","threadId":"41752","inReplyTo":null,"subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Chirayu Desai","fromEmail":"chirayudesai1@gmail.com","sentAt":"2016-03-19T16:49:02Z","receivedAt":"2016-03-19T16:49:02Z","isPatch":false,"sender":{"key":"chirayudesai1@gmail.com","avatar":"https://gravatar.com/avatar/c2d0bd062b197c940eaaf3ac349f63fdbe0df58a2421c140ee8c9b4f52af95e3?d=mp&s=160"},"body":"Hi, I want to work on this as my GSoC micro project.\n\n> On Mon, Jan 18, 2016 at 10:24:31PM +0100, Toralf Förster wrote:\n> > very first line is \"error: malformed object name \" which tells all, or ?\n> Yeah, I agree that showing the \"-h\" help is a bit much.\n> This is a side effect of looking up in the commit in the parse-options\n> callback. It has to signal an error to the option parser, and then the\n> option parser always shows the help on an error.\n> I think we'd need to do one of:\n> 1. call die() in the option-parsing callback (this is probably a bad\n> precedent, as the callbacks might be reused from a place that wants\n> to behave differently)\nI assume you mean parse-options-cb.c:parse_opt_commits() by the callback.\nI see that it is currently used only by commands which have a \"--with\"\nor \"--contains\" option,\nand all of them behave the same way, printing the full usage, so a one\nline change in that function would fix it for all of those.\n> 2. have the callback just store the argument string, and then resolve\n> the commit later (and die or whatever if it doesn't exist). This\n> pushes more work onto the caller, but in this case it's all done by\n> the ref-filter code, so it could presumably happen during another\n> part of the ref-filter setup.\nI'm not quire sure how exactly to do that.\n> 3. teach parse-options to accept some specific non-zero return code\n> that means \"return an error, but don't show the usage\"\nThis sounds good, but also the most intrusive of 3.\n> I think any one of those would be a good project for somebody looking to\n> get their feet wet in working on git. I think (2) is the cleanest.\n> -Peff\n\nWhat would be the best way to proceed with this?\n\nThanks,\nChirayu Desai\n"},{"id":"281239","messageId":"CAFZEwPP1GwH6a1kLTCn6ETov6YeK-t9PFJ_-wWP2P6v7CObiGQ@mail.gmail.com","threadId":"41752","inReplyTo":"CAJj6+1Fcp+Fjx9N6Mon1A5uP-_npnPL1Acu5-cR_bHVfs3EMWA@mail.gmail.com","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-03-19T17:04:58Z","receivedAt":"2016-03-19T17:04:58Z","isPatch":false,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"On Sat, Mar 19, 2016 at 10:19 PM, Chirayu Desai wrote:\n> Hi, I want to work on this as my GSoC micro project.\n>\n>> On Mon, Jan 18, 2016 at 10:24:31PM +0100, Toralf Förster wrote:\n>> > very first line is \"error: malformed object name \" which tells all, or ?\n>> Yeah, I agree that showing the \"-h\" help is a bit much.\n>> This is a side effect of looking up in the commit in the parse-options\n>> callback. It has to signal an error to the option parser, and then the\n>> option parser always shows the help on an error.\n>> I think we'd need to do one of:\n>> 1. call die() in the option-parsing callback (this is probably a bad\n>> precedent, as the callbacks might be reused from a place that wants\n>> to behave differently)\n> I assume you mean parse-options-cb.c:parse_opt_commits() by the callback.\n> I see that it is currently used only by commands which have a \"--with\"\n> or \"--contains\" option,\n> and all of them behave the same way, printing the full usage, so a one\n> line change in that function would fix it for all of those.\n>> 2. have the callback just store the argument string, and then resolve\n>> the commit later (and die or whatever if it doesn't exist). This\n>> pushes more work onto the caller, but in this case it's all done by\n>> the ref-filter code, so it could presumably happen during another\n>> part of the ref-filter setup.\n> I'm not quire sure how exactly to do that.\n>> 3. teach parse-options to accept some specific non-zero return code\n>> that means \"return an error, but don't show the usage\"\n> This sounds good, but also the most intrusive of 3.\n>> I think any one of those would be a good project for somebody looking to\n>> get their feet wet in working on git. I think (2) is the cleanest.\n>> -Peff\n>\n> What would be the best way to proceed with this?\n\nThe extract that you posted isn't very clear.\nI guess posting a link with the previous discussion would be quite\nhelpful as some people don't have the previous emails in the inbox.\nThe archives can be found at\nhttp://dir.gmane.org/gmane.comp.version-control.git .\n"},{"id":"281241","messageId":"CAJj6+1Fgj7VyVSSi2Qy=yEtEjuQWb7A8GDX-mY7h3V_mnRY2bw@mail.gmail.com","threadId":"41752","inReplyTo":"CAFZEwPP1GwH6a1kLTCn6ETov6YeK-t9PFJ_-wWP2P6v7CObiGQ@mail.gmail.com","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Chirayu Desai","fromEmail":"chirayudesai1@gmail.com","sentAt":"2016-03-19T17:51:42Z","receivedAt":"2016-03-19T17:51:42Z","isPatch":false,"sender":{"key":"chirayudesai1@gmail.com","avatar":"https://gravatar.com/avatar/c2d0bd062b197c940eaaf3ac349f63fdbe0df58a2421c140ee8c9b4f52af95e3?d=mp&s=160"},"body":"The original discussion [1] (the e-mail to which Jeff replied) doesn't\ncontain much either, but I'll try to explain it.\n\n$ git tag --contains q\nerror: malformed object name q\nusage: git tag ...\n\n\nThe same happens with 'git branch --contains', and 'git for-each-ref\n--contains`, as they use the same underlying code. Just showing the\nerror message would be enough in this case.\n\nThe issue here is that the callback returns the \"error: malformed\nobject name %s\" print, which parse_options sees as some sort of error\nand tries to help the user by printing the usage. [2]\n\nOne simple solution would be to change \"return error(\"malformed object\nname %s\", arg);\" to a die(\"message\"); return;, however it might not be\nthe best option (though I'm not seeing any other user of this\nfunction, the only place it is being used is for OPT_CONTAINS and\nOPT_WITH\n\n> 3. teach parse-options to accept some specific non-zero return code that means \"return an error, but don't show the usage\"\nWould be a good general purpose alternative.\n\nI need to look at this a bit more, and also get some hints / clarity\nfrom Jeff regarding 2. if possible.\n\nThanks,\nChirayu Desai\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/284323\n[2] https://github.com/git/git/blob/047057bb4159533b3323003f89160588c9e61fbd/parse-options-cb.c#L81\n\nOn Sat, Mar 19, 2016 at 10:34 PM, Pranit Bauva wrote:\n> On Sat, Mar 19, 2016 at 10:19 PM, Chirayu Desai wrote:\n>> Hi, I want to work on this as my GSoC micro project.\n>>\n>>> On Mon, Jan 18, 2016 at 10:24:31PM +0100, Toralf Förster wrote:\n>>> > very first line is \"error: malformed object name \" which tells all, or ?\n>>> Yeah, I agree that showing the \"-h\" help is a bit much.\n>>> This is a side effect of looking up in the commit in the parse-options\n>>> callback. It has to signal an error to the option parser, and then the\n>>> option parser always shows the help on an error.\n>>> I think we'd need to do one of:\n>>> 1. call die() in the option-parsing callback (this is probably a bad\n>>> precedent, as the callbacks might be reused from a place that wants\n>>> to behave differently)\n>> I assume you mean parse-options-cb.c:parse_opt_commits() by the callback.\n>> I see that it is currently used only by commands which have a \"--with\"\n>> or \"--contains\" option,\n>> and all of them behave the same way, printing the full usage, so a one\n>> line change in that function would fix it for all of those.\n>>> 2. have the callback just store the argument string, and then resolve\n>>> the commit later (and die or whatever if it doesn't exist). This\n>>> pushes more work onto the caller, but in this case it's all done by\n>>> the ref-filter code, so it could presumably happen during another\n>>> part of the ref-filter setup.\n>> I'm not quire sure how exactly to do that.\n>>> 3. teach parse-options to accept some specific non-zero return code\n>>> that means \"return an error, but don't show the usage\"\n>> This sounds good, but also the most intrusive of 3.\n>>> I think any one of those would be a good project for somebody looking to\n>>> get their feet wet in working on git. I think (2) is the cleanest.\n>>> -Peff\n>>\n>> What would be the best way to proceed with this?\n>\n> The extract that you posted isn't very clear.\n> I guess posting a link with the previous discussion would be quite\n> helpful as some people don't have the previous emails in the inbox.\n> The archives can be found at\n> http://dir.gmane.org/gmane.comp.version-control.git .\n"},{"id":"281242","messageId":"20160319175705.GA6989@sigill.intra.peff.net","threadId":"41752","inReplyTo":"CAJj6+1Fcp+Fjx9N6Mon1A5uP-_npnPL1Acu5-cR_bHVfs3EMWA@mail.gmail.com","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-19T17:57:05Z","receivedAt":"2016-03-19T17:57:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 19, 2016 at 10:19:02PM +0530, Chirayu Desai wrote:\n\n> > Yeah, I agree that showing the \"-h\" help is a bit much.\n> > This is a side effect of looking up in the commit in the parse-options\n> > callback. It has to signal an error to the option parser, and then the\n> > option parser always shows the help on an error.\n> > I think we'd need to do one of:\n> > 1. call die() in the option-parsing callback (this is probably a bad\n> > precedent, as the callbacks might be reused from a place that wants\n> > to behave differently)\n> I assume you mean parse-options-cb.c:parse_opt_commits() by the callback.\n> I see that it is currently used only by commands which have a \"--with\"\n> or \"--contains\" option,\n> and all of them behave the same way, printing the full usage, so a one\n> line change in that function would fix it for all of those.\n\nYes, that is the right callback.\n\n> > 2. have the callback just store the argument string, and then resolve\n> > the commit later (and die or whatever if it doesn't exist). This\n> > pushes more work onto the caller, but in this case it's all done by\n> > the ref-filter code, so it could presumably happen during another\n> > part of the ref-filter setup.\n> I'm not quire sure how exactly to do that.\n\nYou'd teach parse_opt_commits() to store the string _name_ of the\nargument (e.g., using a string_list rather than a commit_list), and then\nlater resolve those names into commits.\n\n> > 3. teach parse-options to accept some specific non-zero return code\n> > that means \"return an error, but don't show the usage\"\n> This sounds good, but also the most intrusive of 3.\n\nYeah. Reading the options again, I kind of like this one. The only trick\nis that you would need to make sure no other callbacks are returning the\nvalue you choose for the \"don't show the usage\" flag. That is probably\nnot too bad, though. There aren't that many callbacks, and they are not\nlikely to be using values besides \"-1\" and \"0\".\n\n-Peff\n"},{"id":"281243","messageId":"CAJj6+1HaVnRcmDHOTDdx=o8a+aXvSi8+LykWzrfx7knE-_3ocg@mail.gmail.com","threadId":"41752","inReplyTo":"20160319175705.GA6989@sigill.intra.peff.net","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Chirayu Desai","fromEmail":"chirayudesai1@gmail.com","sentAt":"2016-03-19T18:08:09Z","receivedAt":"2016-03-19T18:08:09Z","isPatch":false,"sender":{"key":"chirayudesai1@gmail.com","avatar":"https://gravatar.com/avatar/c2d0bd062b197c940eaaf3ac349f63fdbe0df58a2421c140ee8c9b4f52af95e3?d=mp&s=160"},"body":"On Sat, Mar 19, 2016 at 11:27 PM, Jeff King wrote:\n> On Sat, Mar 19, 2016 at 10:19:02PM +0530, Chirayu Desai wrote:\n>\n>> > Yeah, I agree that showing the \"-h\" help is a bit much.\n>> > This is a side effect of looking up in the commit in the parse-options\n>> > callback. It has to signal an error to the option parser, and then the\n>> > option parser always shows the help on an error.\n>> > I think we'd need to do one of:\n>> > 1. call die() in the option-parsing callback (this is probably a bad\n>> > precedent, as the callbacks might be reused from a place that wants\n>> > to behave differently)\n>> I assume you mean parse-options-cb.c:parse_opt_commits() by the callback.\n>> I see that it is currently used only by commands which have a \"--with\"\n>> or \"--contains\" option,\n>> and all of them behave the same way, printing the full usage, so a one\n>> line change in that function would fix it for all of those.\n>\n> Yes, that is the right callback.\n>\n>> > 2. have the callback just store the argument string, and then resolve\n>> > the commit later (and die or whatever if it doesn't exist). This\n>> > pushes more work onto the caller, but in this case it's all done by\n>> > the ref-filter code, so it could presumably happen during another\n>> > part of the ref-filter setup.\n>> I'm not quire sure how exactly to do that.\n>\n> You'd teach parse_opt_commits() to store the string _name_ of the\n> argument (e.g., using a string_list rather than a commit_list), and then\n> later resolve those names into commits.\nGotcha, will need to figure out where exactly would those names be\nresolved, can do after following the code path a bit more, can do.\n>\n>> > 3. teach parse-options to accept some specific non-zero return code\n>> > that means \"return an error, but don't show the usage\"\n>> This sounds good, but also the most intrusive of 3.\n>\n> Yeah. Reading the options again, I kind of like this one. The only trick\n> is that you would need to make sure no other callbacks are returning the\n> value you choose for the \"don't show the usage\" flag. That is probably\n> not too bad, though. There aren't that many callbacks, and they are not\n> likely to be using values besides \"-1\" and \"0\".\nI'll try to go through that as well.\n>\n> -Peff\n\n-Chirayu\n"},{"id":"281244","messageId":"20160319181228.GA9115@sigill.intra.peff.net","threadId":"41752","inReplyTo":"CAJj6+1HaVnRcmDHOTDdx=o8a+aXvSi8+LykWzrfx7knE-_3ocg@mail.gmail.com","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-19T18:12:28Z","receivedAt":"2016-03-19T18:12:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 19, 2016 at 11:38:09PM +0530, Chirayu Desai wrote:\n\n> > You'd teach parse_opt_commits() to store the string _name_ of the\n> > argument (e.g., using a string_list rather than a commit_list), and then\n> > later resolve those names into commits.\n> Gotcha, will need to figure out where exactly would those names be\n> resolved, can do after following the code path a bit more, can do.\n\nWithout looking too closely, I suspect you could do it as the first step\nin filter_refs(). Or alternatively, add a function to \"finalize\" the\nfilter state before making queries of it.\n\n-Peff\n"},{"id":"281260","messageId":"CAJj6+1H6L=LxnDRzuC6OzXgVvzXsngGJ5X=E5Fi6Fg7JXkEJaQ@mail.gmail.com","threadId":"41752","inReplyTo":"20160319181228.GA9115@sigill.intra.peff.net","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Chirayu Desai","fromEmail":"chirayudesai1@gmail.com","sentAt":"2016-03-20T06:49:46Z","receivedAt":"2016-03-20T06:49:46Z","isPatch":false,"sender":{"key":"chirayudesai1@gmail.com","avatar":"https://gravatar.com/avatar/c2d0bd062b197c940eaaf3ac349f63fdbe0df58a2421c140ee8c9b4f52af95e3?d=mp&s=160"},"body":"I went for 3, and have sent a patch for that here - [PATCH/GSoC]\nparse-options: Add a new nousage opt\nHowever, it currently has one bug\nRunning 'git tag --contains qq' twice will first show an error, then\nprint qq, meaning that the first command creates the tag qq.\nRunning 'git tag -l --contains qq' works fine.\nMy first question is if 'git tag --contains' (without '-l') supposed to work?\nIf not, then I would fix that bug, otherwise fix the bug my code\nintroduced, and add tests for it.\n\n-Chirayu\n\nOn Sat, Mar 19, 2016 at 11:42 PM, Jeff King wrote:\n> On Sat, Mar 19, 2016 at 11:38:09PM +0530, Chirayu Desai wrote:\n>\n>> > You'd teach parse_opt_commits() to store the string _name_ of the\n>> > argument (e.g., using a string_list rather than a commit_list), and then\n>> > later resolve those names into commits.\n>> Gotcha, will need to figure out where exactly would those names be\n>> resolved, can do after following the code path a bit more, can do.\n>\n> Without looking too closely, I suspect you could do it as the first step\n> in filter_refs(). Or alternatively, add a function to \"finalize\" the\n> filter state before making queries of it.\n>\n> -Peff\n"},{"id":"281311","messageId":"xmqqoaa8okhr.fsf@gitster.mtv.corp.google.com","threadId":"41752","inReplyTo":"20160319175705.GA6989@sigill.intra.peff.net","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-20T22:25:04Z","receivedAt":"2016-03-20T22:25:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sat, Mar 19, 2016 at 10:19:02PM +0530, Chirayu Desai wrote:\n>\n>> > Yeah, I agree that showing the \"-h\" help is a bit much.\n>> > This is a side effect of looking up in the commit in the parse-options\n>> > callback. It has to signal an error to the option parser, and then the\n>> > option parser always shows the help on an error.\n>> > I think we'd need to do one of:\n>> > 1. call die() in the option-parsing callback (this is probably a bad\n>> > precedent, as the callbacks might be reused from a place that wants\n>> > to behave differently)\n>> I assume you mean parse-options-cb.c:parse_opt_commits() by the callback.\n>> I see that it is currently used only by commands which have a \"--with\"\n>> or \"--contains\" option,\n>> and all of them behave the same way, printing the full usage, so a one\n>> line change in that function would fix it for all of those.\n>\n> Yes, that is the right callback.\n>\n>> > 2. have the callback just store the argument string, and then resolve\n>> > the commit later (and die or whatever if it doesn't exist). This\n>> > pushes more work onto the caller, but in this case it's all done by\n>> > the ref-filter code, so it could presumably happen during another\n>> > part of the ref-filter setup.\n>> I'm not quire sure how exactly to do that.\n>\n> You'd teach parse_opt_commits() to store the string _name_ of the\n> argument (e.g., using a string_list rather than a commit_list), and then\n> later resolve those names into commits.\n>\n>> > 3. teach parse-options to accept some specific non-zero return code\n>> > that means \"return an error, but don't show the usage\"\n>> This sounds good, but also the most intrusive of 3.\n>\n> Yeah. Reading the options again, I kind of like this one. The only trick\n> is that you would need to make sure no other callbacks are returning the\n> value you choose for the \"don't show the usage\" flag. That is probably\n> not too bad, though. There aren't that many callbacks, and they are not\n> likely to be using values besides \"-1\" and \"0\".\n\nMy knee-jerk preference among the three is 2., but I think I'll know\nwhen I see a patch ;-)\n\nThanks for helping the candidates.\n"},{"id":"281612","messageId":"20160323224113.GB12531@sigill.intra.peff.net","threadId":"41752","inReplyTo":"CAJj6+1H6L=LxnDRzuC6OzXgVvzXsngGJ5X=E5Fi6Fg7JXkEJaQ@mail.gmail.com","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-23T22:41:13Z","receivedAt":"2016-03-23T22:41:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 20, 2016 at 12:19:46PM +0530, Chirayu Desai wrote:\n\n> I went for 3, and have sent a patch for that here - [PATCH/GSoC]\n> parse-options: Add a new nousage opt\n> However, it currently has one bug\n> Running 'git tag --contains qq' twice will first show an error, then\n> print qq, meaning that the first command creates the tag qq.\n> Running 'git tag -l --contains qq' works fine.\n> My first question is if 'git tag --contains' (without '-l') supposed to work?\n> If not, then I would fix that bug, otherwise fix the bug my code\n> introduced, and add tests for it.\n\nYes, \"--contains\" should imply \"-l\", and we should complain if there is\nan attempt to create a tag.\n\nThis seems to work with the tip of \"master\":\n\n $ git tag --contains v2.8.0-rc3\n v2.8.0-rc3\n v2.8.0-rc4\n\n $ git tag --contains qq\n error: malformed object name qq\n [...and then the usage...]\n\n $ git tag --contains HEAD qq\n fatal: --contains option is only allowed with -l.\n\n $ git rev-parse --verify qq\n fatal: Needed a single revision\n\nbut with your patch:\n\n $ git tag --contains qq\n error: malformed object name qq\n\n $ git rev-parse --verify qq\n e9cacb7f8231dd6616671f9bcdd0945043483064\n\nSo presumably we're not aborting the program when the options fail to\nparse, and it continues to process the \"qq\" as a tag to be created.\n\n-Peff\n"},{"id":"281690","messageId":"CAJj6+1Fvei-iv3QgP+P8o-0XqxOFVPL=DP6GOa1VKEx04VfGUg@mail.gmail.com","threadId":"41752","inReplyTo":"20160323224113.GB12531@sigill.intra.peff.net","subject":"Re: \"git tag --contains \" is too chatty, if is invalid","fromName":"Chirayu Desai","fromEmail":"chirayudesai1@gmail.com","sentAt":"2016-03-24T17:22:12Z","receivedAt":"2016-03-24T17:22:12Z","isPatch":false,"sender":{"key":"chirayudesai1@gmail.com","avatar":"https://gravatar.com/avatar/c2d0bd062b197c940eaaf3ac349f63fdbe0df58a2421c140ee8c9b4f52af95e3?d=mp&s=160"},"body":"On Thu, Mar 24, 2016 at 4:11 AM, Jeff King wrote:\n> On Sun, Mar 20, 2016 at 12:19:46PM +0530, Chirayu Desai wrote:\n>\n>> I went for 3, and have sent a patch for that here - [PATCH/GSoC]\n>> parse-options: Add a new nousage opt\n>> However, it currently has one bug\n>> Running 'git tag --contains qq' twice will first show an error, then\n>> print qq, meaning that the first command creates the tag qq.\n>> Running 'git tag -l --contains qq' works fine.\n>> My first question is if 'git tag --contains' (without '-l') supposed to work?\n>> If not, then I would fix that bug, otherwise fix the bug my code\n>> introduced, and add tests for it.\n>\n> Yes, \"--contains\" should imply \"-l\", and we should complain if there is\n> an attempt to create a tag.\nRight, makes sense.\n>\n> This seems to work with the tip of \"master\":\n>\n> $ git tag --contains v2.8.0-rc3\n> v2.8.0-rc3\n> v2.8.0-rc4\n>\n> $ git tag --contains qq\n> error: malformed object name qq\n> [...and then the usage...]\n>\n> $ git tag --contains HEAD qq\n> fatal: --contains option is only allowed with -l.\n>\n> $ git rev-parse --verify qq\n> fatal: Needed a single revision\n>\n> but with your patch:\n>\n> $ git tag --contains qq\n> error: malformed object name qq\n>\n> $ git rev-parse --verify qq\n> e9cacb7f8231dd6616671f9bcdd0945043483064\n>\n> So presumably we're not aborting the program when the options fail to\n> parse, and it continues to process the \"qq\" as a tag to be created.\nYep, it was because I was returning PARSE_OPT_DONE on case -3 in\nparse-options.c::parse_options_step\nMaking it return -1 fixed that.\n>\n> -Peff\n\nThank you for the help, detailed explanations and code reviews.\n"}]}