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

From: Jeff King <peff@peff.net>
Date: Mar 19, 2016, 17:57 UTC
Message-ID: <20160319175705.GA6989@sigill.intra.peff.net>
In-Reply-To: <CAJj6+1Fcp+Fjx9N6Mon1A5uP-_npnPL1Acu5-cR_bHVfs3EMWA@mail.gmail.com>

On Sat, Mar 19, 2016 at 10:19:02PM +0530, Chirayu Desai wrote:

Show 13 quoted lines

> > Yeah, I agree that showing the "-h" help is a bit much.
> > This is a side effect of looking up in the commit in the parse-options
> > callback. It has to signal an error to the option parser, and then the
> > option parser always shows the help on an error.
> > I think we'd need to do one of:
> > 1. call die() in the option-parsing callback (this is probably a bad
> > precedent, as the callbacks might be reused from a place that wants
> > to behave differently)
> I assume you mean parse-options-cb.c:parse_opt_commits() by the callback.
> I see that it is currently used only by commands which have a "--with"
> or "--contains" option,
> and all of them behave the same way, printing the full usage, so a one
> line change in that function would fix it for all of those.

Yes, that is the right callback.

Show 6 quoted lines

> > 2. have the callback just store the argument string, and then resolve
> > the commit later (and die or whatever if it doesn't exist). This
> > pushes more work onto the caller, but in this case it's all done by
> > the ref-filter code, so it could presumably happen during another
> > part of the ref-filter setup.
> I'm not quire sure how exactly to do that.

You'd teach parse_opt_commits() to store the string _name_ of the argument (e.g., using a string_list rather than a commit_list), and then later resolve those names into commits.

> > 3. teach parse-options to accept some specific non-zero return code
> > that means "return an error, but don't show the usage"
> This sounds good, but also the most intrusive of 3.

Yeah. Reading the options again, I kind of like this one. The only trick is that you would need to make sure no other callbacks are returning the value you choose for the "don't show the usage" flag. That is probably not too bad, though. There aren't that many callbacks, and they are not likely to be using values besides "-1" and "0".

-Peff