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

From: Derrick Stolee <stolee@gmail.com>
Date: Aug 20, 2021, 15:30 UTC
Message-ID: <f9c451e4-c1b2-1680-944a-e3355248cc44@gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.2108191005560.55@tvgsbejvaqbjf.bet>

On 8/19/2021 4:07 AM, Johannes Schindelin wrote: ...

Show 16 quoted lines

>>  			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.

Show 9 quoted lines

>> +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;'.

Show 5 quoted lines

>> +	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