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

[Jeff King <peff@peff.net>]

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

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

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