From: Jonathan Tan <jonathantanmy@google.com>
Date: Thu, 02 Dec 2021 23:14:03 GMT
Subject: Re: [PATCH v3 2/2] config: include file if remote URL matches a glob
Message-ID: <20211202231403.3451471-1-jonathantanmy@google.com>
In-Reply-To: <xmqqlf1417mo.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:
> Jonathan Tan <jonathantanmy@google.com> writes:
> 
> >> variables we read without the includeIf directive", with variations
> >> of "condition" including
> >> 
> >>  - a literal X is among the values of multi-valued variable Y.
> >>  - a pattern X matches one of the values of multi-valued variable Y.
> >>  - a literal Y is the name of an existing configuration variable.
> >>  - a pattern Y matches the name of an existing configuration variable.
> > ...
> > code), in this case, unless there is another use case for this, I think
> > we should proceed with the use case that we know about first
> > (conditional include of a file supplied by a remote repo administrator).
> 
> Doing it that way without thinking flexibility through will paint us
> into a corner, from which we cannot get out of, doesn't it?
> 
> People will start asking "Why should we even have
> 'hasremoteurl:$URL' variant in 'includeIf' conditions, when one of
> the 'variableExists:Y' and friends can express the same thing",
> somebody new who is not yet in this community today will propose
> deprecating hasremoteurl in favor of more generalized approach and
> we have to give a sad answer "no, we earlier made a mistake of
> starting with a special case variant for expediency's sake, without
> thinking the general cases through.  Because existing users depend
> on it, we have to support it til the end of time."
> 
> We have the same regret with "why do we need grep.extendedRegexp
> when grep.patternType suffices?"  I am reluctant to see us knowingly
> commit the same mistake here, unless there is a very good reason.

Hmm...that's true. I was thinking that there wouldn't be a way to
predict exactly what we'll need, but perhaps making the config variable
name be of the form 'includeIf."hasconfig:remote.*.url".path' might give
us enough flexibility for the future.. I'll take a look.