On 5/19/2022 3:50 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> list is NULL as that does the same as ensure_full_index(). In fact,
>> ensure_full_index() is converted to a shim over
>> expand_to_pattern_list().
>
> Sounds like a natural evolution of the API that used to be
> all-or-none to expand-only-those-that-match.
>
> The old one had a sensible name to tell us that it is about the
> in-core index (and "full index" implied it was about sparse-index
> feature because what state other than "full" the index can be---some
> are shrunk into tree entries, which by definition is the
> sparse-index feature). Contrasted to that, the name of the new one
> is horrible. It does not even have index anywhere in the name.
>
> I wonder expand_index() would work?
Makes sense. Good suggestion.
>> - trace2_region_enter("index", "ensure_full_index", istate->repo);
>> + /*
>> + * A NULL pattern set indicates we are expanding a full index, so
>> + * we use a special region name that indicates the full expansion.
>> + * This is used by test cases, but also helps to differentiate the
>> + * two cases.
>> + */
>
> Except that we lost the distinction for non-cone mode, which I am
> not sure matters, but I suspect we do not have to, if we do not want
> to. Nobody used "pl" up to this point, so resetting it to NULL can
> be done much later. In later phases of this series, we add another
> case where we can lose pl even if we are not using cone mode, so
> this distinction may start to matter later. I dunno.
>
> I'd invent a separate "const char *tr2_region_label" variable and
> set it at the beginning, regardless of where we clobber pl and why,
> and use that label variable for trace2 calls, if I were doing this
> patch. That feels much simpler and cleaner.
Good idea.
Thanks,
-Stolee
