Re: [PATCH 1/4] config: factor out config file stack management

2013-02-26 Thread Jeff King
On Tue, Feb 26, 2013 at 08:38:50PM +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

Re: [PATCH 1/4] config: factor out config file stack management

2013-02-26 Thread Heiko Voigt
On Tue, Feb 26, 2013 at 02:54:49PM -0500, Jeff King wrote: On Tue, Feb 26, 2013 at 08:38:50PM +0100, Heiko Voigt wrote: +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 =

Re: [PATCH 1/4] config: factor out config file stack management

2013-02-26 Thread Jeff King
On Tue, Feb 26, 2013 at 09:09:41PM +0100, Heiko Voigt wrote: This function name is a bit weird. I would have thought the from here was going to be a file, or a string, or whatever. But the filename setup happens outside this function (and yet this function depends on it being set up, as

Re: [PATCH 1/4] config: factor out config file stack management

2013-02-26 Thread Junio C Hamano
Jeff King p...@peff.net writes: I wonder if it would be more obvious with the more usual OO-struct functions, like: struct config_source { ... }; void config_source_init_file(struct config_source *, const char *fn); void config_source_init_strbuf(struct config_source *,

Re: [PATCH 1/4] config: factor out config file stack management

2013-02-26 Thread Junio C Hamano
Heiko Voigt hvo...@hvoigt.net writes: The do_config_from means parse from whatever is in 'top'. Later in the series its type changes from config_file to struct config. Yuck. It would be nice to have it as struct config_src or something. struct config sounds as if it represents the entire

Re: [PATCH 1/4] config: factor out config file stack management

2013-02-26 Thread Heiko Voigt
On Tue, Feb 26, 2013 at 02:10:03PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: I wonder if it would be more obvious with the more usual OO-struct functions, like: struct config_source { ... }; void config_source_init_file(struct config_source *,

Re: [PATCH 1/4] config: factor out config file stack management

2013-02-26 Thread Heiko Voigt
On Tue, Feb 26, 2013 at 02:12:23PM -0800, Junio C Hamano wrote: Heiko Voigt hvo...@hvoigt.net writes: The do_config_from means parse from whatever is in 'top'. Later in the series its type changes from config_file to struct config. Yuck. It would be nice to have it as struct config_src