Re: [PATCH 0/8] config-include fixes

From: Jeff King <peff@peff.net>
Date: Feb 17, 2012, 00:14 UTC
Message-ID: <20120217001438.GD4756@sigill.intra.peff.net>
In-Reply-To: <7v1upuzgfr.fsf@alter.siamese.dyndns.org>

On Thu, Feb 16, 2012 at 12:11:52PM -0800, Junio C Hamano wrote:

Show 10 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > I prepared this on top of what you have queued in jk/config-include.
> > However, all of the cleanup is semantically independent of the topic
> > (though there are a few minor textual conflicts). If I were re-rolling,
> > I would put it all at the front, then squash patch 8 into my prior
> > "implement config includes" patch.
> 
> Sorry for being late in answering the "revert or build on top" question; I
> was mostly offline yesterday afternoon.

No problem. I did it as build-on-top because it's much easier to squash and reorder the commits later than it is to pick a re-roll apart into multiple commits.

Which way did you want me to go with it?

Show 11 quoted lines

> Looking at the rebased result, it strikes me that with_options version
> 
>     Furthermore, by providing a more "advanced" interface, we
>     now have a a natural place to add new options for callers
>     like git-config, which care about tweaking the specifics of
>     config lookup, without disturbing the large number of
>     "simple" users (i.e., every other part of git).
> 
> perhaps wants to get a pointer to struct config_lookup_options, instead of
> us having to add a new parameter to all callsites every time a new need
> is discovered.

I considered that, but I noticed that the only callers who are really going to care about the _with_options version will be in builtin/config.c, and they all care about every option. And dealing with creating a struct for each call seemed like more hassle.

OTOH, I could probably just make a single static global config_lookup_options, and have all of the option parsing tweak it directly (i.e., replace the given_config_file and respect_includes static globals).

-Peff