On 5/16/2022 4:38 PM, Victoria Dye wrote:
> Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>> +
>> + /* We now have a sparse directory entry. Should we expand? */
>> + if (pl &&
>> + path_matches_pattern_list(ce->name, ce->ce_namelen,
>> + NULL, &dtype,
>> + pl, istate) <= 0) {
>
> If I'm reading this correctly, what this is doing is:
>
> - if we have a sparse directory entry
> - ...and we're expanding only what matches the pattern list (i.e., not
> 'ensure_full_index')
> - ...and that sparse directory path is either *not matching* or *undecided
> whether it matches* the pattern list
> - ...then we add the sparse directory to the result index and continue.
>
> The part that's confusing me is the "<= 0", which means a return value of
> 'UNDECIDED' from 'path_matches_pattern_list' adds the sparse directory
> as-is. At the moment, it looks like 'UNDECIDED' is only returned if not
> using cone patterns (so it shouldn't make a functional difference at this
> point), but wouldn't that return value indicate that the pattern *may or may
> not* match the path, so we should continue on to 'read_tree_at()'?
>
> All that to say, should the condition be:
>
> /* We now have a sparse directory entry. Should we expand? */
> if (pl &&
> path_matches_pattern_list(ce->name, ce->ce_namelen,
> NULL, &dtype,
> pl, istate) == NOT_MATCHED) {
>
> to reflect that a sparse directory should only be added to the index if we
> *know* it isn't matched?
>
> To be clear, this is ultimately a non-functional nit - my question is mostly
> to make sure I understand the intent of the code.
You are 100% correct, and using "== NOT_MATCHED" makes the intention clear,
as well as being more robust to a breakage in path_matches_pattern_list().
Thanks,
-Stolee
