From: Junio C Hamano Date: Tue, 06 Mar 2018 21:16:03 GMT Subject: Re: [GSoC][PATCH v4] Make options that expect object ids less chatty if id is invalid Message-ID: In-Reply-To: <20180306193116.23876-1-ungureanupaulsebastian@gmail.com> Paul-Sebastian Ungureanu writes: > 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 > --- 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.