Re: [PATCH v3 5/8] sparse-checkout: create helper methods

[Derrick Stolee <stolee@gmail.com>]

On 8/19/2021 4:07 AM, Johannes Schindelin wrote:
...
>>  			if (matches_skip_worktree(pathspec, i, &skip_worktree_seen) ||
>> -			    (sparse_checkout_enabled &&
>> -			     !path_matches_pattern_list(path, strlen(path), NULL,
>> -							&dtype, &pl, &the_index))) {
>> +			    (core_apply_sparse_checkout &&
> 
> Do we need to test for `core_apply_sparse_checkout` here? Or does the `if
> (!istate->sparse_checkout_patterns) return MATCHED;` early return in
> `path_in_sparse_checkout()` suffice to catch this?
> 
> The remainder of the patch looks good to me.
> 
> Thank you,
> Dscho
> 
>> +			     path_in_sparse_checkout(path, &the_index) == NOT_MATCHED)) {

Thank you for pointing out this. This is actually a stale change from an earlier
version where path_in_sparse_checkout returned an 'enum pattern_match_result'
but now casts down to an 'int', meaning '0' is not in the sparse-checkout and '1'
is that it _is_ in the sparse-checkout.

>> +int path_in_sparse_checkout(const char *path,
>> +			    struct index_state *istate)
>> +{
>> +	const char *base;
>> +	int dtype = DT_REG;
>> +	init_sparse_checkout_patterns(istate);
>> +
>> +	if (!istate->sparse_checkout_patterns)
>> +		return MATCHED;

So here, if we do not have sparse-checkout patterns (for example, if
core_apply_sparse_checkout is false), then this returns MATCHED (== 1).

To be extra clear, this should just be 'return 1;'.

>> +	base = strrchr(path, '/');
>> +	return path_matches_pattern_list(path, strlen(path), base ? base + 1 : path,
>> +					 &dtype,
>> +					 istate->sparse_checkout_patterns,
>> +					 istate) > 0;

Here, we are selecting the portion of 'enum pattern_match_result' that
we care about (MATCHED and MATCHED_RECURSIVE). The UNMATCHED (0) and
UNDECIDED (-1) are the other possibilities, but file paths will not
return UNDECIDED, that is instead for directories in non-cone mode
patterns.

Thanks,
-Stolee