On Sat, Mar 19, 2016 at 11:27 PM, Jeff King <peff@xxxxxxxx> wrote: > 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. Gotcha, will need to figure out where exactly would those names be resolved, can do after following the code path a bit more, can do. > >> > 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". I'll try to go through that as well. > > -Peff -Chirayu -- 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