On Fri, Feb 24, 2017 at 2:59 AM, Junio C Hamano <gits...@pobox.com> 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