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

From: Junio C Hamano <gitster@pobox.com>
To: Stephen Oberholtzer <stevie@qrpff.net>
Cc: git@vger.kernel.org, Christian Couder <chriscool@tuxfamily.org>,
	Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [PATCH] bisect run: allow '--' as an options terminator
Date: Mon, 06 Jan 2020 17:34:53 -0800	[thread overview]
Message-ID: <xmqqtv586oci.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <CAD_xR9faremhsGP4G65vHj=cvskUbK1rpPrQisa2_4AD2AGNYw@mail.gmail.com> (Stephen Oberholtzer's message of "Mon, 6 Jan 2020 18:29:35 -0500")

Stephen Oberholtzer <stevie@qrpff.net> writes:

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