Re: [GSoC][PATCH v4] Make options that expect object ids less chatty if id is invalid

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 6, 2018, 21:16 UTC
Message-ID: <xmqqlgf43nxo.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180306193116.23876-1-ungureanupaulsebastian@gmail.com>

Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> writes:

Show 10 quoted lines

> Usually, the usage should be shown only if the user does not know what
> options are available. If the user specifies an invalid value, the user
> is already aware of the available options. In this case, there is no
> point in displaying the usage anymore.
>
> This patch applies to "git tag --contains", "git branch --contains",
> "git branch --points-at", "git for-each-ref --contains" and many more.
>
> Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
> ---

I notice that this version changes the way the case where an unbiguous long option is given, compared to the previous one is handled. And I recall that that single case is what I happened to have noticed during my review of the previous one, in which I was not trying to be exhausitive.

I kind of find it surprising that the one single case I happened to have noticed is the only one that needed special treatment. Did you go though all the codepath and made sure that the ones that still return -1 (not -2 and not -3) to parse_options_step() are all OK (in other words, I was just lucky) or does this version change only the "ambiguous" case, simply because that was the only one I noticed in my review (in other words, this may still not be sufficient)?

Just double checking.

The changes to existing tests have become a lot less noisy, compared to the previous one, which is probably a good thing.

Thanks.