Thread (16 messages) flat view 16 messages, 4 authors, 2016-06-15

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

From: Jeff King <hidden>
Date: 2016-06-15 22:56:16

Possibly related (same subject, not in this thread)

On Tue, Feb 26, 2013 at 09:09:41PM +0100, Heiko Voigt wrote:
quoted
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 it calls git_parse_file). But maybe it will get less
confusing with the other patches on top...
The "do_config_from" means "parse from whatever is in 'top'". Later in
the series its type changes from config_file to struct config.
Ah, I see. The "from" is the "struct config".

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 *,
                                 const struct strbuf *buf);
  void config_source_clear(struct config_source *);

  int config_source_parse(struct config_source *);

and then the use would be something like:

  struct config_source top;
  int ret;

  config_source_init_file(&top, "foo");
  ret = config_source_parse(&top);
  config_source_clear(&top);

  return ret;

I.e., "init" constructors, a "clear" destructor, and any methods like
"parse" that you need.  I haven't though too hard about it, though, so
maybe there is some reason it does not fit that model (it is a little
uncommon that the "init" would push itself onto a stack, but I think
that's OK).

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help