On Fri, Feb 24, 2017 at 2:59 AM, Junio C Hamano <[email protected]> wrote:
> The variable is obviously not treated the same way as include.path ;-)
>
> When includeIf.<condition>.path variable is set in a
> configuration file, the configuration file named by that
> variable is included (in a way similar to how include.path
> works) only if the <condition> holds true.
Yeah. I was thinking "value" and writing "variable" instead (or
perhaps I meant to write "variable value" and accidentally'd the last
word again).
>> + /* 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"));
>
> Instead of half-duplicating it here yourself, can't we let
> expand_user_path() do its thing?
This comment came up in previous iterations. I perform explicit
expansion to make sure we don't apply wildcards on the expanded "~".
But I guess going with expand_user_path() is better (less confusion
for future readers). We can come back to it when that "don't apply
wildcards" concern becomes real.
>> +static int include_condition_is_true(const char *cond, size_t cond_len)
>> +{
>> + /* no condition (i.e., "include.path") is always true */
>> + if (!cond)
>> + return 1;
>> +
>> + if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len))
>> + return include_by_gitdir(cond, cond_len, 0);
>> + else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond,
>> &cond_len))
>> + return include_by_gitdir(cond, cond_len, 1);
>
> This may be OK for now, but it should be trivial to start from a
> table with two entries, i.e.
>
> struct include_cond {
> const char *keyword;
> int (*fn)(const char *, size_t);
> };
>
> and will show a better way to do things to those who follow your
> footsteps.
Yeah I don't see a third include coming soon and did not go with that.
Let's way for it and refactor then.
--
Duy
