From: Duy Nguyen <pclouds@gmail.com>
Date: Wed, 29 Jun 2016 04:06:44 GMT
Subject: Re: [PATCH v2 2/2] config: add conditional include
Message-ID: <CACsJy8BQu6vcxdt0tESjBJVnbmvmW8oRNkLfMA157s1bE-S_eA@mail.gmail.com>
In-Reply-To: <20160628204959.GB21002@sigill.intra.peff.net>

On Tue, Jun 28, 2016 at 10:49 PM, Jeff King <peff@peff.net> wrote:
>> +static int prepare_include_condition_pattern(struct strbuf *pat)
>> +{
>> +     struct strbuf path = STRBUF_INIT;
>> +     int prefix = 0;
>> +
>> +     /* TODO: maybe support ~user/ too */
>> +     if (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {
>> +             const char *home = getenv("HOME");
>> +
>> +             if (!home)
>> +                     return error(_("$HOME is not defined"));
>> +
>> +             strbuf_add_absolute_path(&path, home);
>> +             strbuf_splice(pat, 0, 1, path.buf, path.len);
>> +             prefix = path.len + 1 /*slash*/;
>
> Why did you drop expand_user_path() here?
>
> I guess it's to do with knowing the length of the prefix portion? I'm
> not sure I understand why the prefix is necessary. I thought the goal
> was to construct a wildmatch pattern that could be used against
> get_git_dir().

We need to make sure any '*', '?' and '[' in the $HOME (or `pwd`)
portion are not automatically promoted to wildcards. One option is
split the pattern in two parts, the prefix part is matched literally
then the rest passed to wildmatch(). The other option is escape
$HOME/`pwd`, but I think that's more complicated (or at least slower).
-- 
Duy