From: Stefan Beller <sbeller@google.com>
Date: Fri, 30 Oct 2015 18:37:18 GMT
Subject: Re: [PATCHv2 3/8] submodule config: remove name_and_item_from_var
Message-ID: <CAGZ79kZekEa5CwaisXNvpXLxfEp9zdDQKYULEhwiikwj1AyiFA@mail.gmail.com>
In-Reply-To: <CAPig+cSpnGE5Acgvd+b0arcFx8oStuRRKR4fcSwZG2fbEZ6wSQ@mail.gmail.com>

On Thu, Oct 29, 2015 at 6:23 PM, Eric Sunshine <ericsunshine@gmail.com> wrote:
> On Wed, Oct 28, 2015 at 7:21 PM, Stefan Beller <sbeller@google.com> wrote:
>> submodule config: remove name_and_item_from_var
>>
>> By inlining `name_and_item_from_var` it is easy to add later options
>> which are not required to have a submodule name.
>
> I guess you're trying to say that name_and_item_from_var() didn't
> provide a proper abstraction, thus wasn't as useful as expected.
> Perhaps that commit message could make this shortcoming clearer.
>

ok

>
>     if (parse_config_key(var, "submodule", &subsection,
>             &subsection_len, &key) < 0 || !subsection_len)
>         return 0;

done

>>         submodule = lookup_or_create_by_name(me->cache,
>>                                              me->gitmodules_sha1,
>> -                                            name.buf);
>> +                                            name);
>> +       free(name);
>
> Since this is all private to submodule-config.c, I wonder if it would
> be cleaner to change lookup_or_create_by_name() to accept a
> name_length argument?
>

That looks amazingly clean. :)