Re: [PATCH v4] config: add conditional include

From: Duy Nguyen <pclouds@gmail.com>
Date: Jul 14, 2016, 16:13 UTC
Message-ID: <CACsJy8AjVX1Say0srEq+ezGg=CzmbjBAt4PnuikXiqdnVC4G6g@mail.gmail.com>
In-Reply-To: <alpine.DEB.2.20.1607141750190.6426@virtualbox>

On Thu, Jul 14, 2016 at 5:53 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 8 quoted lines

> Hi Duy,
>
> On Thu, 14 Jul 2016, Nguyễn Thái Ngọc Duy wrote:
>
>> Helped-by: Jeff King <peff@peff.com>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> This commit message is quite a bit short on details.

Because it's fully documented in config.txt.

> I still fail to see what use case this would benefit,

It allows you to group repos together. The first mail that started all this [1] is one of the use cases: you may want to use one identity in a group of repos, and another identity in some other. I want some more, to use a private key one some repos and another private key on some other. Some more settings may be shareable this way, like coding style-related (trailing spaces or not...)

[1] http://thread.gmane.org/gmane.comp.version-control.git/273811

> and I already start to suspect that the change would open a can of worms that might not be desired.

You can choose not to use it, I guess. From what I see it's nothing super tricky. The config hierarchy we have now is: system -> per-user -> repo. This could change this to: system -> per-user -> per-directory -> repo. You could create a different hierarchy, but with git-config now showing where a config var comes from, it's not hard to troubleshoot.

Show 7 quoted lines

>> +     ; include if $GIT_DIR is /path/to/foo/.git
>> +     [include "gitdir:/path/to/foo/.git"]
>> +             path = /path/to/foo.inc
>
> I find this way to specify a conditional unintuitive. Reading
> "gitdir:/path/to/foo/.git" suggests to me that the Git dir is *re-set*,
> not that this is a condition.

Well.. to me it's no different than [remote "foo"] to apply stuff to "foo".

Show 5 quoted lines

> I would have expected something like
>
>         [include "if-gitdir-is:/path/to/foo/.git"]
>
> instead.

If we do this, should we change gitdir/i to if-git-dir-case-insensitively-is ? I think we are supposed to read documents before using any feature, not just guessing then trial and error.

-- 
Duy