Re: [PATCH] bisect run: allow '--' as an options terminator

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 7, 2020, 01:34 UTC
Message-ID: <xmqqtv586oci.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAD_xR9faremhsGP4G65vHj=cvskUbK1rpPrQisa2_4AD2AGNYw@mail.gmail.com>

Stephen Oberholtzer <stevie@qrpff.net> writes:

Show 5 quoted lines

>> I do not think I have seen enough to justify addition of "--",
>
> That's fine; I was just trying to be thorough (also, it was easy to
> test.) I was taught: if you accept any -options, honor -- as well. If
> you're not concerned about that, that's fine with me.

Ahh, that indeed is the crucial piece of information that was missing. My review was about "Do we really *need* it?", but if you are doing so from following a rule/dogma/principle, that changes the equation quite a bit.

I do not think this project officially subscribes to the "anywhere -option is taken should accept '--' as the end of options marker" school, but because most modern command line processors use parse_options() API which gets "--" for free, we can consider ourselves practically accepting it already.

And adopting such a rule/dogma/principle frees us from having to think about each individual case and helps us being consistent, so it is not necessarily a bad thing as long as the cost of following the rule/dogma/principle is not too onerous. At that point, what needs to be reviewed instead becomes to "does this new code follow the rule, and is it not bending backwards to do so?"

So, I do not have strong objection against "bisect run -- <cmd>". It was, as I said in the original review, that it was so unclear why double-dash is a logical consequence of accepting options, because it was left unsaid. It would have been an easy sell if this were a part of a patch that actually adds new option(s) and explained perhaps like so:

	Teach 'bisect run' to take --foo and --bar options, that
	tell the command to do Foo and Bar.  While at it, teach it
	the common convention that "--" is used to terminate the
	list of options.  Nobody sane may try to use <cmd> that
	begins with a dash, but supporting "--" everywhere we accept
	a dashed option is good for consistency.

Thanks.