Re: [PATCH v7 12/16] diff-lib: handle index diffs with sparse dirs

>>> +       /*
>>> +        * If both are sparse directory entries, then expand the
>>> +        * modifications to the file level.
>>> +        */
>>> +       if (old_entry && new_entry &&
>>> +           S_ISSPARSEDIR(old_entry->ce_mode) &&
>>> +           S_ISSPARSEDIR(new_entry->ce_mode)) {
>>> +               diff_tree_oid(&old_entry->oid, &new_entry->oid, new_entry->name, &revs->diffopt);
>>> +               return 0;
>>> +       }
>>> +
>>>         if (get_stat_data(istate, new_entry, &oid, &mode, cached, match_missing,
>>>                           &dirty_submodule, &revs->diffopt) < 0) {
>>>                 if (report_missing)
>>
>> Love the simpler patch.
>>
>> I'm curious about the case where S_ISSPARSEDIR(old_entry->ce_mode) !=
>> S_ISSPARSEDIR(new_entry->ce_mode), though; how is that handled?
> 
> Digging a little deeper, it appears that we could add this just before
> your new if-block:
> 
>     assert(S_ISSPARSEDIR(old_entry->ce_mode) ==
>            S_ISSPARSEDIR(new_entry->ce_mode));
> 
> And the code still functions, while that also removes some of the
> surprise factor.  I'm guessing that the difference between "folder1"
> and "folder1/" cause us to never try to directly compare a file to a
> directory...but if that's accurate, a comment of some effect might
> help make this code be a little clearer and make readers less likely
> to wonder why you need to check that both old and new are sparse
> directories.