public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Chirayu Desai <chirayudesai1@gmail•com>, git@vger•kernel.org
Subject: Re: "git tag --contains <id>" is too chatty, if <id> is invalid
Date: Sun, 20 Mar 2016 15:25:04 -0700	[thread overview]
Message-ID: <xmqqoaa8okhr.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160319175705.GA6989@sigill.intra.peff.net> (Jeff King's message of "Sat, 19 Mar 2016 13:57:05 -0400")

Jeff King <peff@peff•net> writes:

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

My knee-jerk preference among the three is 2., but I think I'll know
when I see a patch ;-)

Thanks for helping the candidates.

next prev parent reply	other threads:[~2016-03-20 22:25 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-19 16:49 "git tag --contains <id>" is too chatty, if <id> is invalid Chirayu Desai
2016-03-19 17:04 ` Pranit Bauva
2016-03-19 17:51   ` Chirayu Desai
2016-03-19 17:57 ` Jeff King
2016-03-19 18:08   ` Chirayu Desai
2016-03-19 18:12     ` Jeff King
2016-03-20  6:49       ` Chirayu Desai
2016-03-23 22:41         ` Jeff King
2016-03-24 17:22           ` Chirayu Desai
2016-03-20 22:25   ` Junio C Hamano [this message]
  -- strict thread matches above, loose matches on Subject: below --
2016-01-18 21:24 Toralf Förster
2016-01-18 21:54 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqoaa8okhr.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=chirayudesai1@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox