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:

On Sun, Mar 10, 2013 at 05:57:53PM +0100, Heiko Voigt wrote:

quoted

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