Re: [PATCH] config: add conditional include

[Duy Nguyen <pclouds@gmail.com>]

On Sun, Jun 26, 2016 at 8:27 PM, Jeff King <peff@peff.net> wrote:
> On Sun, Jun 26, 2016 at 09:06:17AM +0200, Nguyễn Thái Ngọc Duy wrote:
>
>> If the path argument in "include" starts with "gitdir:", it is
>> followed by a wildmatch pattern. The include is only effective if
>> $GIT_DIR matches the pattern. This is very useful to add configuration
>> to a group of repositories.
>
> I think this needs some more introduction to the concept. When you say
> "path argument" here, I assumed you meant the value of include.path. But
> you really mean: we are introducing a new concept for the "subsection"
> field of include.*, which is to provide restrictions for conditional
> includes.

Yep.

>
> It also may be worth discussing the motivation or examples.
>
>> For convenience
>>
>>  - "~" is expanded to $USER
>>
>>  - if the pattern ends with '/', "**" will be appended (e.g. foo/
>>    becomes foo/**). In other words, "foo/" automatically matches
>>    everything in starting with "foo/".
>>
>>  - if the pattern contains no slashes, it's wrapped around by "**/"
>>    and "/**" (e.g. "foo" becomes "**/foo/**"). In other words, "foo"
>>    matches any directory component in $GIT_DIR.
>>
>> The combination of the first two is used to group repositories by
>> path. While the last one could be used to match worktree's basename.
>
> This is a nice description, but it probably belongs in the
> documentation.

Yeah.. just too lazy for proper documentation at this stage.

>
> I don't have any real opinion on the rules themselves, though they seem
> reasonable to me (though in the first one I assume you mean $HOME).

Yep $HOME, what was I thinking...

(skipping all the technical suggestions, will do.. most likely)

>> +             } else if (!strchr(pattern.buf, '/')) {
>> +                     /* no slashes match one directory component */
>> +                     strbuf_insert(&pattern, 0, "**/", 3);
>> +                     strbuf_addstr(&pattern, "/**");
>> +             }
>
> I guess it's a little funny that "foo" and "foo/bar" are matched quite
> differently. I wonder if a simpler rule would just be: relative paths
> are unanchored.

I modeled it after .gitignore patterns, but that's probably not a good
fit here. Making all relative paths un-anchored means I can't say
"paths that end with this suffix". How useful that statement is, I
can't say though. Or if you mean only prepend "**/" to relative paths,
not "/**" then that door is still open.

(after a couple more minutes..) hmm.. I think that "paths that end
with ..." may have its use. The degenerated case is "match $(basename
<path>)", not very useful for gitdir when most of the time $(basename)
is ".git". It could be useful for "worktree:" matching and would
reduce false positives a bit (compared to un-anchoring both ends).
Conditionally including some config per worktree this way is also an
interesting way to deal with per-worktree config, but I need more time
to think about that...
-- 
Duy