git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/8] config-include fixes
Date: Thu, 16 Feb 2012 19:14:38 -0500	[thread overview]
Message-ID: <20120217001438.GD4756@sigill.intra.peff.net> (raw)
In-Reply-To: <7v1upuzgfr.fsf@alter.siamese.dyndns.org>

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

> 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?

> 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

  reply	other threads:[~2012-02-17  0:14 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-13 20:42 What's cooking in git.git (Feb 2012, #05; Mon, 13) Junio C Hamano
2012-02-14  7:22 ` An incremental update to "What's cooking" Junio C Hamano
2012-02-14 21:47 ` What's cooking in git.git (Feb 2012, #05; Mon, 13) Jeff King
2012-02-14 22:05   ` Junio C Hamano
2012-02-14 22:09     ` Jeff King
2012-02-16  8:01       ` [PATCH 0/8] config-include fixes Jeff King
2012-02-16  8:02         ` [PATCH 1/8] t1300: add missing &&-chaining Jeff King
2012-02-16  8:03         ` [PATCH 2/8] config: copy the return value of prefix_filename Jeff King
2012-02-16  8:04         ` [PATCH 3/8] config: teach git_config_set_multivar_in_file a default path Jeff King
2012-02-16  8:04         ` [PATCH 4/8] config: teach git_config_rename_section a file argument Jeff King
2012-02-16  8:05         ` [PATCH 5/8] config: provide a version of git_config with more options Jeff King
2012-02-16  8:07         ` [PATCH 6/8] config: stop using config_exclusive_filename Jeff King
2012-02-16  8:09         ` [PATCH 7/8] config: eliminate config_exclusive_filename Jeff King
2012-02-16  8:10         ` [PATCH 8/8] config: do not respect includes for single-file --list Jeff King
2012-02-16 20:11         ` [PATCH 0/8] config-include fixes Junio C Hamano
2012-02-17  0:14           ` Jeff King [this message]
2012-02-17  2:50             ` Junio C Hamano
2012-02-17  3:17               ` Jeff King
2012-02-17  3:23                 ` Jeff King
2012-02-17  8:17                   ` [PATCH 0/2] api-config documentation leftovers Jeff King
2012-02-17 17:04                     ` Junio C Hamano
2012-02-17  8:18                   ` [PATCH 1/2] docs/api-config: minor clarifications Jeff King
2012-02-17  8:18                   ` [PATCH 2/2] docs/api-config: describe git_config_with_options Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120217001438.GD4756@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).