# Re: \[PATCH v4 0/2\] Conditional config includes based on remote URL

- From: Junio C Hamano \<hidden\>
- Date: 2021-11-30 07:51:51 
- List: git
- Page: https://ratatoskr.run/git/2021/11/7913471
- Thread: https://ratatoskr.run/git/2021/11/7913471/t.md

```text
Ævar Arnfjörð Bjarmason [ref] writes:

> On Mon, Nov 29 2021, Jonathan Tan wrote:
>
>> Thanks everyone for your comments. Here's an update.
>
> Just from skimming this (minor) feedback on v3 still applies:
> https://lore.kernel.org/git/211123.86pmqrwtf2.gmgdl@evledraar.gmail.com/
>
> I.e. s/hasremoteurl/hasRemoteURL/ etc. in appropriate places.

Is there any appropriate place, though?

"hasremoteurl" is a new directive to be used as the leading part of
<condition> in the name of `includeIf.<condition>.path` variable.
The <condtion> part is case sensitive, and we do not want people to
spell it, and the existing "gitdir", "gitdir/i", and "onbranch", in
mixed cases.

See config.c::include_condition_is_true() function and its use of
skip_prefix_mem() to locate these existing conditions.

It is troubling that this patch is *NOT* extend the implementation
of include_condition_is_true() function (which gives a very clean
abstraction and makes the caller very readable); it instead mucks
with the caller of include_condition_is_true() and adds a parallel
logic that include_condition_is_true() does not know about.  It may
have been an expedite way to implement this, and the result may not
seem to hurt when include_condition_is_true() is called by only one
caller, but I find the resulting code structure unnecessarily ugly.

Can't the body of if (skip_prefix_mem(..."hasremoteurl:", ...)) block
become include_by_remoteurl() function, similar to include_by_foo()
functions include_condition_is_true() already calls?
```