Re: js/bisect-in-c, was Re: What's cooking in git.git (Jul 2022, #03; Mon, 11)

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 14, 2022, 21:09 UTC
Message-ID: <220714.86mtdb1jmp.gmgdl@evledraar.gmail.com>
In-Reply-To: <p8srrprq-s23s-711n-n452-34qr856qso29@tzk.qr>

On Thu, Jul 14 2022, Johannes Schindelin wrote:

Show 27 quoted lines

> Hi Junio,
>
> On Wed, 13 Jul 2022, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>> > I'm not claiming that we always use 129 when we're fed bad options etc.,
>> > but rather that that's what parse_options() does, so at this point most
>> > commands do that consistently.
>> >
>> > 	./git --blah >/dev/null 2>&1; echo $?
>> > 	129
>> > 	./git status --blah >/dev/null 2>&1; echo $?
>> > 	129
>> >
>> > But yes, you can find exceptions still, e.g. try that with "git log" and
>> > it'll return 128.
>>
>> Yup, that was my understanding as well.  We may have existing
>> breakage that we shouldn't be actively imitating when we do not have
>> to.
>
> This patch series already implements `git bisect` in the desired way:
>
> 	$ ./git bisect --invalid; echo $?
> 	usage: git bisect [help|start|bad|good|new|old|terms|skip|next|reset|visualize|view|replay|log|run]
> 	129

It doesn't, the claim isn't that there's no way to have it return exit code 129 on *some* invalid usage. In this case we do the "right" thing.

Rather that as noted in [1] there's other cases where we call die() and should call usage_msg_opt().

Of course both of those would be more useful if the new built-in still had the parse_options() usage info we could display. I.e. in this case the conversion to a built-in leaves on the table nice benefits we'd get for free.

Compare that with e.g. how "git bundle" handles it, note how we get not
only "don't do that", but also how valid usage would look like:
	
	$ git bundle -h
	usage: git bundle create [-q | --quiet | --progress | --all-progress] [--all-progress-implied]
	                         [--version=<version>] <file> <git-rev-list-args>
	   or: git bundle verify [-q | --quiet] <file>
	   or: git bundle fetch [--filter=<spec>] <uri>
	   or: git bundle list-heads <file> [<refname>...]
	   or: git bundle unbundle [--progress] <file> [<refname>...]
	$ git bundle create -h
	usage: git bundle create [-q | --quiet | --progress | --all-progress] [--all-progress-implied]
	                         [--version=<version>] <file> <git-rev-list-args>
	
	    -q, --quiet           do not show progress meter
	    --progress            show progress meter
	    --all-progress        show progress meter during object writing phase
	    --all-progress-implied
	                          similar to --all-progress when progress meter is shown
	    --version <n>         specify bundle format version

1. https://lore.kernel.org/git/220627.86ilolhnnn.gmgdl@evledraar.gmail.com/