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.
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.
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).
> This code is originally written by Jeff King [1]. All genius designs
> are his. All bugs are mine (claiming bugs is just more fun :).
Heh. I have written this code in a "something like this" form at least 3
times over the years. Conditional includes were always something I
planned into the original scheme, but had never actually found a good
use for it. ;)
> + /*
> + * It's OK to run over cond_len in our checks here, as that just pushes
> + * us past the final ".", which cannot match any of our prefixes.
> + */
> + if (skip_prefix(cond, "gitdir:", &value)) {
This would benefit from the skip_prefix_mem I proposed in:
http://article.gmane.org/gmane.comp.version-control.git/298050
(and which is ae989a61dad98debe9899823ca987305f8e8020d in Junio's tree,
though it is only in pu so far, I think).
That eliminates the need for the comment, and auto-update cond_len, so
that later:
> + strbuf_add(&pattern, value, cond_len - (value - cond));
...you do not have to do extra computation to get the correct length.
This is a tangent, but I wonder if expand_user_path() should take a
buf/len. It always puts the result into a new strbuf anyway, so it would
not be a big deal to do so. Skimming the output of grep, though, it
looks like this might be the only caller that would be helped.
> + buf = expand_user_path(pattern.buf);
> + if (buf) {
> + strbuf_reset(&pattern);
> + strbuf_addstr(&pattern, buf);
> + free(buf);
> + }
Maybe strbuf_attach() would be shorter here?
> + } 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.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html