Re: [PATCH v7 3/3] config: add conditional include
- From
Jeff King <peff@peff.net>
- Date
- Mar 3, 2017, 22:24 UTC
- Message-ID
- <20170303222456.ouujfjwusk4l45n4@sigill.intra.peff.net>
- In-Reply-To
- <xmqq4lza9o5r.fsf@gitster.mtv.corp.google.com>
On Fri, Mar 03, 2017 at 11:05:20AM -0800, Junio C Hamano wrote:
Show 13 quoted lines
> I am not sure about "obviousness", but I agree that we do not know > that "conditional include" would be the only thing we want for the > second level for "include.path" directive. "include-if.<cond>.path" > is better for that reason. > > I presume that you could still do > > [include "if:gitdir=$path"] > path = ... > > i.e. design the second level to begin with a token that tells > readers what it means (and assign "if:" token for "conditional > include"), but I do not think it is worth it.
Yep, all true.
Show 9 quoted lines
> I also imagine that > > [include] > condition = ... > path = ... > > is easier to read and write by end-users, but it probably is not > feasible because it requires too invasive change to the current code > to teach it to grok such construct.
I am against that as it introduces a dependency in the presence and ordering between two config variables, which can yield some surprises.
Show 7 quoted lines
> Between "include-if" and "includeIf", if people find the latter not > too ugly, I'd prefer to keep it the way Duy posted. Because of the > way "include.path" and "include-if.<cond>.path" work, we can declare > that they are not like ordinary configuration variable definition > at all but are higher-level directives and that may be a sufficient > justification to allow "-" in its name, though, if people find > "includeIf" too ugly a name to live.
OK. I can live with includeIf.
-Peff