On 5/31/2022 5:56 AM, Shaoxuan Yuan wrote:
> On Fri, May 27, 2022 at 11:27 PM Derrick Stolee
> <derrickstolee@xxxxxxxxxx> wrote:
>>
>> On 5/27/2022 6:08 AM, Shaoxuan Yuan wrote:
...
>> This appears to check if the _first_ entry under the directory
>> is sparse, but not if _all_ entries are sparse. These are not
>> the same thing, even in cone-mode sparse-checkout. The t1092
>> test directory has files like "folder1/0/0/a" but if
>> "folder1/1" is in the sparse-checkout cone, then that first
>> entry has the skip-worktree bit, but "folder1/1/a" and "folder1/a"
>> do not.
>
> Yes, it is checking the first entry and this would not work without the
> lstat in the front. But I think the "lstat < 0" makes sure that this directory
> cannot be partially sparsified.
>
> It is either missing both in the worktree and index, or missing in the worktree
> but present in index (with all its content sparsified). And because of that,
> I think only the first entry needs to be checked.
Ah! Good thinking. I hadn't considered that extra detail, so
we get to save some cycles here.
>>> + }
>>> + return ret;
>>
>> At the moment, it doesn't seem like we need 'ret' since the
>> only place you set it is in "return ret = 0;" (which could
>> just be "return 0;" while the others are "return 1;"). But,
>> perhaps you intended to create a loop over 'pos' while
>> with_slash is a prefix of the cache entry?
>
> I agree that this variable is redundant. But I fail to understand
> the logical relation between before "But," and after "But,". Please
> elaborate on that?
I was just thinking that if you intended to write a loop as
I had suggested, then 'ret' could be modified or used in more
places. Feel free to ignore since we resolved that.
>>> + else if (!check_dir_in_index(src, length) &&
>>> + !path_in_sparse_checkout(src_w_slash, &the_index)) {
>>
>> style-nit: You'll want to align the different parts of your
>> logical statement to agree with the end of the "else if (",
>>
>> else if (A &&
>> B) {
>>
>
> This one is interesting because it appears just alright in my VSCode editor.
> Later I found that it is because git-diff is using a tab size of 8 or something,
> but my VSCode uses tab size of 4. After I configured the git-diff tab rendering
> size, it looks alright. Same for another style nit down below.
That'll do it. You can double-check the alignment in your GGG
PR, which should use the correct tab width.
Thanks,
-Stolee
