On Fri, Jan 27, 2012 at 06:07:33AM +0100, Michael Haggerty wrote:
> > +struct git_config_include_data {
> > + config_fn_t fn;
> > + void *data;
> > +};
> > +int git_config_include(const char *name, const char *value, void *vdata);
> > +
> > extern const char *config_exclusive_filename;
> >
> > #define MAX_GITNAME (1000)
>
> How about a short comment or two?
I had originally planned to document this somewhat non-intuitive
interface in the config API documentation. But then I noticed we didn't
have such a document, and promptly forgot about documenting.
I'd rather have an API document, but I admit that the thought of
describing the config interface frightens me. It has some nasty corners.
But maybe starting one with the non-scary bits would be better, and then
I could add this to it.
> > +int git_config_include(const char *name, const char *value, void *vdata)
> > +{
> > + const struct git_config_include_data *data = vdata;
> > + const char *type;
> > + int ret;
> > +
> > + /*
> > + * Pass along all values, including "include" directives; this makes it
> > + * possible to query information on the includes themselves.
> > + */
> > + ret = data->fn(name, value, data->data);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (prefixcmp(name, "include."))
> > + return ret;
> > + type = strrchr(name, '.') + 1;
> > +
> > + if (!strcmp(type, "path"))
> > + ret = handle_path_include(value, vdata);
> > +
> > + return ret;
> > +}
> > +
>
> Doesn't this code accept all keys of the form "include\.(.*\.)?path"
> (e.g., "include.foo.path")? If that is your intention, then the
> documentation should be fixed. If not, then a single strcmp(name,
> "include.path") would seem sufficient.
It does. I was considering (but haven't yet written) a patch that would
allow for conditional inclusion, like:
[include "foo"]
path = /some/file
where "foo" would be the condition. Specifically, I wanted to enable
includes when certain features were available in the parsing version of
git. For example, the pager.* variables were originally bools, but later
learned to take arbitrary strings. So my config with arbitrary strings
works on modern git, but causes earlier versions of git to barf. I'd
like to be able to do something like:
[include "per-command-pager-strings"]
path = /path/to/my/pager.config
where "per-command-pager-strings" would be a flag known internally to
git versions that support that feature.
I didn't end up implementing it right away, because of course those same
early versions of git also don't know about "include" at all. So using
any include effectively works as a conditional for that particular
feature. But as new incompatible config semantics are added
post-include, they could take advantage of a similar scheme.
So I wanted to leave the code open to adding such a patch later, if and
when it becomes useful. That being said, the code above is wrong.
For my scheme to work, versions of git that handle includes but don't
have the conditional-include patch (if it ever comes) would want to
explicitly disallow includes with subsections.
I'll fix it in the re-roll.
> > + struct git_config_include_data inc;
> > +
> > + inc.fn = fn;
> > + inc.data = data;
> > + fn = git_config_include;
> > + data = &inc;
> >
> > /* Setting $GIT_CONFIG makes git read _only_ the given config file. */
> > if (config_exclusive_filename)
>
> The comment just after your addition should be adjusted, since now "the
> given config file and any files that it includes" are read.
Will do.
-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