Hi, On Sun, Feb 24, 2013 at 09:54:43PM -0800, Junio C Hamano wrote: > The idea to allow more kinds of sources specified for "config_file" > structure is not bad per-se, but whenever you design an enhancement > to something that currently supports only on thing to allow taking > another kind, please consider what needs to be done by the next > person who adds the third kind. That would help catch design > mistakes early. For example, will the "string-list" (I am not > saying use of string-list makes sense as the third kind; just as an > example off the top of my head) source patch add > > int is_string_list; > struct string_list *string_list_contents; > > fields to this structure? Sounds insane for at least two reasons: > > * if both is_strbuf and is_string_list are true, what should > happen? > > * is there a good reason to waste storage for the three fields your > patch adds when sring_list strage (or FILE * storage for that > matter) is used? > > The helper functions like config_fgetc() and config_ftell() sounds > like you are going in the right direction but may want to do the > OO-in-C in a similar way transport.c does, keeping a pointer to a > structure of methods, but I didn't read the remainder of this patch > very carefully enough to comment further. I split up my config-from-strings patch from the "fetch moved submodules on-demand" series[1] for nicer reviewability. Since Peff almost needed it and the recursive submodule checkout will definitely[2] need it: How about making it a seperate topic and implement this infrastructure first? Junio I incorporated your comments this seems like a result ready for inclusion. [1] http://article.gmane.org/gmane.comp.version-control.git/217018 [2] http://article.gmane.org/gmane.comp.version-control.git/217155 Heiko Voigt (4): config: factor out config file stack management config: drop file pointer validity check in get_next_char() config: make parsing stack struct independent from actual data source teach config parsing to read from strbuf .gitignore | 1 + Makefile | 1 + cache.h | 1 + config.c | 140 ++++++++++++++++++++++++++++++++++++++----------- t/t1300-repo-config.sh | 4 ++ test-config.c | 41 +++++++++++++++ 6 files changed, 158 insertions(+), 30 deletions(-) create mode 100644 test-config.c -- 1.8.2.rc0.26.gf7384c5 -- 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