Re: [PATCH v7 2/2] submodule: skip redundant active entries when pattern covers path

[Junio C Hamano <gitster@pobox.com>]

JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:

>> The new helper is a maintenance burden to keep in sync with
>> submodule.c:is_tree_submodule_active(); if we really want to go this
>> route, the patch should extract that "ah, submodule.active is set so
>> let's turn it into pathspec and see if the path matches" part of the
>> logic to make sure the logic is shared.  But I am wondering if we
>> can do this without any new helper.
>
> I've actually done something like this, but I've wrapped the core logic within
> the if else after checking [1]

I do not think I follow what you want to say here.  Care to rephrase?

>> I.e.  Can we replace this if() condition with something like this?
>>
>>         /*
>>          * Explicitly set 'submodule.<name>.active' only if it is not
>>          * 'active' due to other reasons.
>>          */
>>         if (!is_submodule_active(the_repository, add_data->sm_path)) {
>>
>> That is, we ask if the submodule is already active (we are before
>> adding submodule.<name>.active for this thing---if may be active due
>> to submodule.active or submodule.<name>.url) and enter the block
>> only when it is not yet.
>>
>> That way, this codepath does not have to worry about the exact logic
>> that determines if a submodule is 'active' even when its .active
>> configuration variable is not set.
>>
>> >               key = xstrfmt("submodule.%s.active", add_data->sm_name);
>> >               git_config_set_gently(key, "true");
>> >               free(key);
>
>
> I've done the exact thing in a bit different way

I am confused again, as this reads as if it is an oxymoron --- did
you write the exactly the same thing as suggested above, or did you
write something different?