On 5/16/2022 4:38 PM, Victoria Dye wrote:

Derrick Stolee via GitGitGadget wrote:

quoted

From: Derrick Stolee <redacted>

quoted

+
+		/* We now have a sparse directory entry. Should we expand? */
+		if (pl &&
+		    path_matches_pattern_list(ce->name, ce->ce_namelen,
+					      NULL, &dtype,
+					      pl, istate) <= 0) {

If I'm reading this correctly, what this is doing is:

- if we have a sparse directory entry
- ...and we're expanding only what matches the pattern list (i.e., not
  'ensure_full_index')
- ...and that sparse directory path is either *not matching* or *undecided
  whether it matches* the pattern list
- ...then we add the sparse directory to the result index and continue. 

The part that's confusing me is the "<= 0", which means a return value of
'UNDECIDED' from 'path_matches_pattern_list' adds the sparse directory
as-is. At the moment, it looks like 'UNDECIDED' is only returned if not
using cone patterns (so it shouldn't make a functional difference at this
point), but wouldn't that return value indicate that the pattern *may or may
not* match the path, so we should continue on to 'read_tree_at()'?

All that to say, should the condition be:

		/* We now have a sparse directory entry. Should we expand? */
		if (pl &&
		    path_matches_pattern_list(ce->name, ce->ce_namelen,
					      NULL, &dtype,
					      pl, istate) == NOT_MATCHED) {

to reflect that a sparse directory should only be added to the index if we
*know* it isn't matched?

To be clear, this is ultimately a non-functional nit - my question is mostly
to make sure I understand the intent of the code.

You are 100% correct, and using "== NOT_MATCHED" makes the intention clear,
as well as being more robust to a breakage in path_matches_pattern_list().

Thanks,
-Stolee