Re: [PATCH v2 1/4] config: factor out config file stack management
On Tue, Mar 12, 2013 at 06:52:00AM -0400, Jeff King wrote:
Show 36 quoted lines
> On Sun, Mar 10, 2013 at 05:57:53PM +0100, Heiko Voigt wrote:
>
> > Because a config callback may start parsing a new file, the
> > global context regarding the current config file is stored
> > as a stack. Currently we only need to manage that stack from
> > git_config_from_file. Let's factor it out to allow new
> > sources of config data.
> >
> > [...]
> >
> > +static int do_config_from(struct config_file *top, config_fn_t fn, void *data)
> > +{
> > + int ret;
> > +
> > + /* push config-file parsing state stack */
> > + top->prev = cf;
> > + top->linenr = 1;
> > + top->eof = 0;
> > + strbuf_init(&top->value, 1024);
> > + strbuf_init(&top->var, 1024);
> > + cf = top;
> > +
> > + ret = git_parse_file(fn, data);
> > +
> > + /* pop config-file parsing state stack */
> > + strbuf_release(&top->value);
> > + strbuf_release(&top->var);
> > + cf = top->prev;
> > +
> > + return ret;
> > +}
>
> Can we throw in a comment at the top here with the expected usage? In
> particular, do_config_from is expecting the caller to have filled in
> certain fields (at this point, top->f and top->name), but there is
> nothing to make that clear.Of course. Will do that in the next iteration. How about I squash this in:
diff --git a/config.c b/config.c
index b8c8640..b7632c9 100644
--- a/config.c
+++ b/config.c
@@ -948,6 +954,9 @@ int git_default_config(const char *var, const char *value, v
return 0;
}
+/* The fields data, name and the source specific callbacks of top need
+ * to be initialized before calling this function.
+ */
static int do_config_from_source(struct config_source *top, config_fn_t fn, voi
{
int ret;
I would add that to the third patch:
config: make parsing stack struct independent from actual data source
because that contains the final modification to config_file/config_source.
Cheers Heiko
Heiko Voigt <hvoigt@hvoigt.net>