Re: [PATCH 4/6] config.c: plumb the_reader through callbacks

From: Glen Choo <chooglen@google.com>
Date: Mar 8, 2023, 18:00 UTC
Message-ID: <kl6lttyvgm24.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <230308.86356fzhbd.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> So, there's a lot of churn in this topic from renaming "cf" to "cs" all
> over the place, I really wish that could be avoided for the size of the
> overall diff, but haven't looked into in detail how easy that is, but...

Yeah, I think it can be avoided by taking Junio's suggestion to rename the global "cf" to "cf_global" or something, then accepting a "cf" parameter.

  https://lore.kernel.org/git/xmqqh6v1g1d3.fsf@gitster.g

Show 8 quoted lines

>> -static int configset_add_value(struct config_set *cs, const char *key, const char *value)
>> +static int configset_add_value(struct config_reader *reader,
>> +			       struct config_set *cs, const char *key,
>> +			       const char *value)
>
> ...this is an existing name just seen in the context, but could we in
> this topic at least avoid having a "cs" refer to both a "struct
> config_set" and a "struct config_source" in the end-state?

Fair. This also irked me when I first saw it.

As an aside, I found "struct config_source cf" to be a bit of an eyesore too. It's an unfortunate holdover from before 4d8dd1494e (config: make parsing stack struct independent from actual data source, 2013-07-12) when the struct was called "config_file". Unfortunately, "cs" is already taken by at least one parameter, so maybe we'll never be able to properly rename it...