unpack-trees: rename 'is_excluded_from_list()'

The first consumer of pattern-matching filenames was the .gitignore feature. In that context, storing a list of patterns as a 'struct exclude_list' makes sense. However, the sparse-checkout feature then adopted these structures and methods, but with the opposite meaning: these patterns match the files that should be included! Now that this library is renamed to use 'struct pattern_list' and 'struct pattern', we can now rename the method used by the sparse-checkout feature to determine which paths should appear in the working directory. The method is_excluded_from_list() is only used by the sparse-checkout logic in unpack-trees and list-objects-filter. The confusing part is that it returned 1 for "excluded" (i.e. it matches the list of exclusions) but that really manes that the path matched the list of patterns for _inclusion_ in the working directory. Rename the method to be path_matches_pattern_list() and have it return an explicit 'enum pattern_match_result'. Here, the values MATCHED = 1, UNMATCHED = 0, and UNDECIDED = -1 agree with the previous integer values. This shift allows future consumers to better understand what the retur values mean, and provides more type checking for handling those values. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Sep 3, 2019 at 11:04 UTC 468ce99b77a0efaf1ace4c31a7b0a7d036fd9ca1
4 files changed +72 -42
dir.c
+17 -8
@@ -1072,19 +1072,28 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname
1072 }
1073
1074 /*
1075 - * Scan the list and let the last match determine the fate.
1076 - * Return 1 for exclude, 0 for include and -1 for undecided.
1075 + * Scan the list of patterns to determine if the ordered list
1076 + * of patterns matches on 'pathname'.
1077 + *
1078 + * Return 1 for a match, 0 for not matched and -1 for undecided.
1079 */
1078 -int is_excluded_from_list(const char *pathname,
1079 - int pathlen, const char *basename, int *dtype,
1080 - struct pattern_list *pl, struct index_state *istate)
1080 +enum pattern_match_result path_matches_pattern_list(
1081 + const char *pathname, int pathlen,
1082 + const char *basename, int *dtype,
1083 + struct pattern_list *pl,
1084 + struct index_state *istate)
1085 {
1086 struct path_pattern *pattern;
1087 pattern = last_matching_pattern_from_list(pathname, pathlen, basename,
1088 dtype, pl, istate);
1085 - if (pattern)
1086 - return pattern->flags & PATTERN_FLAG_NEGATIVE ? 0 : 1;
1087 - return -1; /* undecided */
1089 + if (pattern) {
1090 + if (pattern->flags & PATTERN_FLAG_NEGATIVE)
1091 + return NOT_MATCHED;
1092 + else
1093 + return MATCHED;
1094 + }
1095 +
1096 + return UNDECIDED;
1097 }
1098
1099 static struct path_pattern *last_matching_pattern_from_lists(
dir.h
+17 -4
@@ -230,10 +230,23 @@ int read_directory(struct dir_struct *, struct index_state *istate,
230 const char *path, int len,
231 const struct pathspec *pathspec);
232
233 -int is_excluded_from_list(const char *pathname, int pathlen,
234 - const char *basename, int *dtype,
235 - struct pattern_list *pl,
236 - struct index_state *istate);
233 +enum pattern_match_result {
234 + UNDECIDED = -1,
235 + NOT_MATCHED = 0,
236 + MATCHED = 1,
237 +};
238 +
239 +/*
240 + * Scan the list of patterns to determine if the ordered list
241 + * of patterns matches on 'pathname'.
242 + *
243 + * Return 1 for a match, 0 for not matched and -1 for undecided.
244 + */
245 +enum pattern_match_result path_matches_pattern_list(const char *pathname,
246 + int pathlen,
247 + const char *basename, int *dtype,
248 + struct pattern_list *pl,
249 + struct index_state *istate);
250 struct dir_entry *dir_add_ignored(struct dir_struct *dir,
251 struct index_state *istate,
252 const char *pathname, int len);
list-objects-filter.c
+15 -14
@@ -328,12 +328,12 @@ static void filter_blobs_limit__init(
328 */
329 struct frame {
330 /*
331 - * defval is the usual default include/exclude value that
331 + * default_match is the usual default include/exclude value that
332 * should be inherited as we recurse into directories based
333 * upon pattern matching of the directory itself or of a
334 * containing directory.
335 */
336 - int defval;
336 + enum pattern_match_result default_match;
337
338 /*
339 * 1 if the directory (recursively) contains any provisionally
@@ -363,8 +363,9 @@ static enum list_objects_filter_result filter_sparse(
363 void *filter_data_)
364 {
365 struct filter_sparse_data *filter_data = filter_data_;
366 - int val, dtype;
366 + int dtype;
367 struct frame *frame;
368 + enum pattern_match_result match;
369
370 switch (filter_situation) {
371 default:
@@ -373,15 +374,15 @@ static enum list_objects_filter_result filter_sparse(
374 case LOFS_BEGIN_TREE:
375 assert(obj->type == OBJ_TREE);
376 dtype = DT_DIR;
376 - val = is_excluded_from_list(pathname, strlen(pathname),
377 - filename, &dtype, &filter_data->pl,
378 - r->index);
379 - if (val < 0)
380 - val = filter_data->array_frame[filter_data->nr - 1].defval;
377 + match = path_matches_pattern_list(pathname, strlen(pathname),
378 + filename, &dtype, &filter_data->pl,
379 + r->index);
380 + if (match == UNDECIDED)
381 + match = filter_data->array_frame[filter_data->nr - 1].default_match;
382
383 ALLOC_GROW(filter_data->array_frame, filter_data->nr + 1,
384 filter_data->alloc);
384 - filter_data->array_frame[filter_data->nr].defval = val;
385 + filter_data->array_frame[filter_data->nr].default_match = match;
386 filter_data->array_frame[filter_data->nr].child_prov_omit = 0;
387 filter_data->nr++;
388
@@ -435,12 +436,12 @@ static enum list_objects_filter_result filter_sparse(
436 frame = &filter_data->array_frame[filter_data->nr - 1];
437
438 dtype = DT_REG;
438 - val = is_excluded_from_list(pathname, strlen(pathname),
439 + match = path_matches_pattern_list(pathname, strlen(pathname),
440 filename, &dtype, &filter_data->pl,
441 r->index);
441 - if (val < 0)
442 - val = frame->defval;
443 - if (val > 0) {
442 + if (match == UNDECIDED)
443 + match = frame->default_match;
444 + if (match == MATCHED) {
445 if (omits)
446 oidset_remove(omits, &obj->oid);
447 return LOFR_MARK_SEEN | LOFR_DO_SHOW;
@@ -487,7 +488,7 @@ static void filter_sparse_oid__init(
488 die("could not load filter specification");
489
490 ALLOC_GROW(d->array_frame, d->nr + 1, d->alloc);
490 - d->array_frame[d->nr].defval = 0; /* default to include */
491 + d->array_frame[d->nr].default_match = 0; /* default to include */
492 d->array_frame[d->nr].child_prov_omit = 0;
493 d->nr++;
494
unpack-trees.c
+23 -16
@@ -1265,7 +1265,8 @@ static int clear_ce_flags_1(struct index_state *istate,
1265 struct cache_entry **cache, int nr,
1266 struct strbuf *prefix,
1267 int select_mask, int clear_mask,
1268 - struct pattern_list *pl, int defval);
1268 + struct pattern_list *pl,
1269 + enum pattern_match_result default_match);
1270
1271 /* Whole directory matching */
1272 static int clear_ce_flags_dir(struct index_state *istate,
@@ -1273,19 +1274,21 @@ static int clear_ce_flags_dir(struct index_state *istate,
1274 struct strbuf *prefix,
1275 char *basename,
1276 int select_mask, int clear_mask,
1276 - struct pattern_list *pl, int defval)
1277 + struct pattern_list *pl,
1278 + enum pattern_match_result default_match)
1279 {
1280 struct cache_entry **cache_end;
1281 int dtype = DT_DIR;
1280 - int ret = is_excluded_from_list(prefix->buf, prefix->len,
1281 - basename, &dtype, pl, istate);
1282 int rc;
1283 + enum pattern_match_result ret;
1284 + ret = path_matches_pattern_list(prefix->buf, prefix->len,
1285 + basename, &dtype, pl, istate);
1286
1287 strbuf_addch(prefix, '/');
1288
1289 /* If undecided, use matching result of parent dir in defval */
1287 - if (ret < 0)
1288 - ret = defval;
1290 + if (ret == UNDECIDED)
1291 + ret = default_match;
1292
1293 for (cache_end = cache; cache_end != cache + nr; cache_end++) {
1294 struct cache_entry *ce = *cache_end;
@@ -1298,7 +1301,7 @@ static int clear_ce_flags_dir(struct index_state *istate,
1301 * with ret (iow, we know in advance the incl/excl
1302 * decision for the entire directory), clear flag here without
1303 * calling clear_ce_flags_1(). That function will call
1301 - * the expensive is_excluded_from_list() on every entry.
1304 + * the expensive path_matches_pattern_list() on every entry.
1305 */
1306 rc = clear_ce_flags_1(istate, cache, cache_end - cache,
1307 prefix,
@@ -1327,7 +1330,8 @@ static int clear_ce_flags_1(struct index_state *istate,
1330 struct cache_entry **cache, int nr,
1331 struct strbuf *prefix,
1332 int select_mask, int clear_mask,
1330 - struct pattern_list *pl, int defval)
1333 + struct pattern_list *pl,
1334 + enum pattern_match_result default_match)
1335 {
1336 struct cache_entry **cache_end = cache + nr;
1337
@@ -1338,7 +1342,8 @@ static int clear_ce_flags_1(struct index_state *istate,
1342 while(cache != cache_end) {
1343 struct cache_entry *ce = *cache;
1344 const char *name, *slash;
1341 - int len, dtype, ret;
1345 + int len, dtype;
1346 + enum pattern_match_result ret;
1347
1348 if (select_mask && !(ce->ce_flags & select_mask)) {
1349 cache++;
@@ -1362,7 +1367,7 @@ static int clear_ce_flags_1(struct index_state *istate,
1367 prefix,
1368 prefix->buf + prefix->len - len,
1369 select_mask, clear_mask,
1365 - pl, defval);
1370 + pl, default_match);
1371
1372 /* clear_c_f_dir eats a whole dir already? */
1373 if (processed) {
@@ -1374,18 +1379,20 @@ static int clear_ce_flags_1(struct index_state *istate,
1379 strbuf_addch(prefix, '/');
1380 cache += clear_ce_flags_1(istate, cache, cache_end - cache,
1381 prefix,
1377 - select_mask, clear_mask, pl, defval);
1382 + select_mask, clear_mask, pl,
1383 + default_match);
1384 strbuf_setlen(prefix, prefix->len - len - 1);
1385 continue;
1386 }
1387
1388 /* Non-directory */
1389 dtype = ce_to_dtype(ce);
1384 - ret = is_excluded_from_list(ce->name, ce_namelen(ce),
1385 - name, &dtype, pl, istate);
1386 - if (ret < 0)
1387 - ret = defval;
1388 - if (ret > 0)
1390 + ret = path_matches_pattern_list(ce->name,
1391 + ce_namelen(ce),
1392 + name, &dtype, pl, istate);
1393 + if (ret == UNDECIDED)
1394 + ret = default_match;
1395 + if (ret == MATCHED)
1396 ce->ce_flags &= ~clear_mask;
1397 cache++;
1398 }