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

Junio I incorporated your comments this seems like a result ready for

[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

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to