Re: [PATCH 4/8] submodule-config: introduce parse_generic_submodule_config

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 4, 2016, 00:51 UTC
Message-ID: <xmqqio25jo1n.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kZwGPNHeZFcu7-60U8cTZhC8t6TZ5=4ZzBbP+4MrLqRZQ@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 14 quoted lines

>> The same comment as in the footnote for the review on [3/8] applies
>> here.
>
> Ok, so we want to have
>
>     [section ""]
>
> and
>
>     [section]
>
> be different, such that we could have a submodule with an "" name,
> but still be able to differentiate to the common section which should be
> applied to all submodules?

It is not about submodule per-se, but what is the right way to use the parse_config_key() function. If the caller is interested in seeing if the name is two-level or three-level, NULL-ness of the subsection pointer is the right thing to check, not the length of the subsection (set to 0 or positive number if exists--and otherwise it is set to 0).

People copy & paste existing uses of API functions, and even if for the specific use of your code (i.e. submodule) it is illegal to have an empty second level, we shouldn't leaving a bad practice in the code to be imitated.