From: Derrick Stolee <stolee@gmail.com>
Date: Tue, 06 Apr 2021 11:46:17 GMT
Subject: Re: [PATCH v2 24/25] sparse-index: expand_to_path()
Message-ID: <4768fac6-1d8d-4865-76a2-19f110f66219@gmail.com>
In-Reply-To: <CABPp-BE5xLMsVdamAb=MgiBnXKQrYDPJXa1TWutDK+nVYRPyRQ@mail.gmail.com>

On 4/5/2021 3:32 PM, Elijah Newren wrote:
> On Wed, Mar 31, 2021 at 6:50 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Derrick Stolee <dstolee@microsoft.com>>> +               if (index_file_exists(istate, path_mutable.buf,
>> +                                     path_mutable.len, icase)) {
>> +                       /*
>> +                        * We found a parent directory in the name-hash
>> +                        * hashtable, which means that this entry could
>> +                        * exist within a sparse-directory entry. Expand
>> +                        * accordingly.
> 
> "this entry" is a bit unclear; it might be worth referring to the
> "path" variable instead.  I think it'd also help to explain the
> reasoning for using the '/' character, because while it's clear when
> looking at the series, just running across it in the code it might be
> easy to forget or miss.  Perhaps:
> 
>                     * We found a parent directory in the name-hash
>                     * hashtable, because only sparse directory entries
>                     * have a trailing '/' character.  Since "path" wasn't
>                     * in the index, perhaps it exists within this
>                     * sparse-directory.  Expand accordingly.

A good recommendation. Thanks.

-Stolee