Re: [PATCH 9/9] git_config_set: reuse empty sections

From: Jeff King <peff@peff.net>
Date: Mar 29, 2018, 21:50 UTC
Message-ID: <20180329215038.GI2939@sigill.intra.peff.net>
In-Reply-To: <29868fec2606f5f6aa1c5de2928580f2ca24e6e2.1522336130.git.johannes.schindelin@gmx.de>

On Thu, Mar 29, 2018 at 05:19:09PM +0200, Johannes Schindelin wrote:

Show 13 quoted lines

> It can happen quite easily that the last setting in a config section is
> removed, and to avoid confusion when there are comments in the config
> about that section, we keep a lone section header, i.e. an empty
> section.
> 
> The code to add new entries in the config tries to be cute by reusing
> the parsing code that is used to retrieve config settings, but that
> poses the problem that the latter use case does *not* care about empty
> sections, therefore even the former user case won't see them.
> 
> Fix this by introducing a mode where the parser reports also empty
> sections (with a trailing '.' as tell-tale), and then using that when
> adding new config entries.

Heh, so it seems we are partway to the "event-stream" suggestion I made earlier. I agree this is the right way to approach this problem.

I wondered if we allow keys to end in ".", but it seems that we don't.

Show 11 quoted lines

> diff --git a/config.c b/config.c
> index eb1e0d335fc..b04c40f76bc 100644
> --- a/config.c
> +++ b/config.c
> @@ -653,13 +653,15 @@ static int get_base_var(struct strbuf *name)
>  	}
>  }
>  
> -static int git_parse_source(config_fn_t fn, void *data)
> +static int git_parse_source(config_fn_t fn, void *data,
> +			    int include_section_headers)

We already have a "struct config_options", but we do a terrible job of passing it around (since it only impacts the include stuff right now, and that all gets handled at a very outer level).

Rather than plumb this one int through everywhere, should we add it to that struct and plumb the struct through?

-Peff