On Thu, Mar 24, 2016 at 4:11 AM, Jeff King <peff@xxxxxxxx> wrote: > On Sun, Mar 20, 2016 at 12:19:46PM +0530, Chirayu Desai wrote: > >> I went for 3, and have sent a patch for that here - [PATCH/GSoC] >> parse-options: Add a new nousage opt >> However, it currently has one bug >> Running 'git tag --contains qq' twice will first show an error, then >> print qq, meaning that the first command creates the tag qq. >> Running 'git tag -l --contains qq' works fine. >> My first question is if 'git tag --contains' (without '-l') supposed to work? >> If not, then I would fix that bug, otherwise fix the bug my code >> introduced, and add tests for it. > > Yes, "--contains" should imply "-l", and we should complain if there is > an attempt to create a tag. Right, makes sense. > > This seems to work with the tip of "master": > > $ git tag --contains v2.8.0-rc3 > v2.8.0-rc3 > v2.8.0-rc4 > > $ git tag --contains qq > error: malformed object name qq > [...and then the usage...] > > $ git tag --contains HEAD qq > fatal: --contains option is only allowed with -l. > > $ git rev-parse --verify qq > fatal: Needed a single revision > > but with your patch: > > $ git tag --contains qq > error: malformed object name qq > > $ git rev-parse --verify qq > e9cacb7f8231dd6616671f9bcdd0945043483064 > > So presumably we're not aborting the program when the options fail to > parse, and it continues to process the "qq" as a tag to be created. Yep, it was because I was returning PARSE_OPT_DONE on case -3 in parse-options.c::parse_options_step Making it return -1 fixed that. > > -Peff Thank you for the help, detailed explanations and code reviews. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html