Re: "git tag --contains <id>" is too chatty, if <id> is invalid

From: Chirayu Desai <chirayudesai1@gmail.com>
Date: Mar 24, 2016, 17:22 UTC
Message-ID: <CAJj6+1Fvei-iv3QgP+P8o-0XqxOFVPL=DP6GOa1VKEx04VfGUg@mail.gmail.com>
In-Reply-To: <20160323224113.GB12531@sigill.intra.peff.net>

On Thu, Mar 24, 2016 at 4:11 AM, Jeff King <peff@peff.net> wrote:

Show 14 quoted lines

> 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.

Show 27 quoted lines

>
> 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.