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

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

quoted

 			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

quoted

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

quoted

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

quoted

+	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