* [PATCH 0/5] Refactor excludes library
@ 2019-09-03 18:04 Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 1/5] treewide: rename 'struct exclude' to 'struct path_pattern' Derrick Stolee via GitGitGadget
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2019-09-03 18:04 UTC (permalink / raw)
To: git; +Cc: newren, pclouds, peff, jon, matvore, Junio C Hamano
The exclude library defined in dir.h was originally written for the
.gitignore feature, but has since been used for .gitattributes and
sparse-checkout. In the later applications, these patterns are used for
inclusion rather than exclusion, so the name is confusing. This gets
particularly bad when looking at how the sparse-checkout feature uses
is_excluded_from_list() to really mean "should be included in the working
directory".
This series performs several renames of structs and methods to generalize
the exclude library to be a "pattern matching" library. Instead of a list of
excludes, we have a list of path patterns. It is up to the consumer to
decide what to do with a match. The .gitignore logic will still treat the
patterns as a list of exclusions, the sparse-checkout logic treats the
patterns as a list of inclusions.
For this reason, some methods and structs in dir.h retain "exclude" in their
name. These are limited to things consumed only by the .gitignore feature
(as far as I can tell).
Most of these changes are mechanical find-and-replaces, with the exception
of some variable names and the last patch.
The last patch, "unpack-trees: rename 'is_excluded_from_list()'", performs a
more meaningful refactor. The method is_excluded_from_list() was only used
by sparse-checkout (inside the clear_ce_flags() methods) to see if a path
should be included in the working directory. The return value of "1 for
excluded" was confusing. Instead, use a new enum value. This required
changing several method prototypes inside unpack-trees.c.
This refactor was inspired by Elijah Newren's feedback [1] on my
sparse-checkout builtin RFC. I am working on a few other orthogonal changes
to make to the existing sparse-checkout behavior before I resubmit that RFC.
I had started working on v2.23.0, but found adjacent-diff conflicts with
md/list-objects-filter-combo [2] and js/partial-clone-sparse-blob [3]. Those
branches are independent, but the conflicts with
md/list-objects-filter-combo were more severe (and that branch seems closer
to merging) so this is based on md/list-object-filter-combo. Hopefully the
conflicts with js/partial-clone-sparse-blob are clear enough to resolve
easily.
Thanks, -Stolee
[1]
https://public-inbox.org/git/CABPp-BFMtO=7UGVZPbqh3tthSetvz5F=W3S=RsryPSuchmZeZw@mail.gmail.com/
Re: [PATCH 8/9] sparse-checkout: use hashmaps for cone patterns
[2] https://public-inbox.org/git/cover.1558030802.git.matvore@google.com/
[RFC PATCH 0/3] implement composite filters
[3] https://public-inbox.org/git/20190828201824.1255-1-jon@jonsimons.org/
[PATCH 0/2] partial-clone: fix two issues with sparse filter handling
Derrick Stolee (5):
treewide: rename 'struct exclude' to 'struct path_pattern'
treewide: rename 'struct exclude_list' to 'struct pattern_list'
treewide: rename 'EXCL_FLAG_' to 'PATTERN_FLAG_'
treewide: rename 'exclude' methods to 'pattern'
unpack-trees: rename 'is_excluded_from_list()'
Documentation/RelNotes/2.7.1.txt | 2 +-
Documentation/RelNotes/2.8.0.txt | 2 +-
.../technical/api-directory-listing.txt | 6 +-
attr.c | 10 +-
builtin/check-ignore.c | 34 +--
builtin/clean.c | 12 +-
builtin/ls-files.c | 8 +-
dir.c | 284 +++++++++---------
dir.h | 71 +++--
list-objects-filter.c | 37 +--
unpack-trees.c | 67 +++--
unpack-trees.h | 4 +-
12 files changed, 285 insertions(+), 252 deletions(-)
base-commit: 90d21f9ebf6906f0ebb4fb1b20ec9536072e2916
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-329%2Fderrickstolee%2Fexclude-refactor-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-329/derrickstolee/exclude-refactor-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/329
--
gitgitgadget
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] treewide: rename 'struct exclude' to 'struct path_pattern'
2019-09-03 18:04 [PATCH 0/5] Refactor excludes library Derrick Stolee via GitGitGadget
@ 2019-09-03 18:04 ` Derrick Stolee via GitGitGadget
2019-09-05 6:55 ` Jeff King
2019-09-03 18:04 ` [PATCH 3/5] treewide: rename 'EXCL_FLAG_' to 'PATTERN_FLAG_' Derrick Stolee via GitGitGadget
` (5 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2019-09-03 18:04 UTC (permalink / raw)
To: git; +Cc: newren, pclouds, peff, jon, matvore, Junio C Hamano,
Derrick Stolee
From: Derrick Stolee <dstolee@microsoft.com>
The first consumer of pattern-matching filenames was the
.gitignore feature. In that context, storing a list of patterns
as a list of 'struct exclude' items 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!
It would be clearer to rename this entire library as a "pattern
matching" library, and the callers apply exclusion/inclusion
logic accordingly based on their needs.
This commit renames 'struct exclude' to 'struct path_pattern'
and renames several variable names to match. 'struct pattern'
was already taken by attr.c, and this more completely describes
that the patterns are specific to file paths.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
builtin/check-ignore.c | 34 ++++++------
dir.c | 115 +++++++++++++++++++++--------------------
dir.h | 8 +--
3 files changed, 80 insertions(+), 77 deletions(-)
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 599097304b..9a0f234514 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -32,19 +32,19 @@ static const struct option check_ignore_options[] = {
OPT_END()
};
-static void output_exclude(const char *path, struct exclude *exclude)
+static void output_pattern(const char *path, struct path_pattern *pattern)
{
- char *bang = (exclude && exclude->flags & EXC_FLAG_NEGATIVE) ? "!" : "";
- char *slash = (exclude && exclude->flags & EXC_FLAG_MUSTBEDIR) ? "/" : "";
+ char *bang = (pattern && pattern->flags & EXC_FLAG_NEGATIVE) ? "!" : "";
+ char *slash = (pattern && pattern->flags & EXC_FLAG_MUSTBEDIR) ? "/" : "";
if (!nul_term_line) {
if (!verbose) {
write_name_quoted(path, stdout, '\n');
} else {
- if (exclude) {
- quote_c_style(exclude->el->src, NULL, stdout, 0);
+ if (pattern) {
+ quote_c_style(pattern->el->src, NULL, stdout, 0);
printf(":%d:%s%s%s\t",
- exclude->srcpos,
- bang, exclude->pattern, slash);
+ pattern->srcpos,
+ bang, pattern->pattern, slash);
}
else {
printf("::\t");
@@ -56,11 +56,11 @@ static void output_exclude(const char *path, struct exclude *exclude)
if (!verbose) {
printf("%s%c", path, '\0');
} else {
- if (exclude)
+ if (pattern)
printf("%s%c%d%c%s%s%s%c%s%c",
- exclude->el->src, '\0',
- exclude->srcpos, '\0',
- bang, exclude->pattern, slash, '\0',
+ pattern->el->src, '\0',
+ pattern->srcpos, '\0',
+ bang, pattern->pattern, slash, '\0',
path, '\0');
else
printf("%c%c%c%s%c", '\0', '\0', '\0', path, '\0');
@@ -74,7 +74,7 @@ static int check_ignore(struct dir_struct *dir,
const char *full_path;
char *seen;
int num_ignored = 0, i;
- struct exclude *exclude;
+ struct path_pattern *pattern;
struct pathspec pathspec;
if (!argc) {
@@ -103,15 +103,15 @@ static int check_ignore(struct dir_struct *dir,
seen = find_pathspecs_matching_against_index(&pathspec, &the_index);
for (i = 0; i < pathspec.nr; i++) {
full_path = pathspec.items[i].match;
- exclude = NULL;
+ pattern = NULL;
if (!seen[i]) {
int dtype = DT_UNKNOWN;
- exclude = last_exclude_matching(dir, &the_index,
+ pattern = last_exclude_matching(dir, &the_index,
full_path, &dtype);
}
- if (!quiet && (exclude || show_non_matching))
- output_exclude(pathspec.items[i].original, exclude);
- if (exclude)
+ if (!quiet && (pattern || show_non_matching))
+ output_pattern(pathspec.items[i].original, pattern);
+ if (pattern)
num_ignored++;
}
free(seen);
diff --git a/dir.c b/dir.c
index ba4a51c296..4128d59d7a 100644
--- a/dir.c
+++ b/dir.c
@@ -602,27 +602,27 @@ void parse_exclude_pattern(const char **pattern,
void add_exclude(const char *string, const char *base,
int baselen, struct exclude_list *el, int srcpos)
{
- struct exclude *x;
+ struct path_pattern *pattern;
int patternlen;
unsigned flags;
int nowildcardlen;
parse_exclude_pattern(&string, &patternlen, &flags, &nowildcardlen);
if (flags & EXC_FLAG_MUSTBEDIR) {
- FLEXPTR_ALLOC_MEM(x, pattern, string, patternlen);
+ FLEXPTR_ALLOC_MEM(pattern, pattern, string, patternlen);
} else {
- x = xmalloc(sizeof(*x));
- x->pattern = string;
+ pattern = xmalloc(sizeof(*pattern));
+ pattern->pattern = string;
}
- x->patternlen = patternlen;
- x->nowildcardlen = nowildcardlen;
- x->base = base;
- x->baselen = baselen;
- x->flags = flags;
- x->srcpos = srcpos;
- ALLOC_GROW(el->excludes, el->nr + 1, el->alloc);
- el->excludes[el->nr++] = x;
- x->el = el;
+ pattern->patternlen = patternlen;
+ pattern->nowildcardlen = nowildcardlen;
+ pattern->base = base;
+ pattern->baselen = baselen;
+ pattern->flags = flags;
+ pattern->srcpos = srcpos;
+ ALLOC_GROW(el->patterns, el->nr + 1, el->alloc);
+ el->patterns[el->nr++] = pattern;
+ pattern->el = el;
}
static int read_skip_worktree_file_from_index(const struct index_state *istate,
@@ -651,8 +651,8 @@ void clear_exclude_list(struct exclude_list *el)
int i;
for (i = 0; i < el->nr; i++)
- free(el->excludes[i]);
- free(el->excludes);
+ free(el->patterns[i]);
+ free(el->patterns);
free(el->filebuf);
memset(el, 0, sizeof(*el));
@@ -1021,51 +1021,54 @@ int match_pathname(const char *pathname, int pathlen,
* any, determines the fate. Returns the exclude_list element which
* matched, or NULL for undecided.
*/
-static struct exclude *last_exclude_matching_from_list(const char *pathname,
+static struct path_pattern *last_exclude_matching_from_list(const char *pathname,
int pathlen,
const char *basename,
int *dtype,
struct exclude_list *el,
struct index_state *istate)
{
- struct exclude *exc = NULL; /* undecided */
+ struct path_pattern *res = NULL; /* undecided */
int i;
if (!el->nr)
return NULL; /* undefined */
for (i = el->nr - 1; 0 <= i; i--) {
- struct exclude *x = el->excludes[i];
- const char *exclude = x->pattern;
- int prefix = x->nowildcardlen;
+ struct path_pattern *pattern = el->patterns[i];
+ const char *exclude = pattern->pattern;
+ int prefix = pattern->nowildcardlen;
- if (x->flags & EXC_FLAG_MUSTBEDIR) {
+ if (pattern->flags & EXC_FLAG_MUSTBEDIR) {
if (*dtype == DT_UNKNOWN)
*dtype = get_dtype(NULL, istate, pathname, pathlen);
if (*dtype != DT_DIR)
continue;
}
- if (x->flags & EXC_FLAG_NODIR) {
+ if (pattern->flags & EXC_FLAG_NODIR) {
if (match_basename(basename,
pathlen - (basename - pathname),
- exclude, prefix, x->patternlen,
- x->flags)) {
- exc = x;
+ exclude, prefix, pattern->patternlen,
+ pattern->flags)) {
+ res = pattern;
break;
}
continue;
}
- assert(x->baselen == 0 || x->base[x->baselen - 1] == '/');
+ assert(pattern->baselen == 0 ||
+ pattern->base[pattern->baselen - 1] == '/');
if (match_pathname(pathname, pathlen,
- x->base, x->baselen ? x->baselen - 1 : 0,
- exclude, prefix, x->patternlen, x->flags)) {
- exc = x;
+ pattern->base,
+ pattern->baselen ? pattern->baselen - 1 : 0,
+ exclude, prefix, pattern->patternlen,
+ pattern->flags)) {
+ res = pattern;
break;
}
}
- return exc;
+ return res;
}
/*
@@ -1076,30 +1079,30 @@ int is_excluded_from_list(const char *pathname,
int pathlen, const char *basename, int *dtype,
struct exclude_list *el, struct index_state *istate)
{
- struct exclude *exclude;
- exclude = last_exclude_matching_from_list(pathname, pathlen, basename,
+ struct path_pattern *pattern;
+ pattern = last_exclude_matching_from_list(pathname, pathlen, basename,
dtype, el, istate);
- if (exclude)
- return exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
+ if (pattern)
+ return pattern->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
return -1; /* undecided */
}
-static struct exclude *last_exclude_matching_from_lists(struct dir_struct *dir,
- struct index_state *istate,
- const char *pathname, int pathlen, const char *basename,
- int *dtype_p)
+static struct path_pattern *last_exclude_matching_from_lists(
+ struct dir_struct *dir, struct index_state *istate,
+ const char *pathname, int pathlen,
+ const char *basename, int *dtype_p)
{
int i, j;
struct exclude_list_group *group;
- struct exclude *exclude;
+ struct path_pattern *pattern;
for (i = EXC_CMDL; i <= EXC_FILE; i++) {
group = &dir->exclude_list_group[i];
for (j = group->nr - 1; j >= 0; j--) {
- exclude = last_exclude_matching_from_list(
+ pattern = last_exclude_matching_from_list(
pathname, pathlen, basename, dtype_p,
&group->el[j], istate);
- if (exclude)
- return exclude;
+ if (pattern)
+ return pattern;
}
}
return NULL;
@@ -1132,7 +1135,7 @@ static void prep_exclude(struct dir_struct *dir,
break;
el = &group->el[dir->exclude_stack->exclude_ix];
dir->exclude_stack = stk->prev;
- dir->exclude = NULL;
+ dir->pattern = NULL;
free((char *)el->src); /* see strbuf_detach() below */
clear_exclude_list(el);
free(stk);
@@ -1140,7 +1143,7 @@ static void prep_exclude(struct dir_struct *dir,
}
/* Skip traversing into sub directories if the parent is excluded */
- if (dir->exclude)
+ if (dir->pattern)
return;
/*
@@ -1189,15 +1192,15 @@ static void prep_exclude(struct dir_struct *dir,
if (stk->baselen) {
int dt = DT_DIR;
dir->basebuf.buf[stk->baselen - 1] = 0;
- dir->exclude = last_exclude_matching_from_lists(dir,
+ dir->pattern = last_exclude_matching_from_lists(dir,
istate,
dir->basebuf.buf, stk->baselen - 1,
dir->basebuf.buf + current, &dt);
dir->basebuf.buf[stk->baselen - 1] = '/';
- if (dir->exclude &&
- dir->exclude->flags & EXC_FLAG_NEGATIVE)
- dir->exclude = NULL;
- if (dir->exclude) {
+ if (dir->pattern &&
+ dir->pattern->flags & EXC_FLAG_NEGATIVE)
+ dir->pattern = NULL;
+ if (dir->pattern) {
dir->exclude_stack = stk;
return;
}
@@ -1223,7 +1226,7 @@ static void prep_exclude(struct dir_struct *dir,
/*
* dir->basebuf gets reused by the traversal, but we
* need fname to remain unchanged to ensure the src
- * member of each struct exclude correctly
+ * member of each struct path_pattern correctly
* back-references its source file. Other invocations
* of add_exclude_list provide stable strings, so we
* strbuf_detach() and free() here in the caller.
@@ -1266,7 +1269,7 @@ static void prep_exclude(struct dir_struct *dir,
* Returns the exclude_list element which matched, or NULL for
* undecided.
*/
-struct exclude *last_exclude_matching(struct dir_struct *dir,
+struct path_pattern *last_exclude_matching(struct dir_struct *dir,
struct index_state *istate,
const char *pathname,
int *dtype_p)
@@ -1277,8 +1280,8 @@ struct exclude *last_exclude_matching(struct dir_struct *dir,
prep_exclude(dir, istate, pathname, basename-pathname);
- if (dir->exclude)
- return dir->exclude;
+ if (dir->pattern)
+ return dir->pattern;
return last_exclude_matching_from_lists(dir, istate, pathname, pathlen,
basename, dtype_p);
@@ -1292,10 +1295,10 @@ struct exclude *last_exclude_matching(struct dir_struct *dir,
int is_excluded(struct dir_struct *dir, struct index_state *istate,
const char *pathname, int *dtype_p)
{
- struct exclude *exclude =
+ struct path_pattern *pattern =
last_exclude_matching(dir, istate, pathname, dtype_p);
- if (exclude)
- return exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
+ if (pattern)
+ return pattern->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
return 0;
}
diff --git a/dir.h b/dir.h
index 680079bbe3..e8b90fc482 100644
--- a/dir.h
+++ b/dir.h
@@ -16,7 +16,7 @@ struct dir_entry {
#define EXC_FLAG_MUSTBEDIR 8
#define EXC_FLAG_NEGATIVE 16
-struct exclude {
+struct path_pattern {
/*
* This allows callers of last_exclude_matching() etc.
* to determine the origin of the matching pattern.
@@ -54,7 +54,7 @@ struct exclude_list {
/* origin of list, e.g. path to filename, or descriptive string */
const char *src;
- struct exclude **excludes;
+ struct path_pattern **patterns;
};
/*
@@ -191,7 +191,7 @@ struct dir_struct {
* matching exclude struct if the directory is excluded.
*/
struct exclude_stack *exclude_stack;
- struct exclude *exclude;
+ struct path_pattern *pattern;
struct strbuf basebuf;
/* Enable untracked file cache if set */
@@ -248,7 +248,7 @@ int match_pathname(const char *, int,
const char *, int,
const char *, int, int, unsigned);
-struct exclude *last_exclude_matching(struct dir_struct *dir,
+struct path_pattern *last_exclude_matching(struct dir_struct *dir,
struct index_state *istate,
const char *name, int *dtype);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/5] treewide: rename 'struct exclude_list' to 'struct pattern_list'
2019-09-03 18:04 [PATCH 0/5] Refactor excludes library Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 1/5] treewide: rename 'struct exclude' to 'struct path_pattern' Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 3/5] treewide: rename 'EXCL_FLAG_' to 'PATTERN_FLAG_' Derrick Stolee via GitGitGadget
@ 2019-09-03 18:04 ` Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 4/5] treewide: rename 'exclude' methods to 'pattern' Derrick Stolee via GitGitGadget
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2019-09-03 18:04 UTC (permalink / raw)
To: git; +Cc: newren, pclouds, peff, jon, matvore, Junio C Hamano,
Derrick Stolee
From: Derrick Stolee <dstolee@microsoft.com>
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!
It would be clearer to rename this entire library as a "pattern
matching" library, and the callers apply exclusion/inclusion
logic accordingly based on their needs.
This commit renames 'struct exclude_list' to 'struct pattern_list'
and renames several variables called 'el' to 'pl'.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
builtin/check-ignore.c | 4 +-
builtin/clean.c | 12 ++---
builtin/ls-files.c | 6 +--
dir.c | 106 ++++++++++++++++++++---------------------
dir.h | 18 +++----
list-objects-filter.c | 8 ++--
unpack-trees.c | 42 ++++++++--------
unpack-trees.h | 4 +-
8 files changed, 100 insertions(+), 100 deletions(-)
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 9a0f234514..97108ccb9c 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -41,7 +41,7 @@ static void output_pattern(const char *path, struct path_pattern *pattern)
write_name_quoted(path, stdout, '\n');
} else {
if (pattern) {
- quote_c_style(pattern->el->src, NULL, stdout, 0);
+ quote_c_style(pattern->pl->src, NULL, stdout, 0);
printf(":%d:%s%s%s\t",
pattern->srcpos,
bang, pattern->pattern, slash);
@@ -58,7 +58,7 @@ static void output_pattern(const char *path, struct path_pattern *pattern)
} else {
if (pattern)
printf("%s%c%d%c%s%s%s%c%s%c",
- pattern->el->src, '\0',
+ pattern->pl->src, '\0',
pattern->srcpos, '\0',
bang, pattern->pattern, slash, '\0',
path, '\0');
diff --git a/builtin/clean.c b/builtin/clean.c
index aaba4af3c2..d8c847d9fd 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -647,7 +647,7 @@ static int filter_by_patterns_cmd(void)
struct strbuf confirm = STRBUF_INIT;
struct strbuf **ignore_list;
struct string_list_item *item;
- struct exclude_list *el;
+ struct pattern_list *pl;
int changed = -1, i;
for (;;) {
@@ -670,7 +670,7 @@ static int filter_by_patterns_cmd(void)
break;
memset(&dir, 0, sizeof(dir));
- el = add_exclude_list(&dir, EXC_CMDL, "manual exclude");
+ pl = add_exclude_list(&dir, EXC_CMDL, "manual exclude");
ignore_list = strbuf_split_max(&confirm, ' ', 0);
for (i = 0; ignore_list[i]; i++) {
@@ -678,7 +678,7 @@ static int filter_by_patterns_cmd(void)
if (!ignore_list[i]->len)
continue;
- add_exclude(ignore_list[i]->buf, "", 0, el, -(i+1));
+ add_exclude(ignore_list[i]->buf, "", 0, pl, -(i+1));
}
changed = 0;
@@ -900,7 +900,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
struct pathspec pathspec;
struct strbuf buf = STRBUF_INIT;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
- struct exclude_list *el;
+ struct pattern_list *pl;
struct string_list_item *item;
const char *qname;
struct option options[] = {
@@ -957,9 +957,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
if (!ignored)
setup_standard_excludes(&dir);
- el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
+ pl = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
for (i = 0; i < exclude_list.nr; i++)
- add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
+ add_exclude(exclude_list.items[i].string, "", 0, pl, -(i+1));
parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_CWD,
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 7f83c9a6f2..df8918a128 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -516,7 +516,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
int require_work_tree = 0, show_tag = 0, i;
const char *max_prefix;
struct dir_struct dir;
- struct exclude_list *el;
+ struct pattern_list *pl;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
struct option builtin_ls_files_options[] = {
/* Think twice before adding "--nul" synonym to this */
@@ -594,9 +594,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
- el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
+ pl = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
for (i = 0; i < exclude_list.nr; i++) {
- add_exclude(exclude_list.items[i].string, "", 0, el, --exclude_args);
+ add_exclude(exclude_list.items[i].string, "", 0, pl, --exclude_args);
}
if (show_tag || show_valid_bit || show_fsmonitor_bit) {
tag_cached = "H ";
diff --git a/dir.c b/dir.c
index 4128d59d7a..b522d61ee0 100644
--- a/dir.c
+++ b/dir.c
@@ -600,7 +600,7 @@ void parse_exclude_pattern(const char **pattern,
}
void add_exclude(const char *string, const char *base,
- int baselen, struct exclude_list *el, int srcpos)
+ int baselen, struct pattern_list *pl, int srcpos)
{
struct path_pattern *pattern;
int patternlen;
@@ -620,9 +620,9 @@ void add_exclude(const char *string, const char *base,
pattern->baselen = baselen;
pattern->flags = flags;
pattern->srcpos = srcpos;
- ALLOC_GROW(el->patterns, el->nr + 1, el->alloc);
- el->patterns[el->nr++] = pattern;
- pattern->el = el;
+ ALLOC_GROW(pl->patterns, pl->nr + 1, pl->alloc);
+ pl->patterns[pl->nr++] = pattern;
+ pattern->pl = pl;
}
static int read_skip_worktree_file_from_index(const struct index_state *istate,
@@ -643,19 +643,19 @@ static int read_skip_worktree_file_from_index(const struct index_state *istate,
}
/*
- * Frees memory within el which was allocated for exclude patterns and
- * the file buffer. Does not free el itself.
+ * Frees memory within pl which was allocated for exclude patterns and
+ * the file buffer. Does not free pl itself.
*/
-void clear_exclude_list(struct exclude_list *el)
+void clear_exclude_list(struct pattern_list *pl)
{
int i;
- for (i = 0; i < el->nr; i++)
- free(el->patterns[i]);
- free(el->patterns);
- free(el->filebuf);
+ for (i = 0; i < pl->nr; i++)
+ free(pl->patterns[i]);
+ free(pl->patterns);
+ free(pl->filebuf);
- memset(el, 0, sizeof(*el));
+ memset(pl, 0, sizeof(*pl));
}
static void trim_trailing_spaces(char *buf)
@@ -764,19 +764,19 @@ static void invalidate_directory(struct untracked_cache *uc,
static int add_excludes_from_buffer(char *buf, size_t size,
const char *base, int baselen,
- struct exclude_list *el);
+ struct pattern_list *pl);
/*
* Given a file with name "fname", read it (either from disk, or from
* an index if 'istate' is non-null), parse it and store the
- * exclude rules in "el".
+ * exclude rules in "pl".
*
* If "ss" is not NULL, compute SHA-1 of the exclude file and fill
* stat data from disk (only valid if add_excludes returns zero). If
* ss_valid is non-zero, "ss" must contain good value as input.
*/
static int add_excludes(const char *fname, const char *base, int baselen,
- struct exclude_list *el, struct index_state *istate,
+ struct pattern_list *pl, struct index_state *istate,
struct oid_stat *oid_stat)
{
struct stat st;
@@ -837,21 +837,21 @@ static int add_excludes(const char *fname, const char *base, int baselen,
}
}
- add_excludes_from_buffer(buf, size, base, baselen, el);
+ add_excludes_from_buffer(buf, size, base, baselen, pl);
return 0;
}
static int add_excludes_from_buffer(char *buf, size_t size,
const char *base, int baselen,
- struct exclude_list *el)
+ struct pattern_list *pl)
{
int i, lineno = 1;
char *entry;
- el->filebuf = buf;
+ pl->filebuf = buf;
if (skip_utf8_bom(&buf, size))
- size -= buf - el->filebuf;
+ size -= buf - pl->filebuf;
entry = buf;
@@ -860,7 +860,7 @@ static int add_excludes_from_buffer(char *buf, size_t size,
if (entry != buf + i && entry[0] != '#') {
buf[i - (i && buf[i-1] == '\r')] = 0;
trim_trailing_spaces(entry);
- add_exclude(entry, base, baselen, el, lineno);
+ add_exclude(entry, base, baselen, pl, lineno);
}
lineno++;
entry = buf + i + 1;
@@ -870,16 +870,16 @@ static int add_excludes_from_buffer(char *buf, size_t size,
}
int add_excludes_from_file_to_list(const char *fname, const char *base,
- int baselen, struct exclude_list *el,
+ int baselen, struct pattern_list *pl,
struct index_state *istate)
{
- return add_excludes(fname, base, baselen, el, istate, NULL);
+ return add_excludes(fname, base, baselen, pl, istate, NULL);
}
int add_excludes_from_blob_to_list(
struct object_id *oid,
const char *base, int baselen,
- struct exclude_list *el)
+ struct pattern_list *pl)
{
char *buf;
size_t size;
@@ -889,22 +889,22 @@ int add_excludes_from_blob_to_list(
if (r != 1)
return r;
- add_excludes_from_buffer(buf, size, base, baselen, el);
+ add_excludes_from_buffer(buf, size, base, baselen, pl);
return 0;
}
-struct exclude_list *add_exclude_list(struct dir_struct *dir,
+struct pattern_list *add_exclude_list(struct dir_struct *dir,
int group_type, const char *src)
{
- struct exclude_list *el;
+ struct pattern_list *pl;
struct exclude_list_group *group;
group = &dir->exclude_list_group[group_type];
- ALLOC_GROW(group->el, group->nr + 1, group->alloc);
- el = &group->el[group->nr++];
- memset(el, 0, sizeof(*el));
- el->src = src;
- return el;
+ ALLOC_GROW(group->pl, group->nr + 1, group->alloc);
+ pl = &group->pl[group->nr++];
+ memset(pl, 0, sizeof(*pl));
+ pl->src = src;
+ return pl;
}
/*
@@ -913,7 +913,7 @@ struct exclude_list *add_exclude_list(struct dir_struct *dir,
static void add_excludes_from_file_1(struct dir_struct *dir, const char *fname,
struct oid_stat *oid_stat)
{
- struct exclude_list *el;
+ struct pattern_list *pl;
/*
* catch setup_standard_excludes() that's called before
* dir->untracked is assigned. That function behaves
@@ -921,8 +921,8 @@ static void add_excludes_from_file_1(struct dir_struct *dir, const char *fname,
*/
if (!dir->untracked)
dir->unmanaged_exclude_files++;
- el = add_exclude_list(dir, EXC_FILE, fname);
- if (add_excludes(fname, "", 0, el, NULL, oid_stat) < 0)
+ pl = add_exclude_list(dir, EXC_FILE, fname);
+ if (add_excludes(fname, "", 0, pl, NULL, oid_stat) < 0)
die(_("cannot use %s as an exclude file"), fname);
}
@@ -1025,17 +1025,17 @@ static struct path_pattern *last_exclude_matching_from_list(const char *pathname
int pathlen,
const char *basename,
int *dtype,
- struct exclude_list *el,
+ struct pattern_list *pl,
struct index_state *istate)
{
struct path_pattern *res = NULL; /* undecided */
int i;
- if (!el->nr)
+ if (!pl->nr)
return NULL; /* undefined */
- for (i = el->nr - 1; 0 <= i; i--) {
- struct path_pattern *pattern = el->patterns[i];
+ for (i = pl->nr - 1; 0 <= i; i--) {
+ struct path_pattern *pattern = pl->patterns[i];
const char *exclude = pattern->pattern;
int prefix = pattern->nowildcardlen;
@@ -1077,11 +1077,11 @@ static struct path_pattern *last_exclude_matching_from_list(const char *pathname
*/
int is_excluded_from_list(const char *pathname,
int pathlen, const char *basename, int *dtype,
- struct exclude_list *el, struct index_state *istate)
+ struct pattern_list *pl, struct index_state *istate)
{
struct path_pattern *pattern;
pattern = last_exclude_matching_from_list(pathname, pathlen, basename,
- dtype, el, istate);
+ dtype, pl, istate);
if (pattern)
return pattern->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
return -1; /* undecided */
@@ -1100,7 +1100,7 @@ static struct path_pattern *last_exclude_matching_from_lists(
for (j = group->nr - 1; j >= 0; j--) {
pattern = last_exclude_matching_from_list(
pathname, pathlen, basename, dtype_p,
- &group->el[j], istate);
+ &group->pl[j], istate);
if (pattern)
return pattern;
}
@@ -1117,7 +1117,7 @@ static void prep_exclude(struct dir_struct *dir,
const char *base, int baselen)
{
struct exclude_list_group *group;
- struct exclude_list *el;
+ struct pattern_list *pl;
struct exclude_stack *stk = NULL;
struct untracked_cache_dir *untracked;
int current;
@@ -1133,11 +1133,11 @@ static void prep_exclude(struct dir_struct *dir,
if (stk->baselen <= baselen &&
!strncmp(dir->basebuf.buf, base, stk->baselen))
break;
- el = &group->el[dir->exclude_stack->exclude_ix];
+ pl = &group->pl[dir->exclude_stack->exclude_ix];
dir->exclude_stack = stk->prev;
dir->pattern = NULL;
- free((char *)el->src); /* see strbuf_detach() below */
- clear_exclude_list(el);
+ free((char *)pl->src); /* see strbuf_detach() below */
+ clear_exclude_list(pl);
free(stk);
group->nr--;
}
@@ -1184,7 +1184,7 @@ static void prep_exclude(struct dir_struct *dir,
stk->baselen = cp - base;
stk->exclude_ix = group->nr;
stk->ucd = untracked;
- el = add_exclude_list(dir, EXC_DIRS, NULL);
+ pl = add_exclude_list(dir, EXC_DIRS, NULL);
strbuf_add(&dir->basebuf, base + current, stk->baselen - current);
assert(stk->baselen == dir->basebuf.len);
@@ -1234,8 +1234,8 @@ static void prep_exclude(struct dir_struct *dir,
struct strbuf sb = STRBUF_INIT;
strbuf_addbuf(&sb, &dir->basebuf);
strbuf_addstr(&sb, dir->exclude_per_dir);
- el->src = strbuf_detach(&sb, NULL);
- add_excludes(el->src, el->src, stk->baselen, el, istate,
+ pl->src = strbuf_detach(&sb, NULL);
+ add_excludes(pl->src, pl->src, stk->baselen, pl, istate,
untracked ? &oid_stat : NULL);
}
/*
@@ -2530,18 +2530,18 @@ void clear_directory(struct dir_struct *dir)
{
int i, j;
struct exclude_list_group *group;
- struct exclude_list *el;
+ struct pattern_list *pl;
struct exclude_stack *stk;
for (i = EXC_CMDL; i <= EXC_FILE; i++) {
group = &dir->exclude_list_group[i];
for (j = 0; j < group->nr; j++) {
- el = &group->el[j];
+ pl = &group->pl[j];
if (i == EXC_DIRS)
- free((char *)el->src);
- clear_exclude_list(el);
+ free((char *)pl->src);
+ clear_exclude_list(pl);
}
- free(group->el);
+ free(group->pl);
}
stk = dir->exclude_stack;
diff --git a/dir.h b/dir.h
index e8b90fc482..4114d6bf78 100644
--- a/dir.h
+++ b/dir.h
@@ -21,7 +21,7 @@ struct path_pattern {
* This allows callers of last_exclude_matching() etc.
* to determine the origin of the matching pattern.
*/
- struct exclude_list *el;
+ struct pattern_list *pl;
const char *pattern;
int patternlen;
@@ -44,7 +44,7 @@ struct path_pattern {
* can also be used to represent the list of --exclude values passed
* via CLI args.
*/
-struct exclude_list {
+struct pattern_list {
int nr;
int alloc;
@@ -72,7 +72,7 @@ struct exclude_stack {
struct exclude_list_group {
int nr, alloc;
- struct exclude_list *el;
+ struct pattern_list *pl;
};
struct oid_stat {
@@ -232,7 +232,7 @@ int read_directory(struct dir_struct *, struct index_state *istate,
int is_excluded_from_list(const char *pathname, int pathlen,
const char *basename, int *dtype,
- struct exclude_list *el,
+ struct pattern_list *pl,
struct index_state *istate);
struct dir_entry *dir_add_ignored(struct dir_struct *dir,
struct index_state *istate,
@@ -256,18 +256,18 @@ int is_excluded(struct dir_struct *dir,
struct index_state *istate,
const char *name, int *dtype);
-struct exclude_list *add_exclude_list(struct dir_struct *dir,
+struct pattern_list *add_exclude_list(struct dir_struct *dir,
int group_type, const char *src);
int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,
- struct exclude_list *el, struct index_state *istate);
+ struct pattern_list *pl, struct index_state *istate);
void add_excludes_from_file(struct dir_struct *, const char *fname);
int add_excludes_from_blob_to_list(struct object_id *oid,
const char *base, int baselen,
- struct exclude_list *el);
+ struct pattern_list *pl);
void parse_exclude_pattern(const char **string, int *patternlen, unsigned *flags, int *nowildcardlen);
void add_exclude(const char *string, const char *base,
- int baselen, struct exclude_list *el, int srcpos);
-void clear_exclude_list(struct exclude_list *el);
+ int baselen, struct pattern_list *pl, int srcpos);
+void clear_exclude_list(struct pattern_list *pl);
void clear_directory(struct dir_struct *dir);
int repo_file_exists(struct repository *repo, const char *path);
diff --git a/list-objects-filter.c b/list-objects-filter.c
index d664264d65..a1fedf8bd8 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -347,7 +347,7 @@ struct frame {
};
struct filter_sparse_data {
- struct exclude_list el;
+ struct pattern_list pl;
size_t nr, alloc;
struct frame *array_frame;
@@ -374,7 +374,7 @@ static enum list_objects_filter_result filter_sparse(
assert(obj->type == OBJ_TREE);
dtype = DT_DIR;
val = is_excluded_from_list(pathname, strlen(pathname),
- filename, &dtype, &filter_data->el,
+ filename, &dtype, &filter_data->pl,
r->index);
if (val < 0)
val = filter_data->array_frame[filter_data->nr - 1].defval;
@@ -436,7 +436,7 @@ static enum list_objects_filter_result filter_sparse(
dtype = DT_REG;
val = is_excluded_from_list(pathname, strlen(pathname),
- filename, &dtype, &filter_data->el,
+ filename, &dtype, &filter_data->pl,
r->index);
if (val < 0)
val = frame->defval;
@@ -483,7 +483,7 @@ static void filter_sparse_oid__init(
{
struct filter_sparse_data *d = xcalloc(1, sizeof(*d));
if (add_excludes_from_blob_to_list(filter_options->sparse_oid_value,
- NULL, 0, &d->el) < 0)
+ NULL, 0, &d->pl) < 0)
die("could not load filter specification");
ALLOC_GROW(d->array_frame, d->nr + 1, d->alloc);
diff --git a/unpack-trees.c b/unpack-trees.c
index 50189909b8..c4dc21affb 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1265,7 +1265,7 @@ static int clear_ce_flags_1(struct index_state *istate,
struct cache_entry **cache, int nr,
struct strbuf *prefix,
int select_mask, int clear_mask,
- struct exclude_list *el, int defval);
+ struct pattern_list *pl, int defval);
/* Whole directory matching */
static int clear_ce_flags_dir(struct index_state *istate,
@@ -1273,12 +1273,12 @@ static int clear_ce_flags_dir(struct index_state *istate,
struct strbuf *prefix,
char *basename,
int select_mask, int clear_mask,
- struct exclude_list *el, int defval)
+ struct pattern_list *pl, int defval)
{
struct cache_entry **cache_end;
int dtype = DT_DIR;
int ret = is_excluded_from_list(prefix->buf, prefix->len,
- basename, &dtype, el, istate);
+ basename, &dtype, pl, istate);
int rc;
strbuf_addch(prefix, '/');
@@ -1294,7 +1294,7 @@ static int clear_ce_flags_dir(struct index_state *istate,
}
/*
- * TODO: check el, if there are no patterns that may conflict
+ * TODO: check pl, if there are no patterns that may conflict
* with ret (iow, we know in advance the incl/excl
* decision for the entire directory), clear flag here without
* calling clear_ce_flags_1(). That function will call
@@ -1303,14 +1303,14 @@ static int clear_ce_flags_dir(struct index_state *istate,
rc = clear_ce_flags_1(istate, cache, cache_end - cache,
prefix,
select_mask, clear_mask,
- el, ret);
+ pl, ret);
strbuf_setlen(prefix, prefix->len - 1);
return rc;
}
/*
* Traverse the index, find every entry that matches according to
- * o->el. Do "ce_flags &= ~clear_mask" on those entries. Return the
+ * o->pl. Do "ce_flags &= ~clear_mask" on those entries. Return the
* number of traversed entries.
*
* If select_mask is non-zero, only entries whose ce_flags has on of
@@ -1327,7 +1327,7 @@ static int clear_ce_flags_1(struct index_state *istate,
struct cache_entry **cache, int nr,
struct strbuf *prefix,
int select_mask, int clear_mask,
- struct exclude_list *el, int defval)
+ struct pattern_list *pl, int defval)
{
struct cache_entry **cache_end = cache + nr;
@@ -1362,7 +1362,7 @@ static int clear_ce_flags_1(struct index_state *istate,
prefix,
prefix->buf + prefix->len - len,
select_mask, clear_mask,
- el, defval);
+ pl, defval);
/* clear_c_f_dir eats a whole dir already? */
if (processed) {
@@ -1374,7 +1374,7 @@ static int clear_ce_flags_1(struct index_state *istate,
strbuf_addch(prefix, '/');
cache += clear_ce_flags_1(istate, cache, cache_end - cache,
prefix,
- select_mask, clear_mask, el, defval);
+ select_mask, clear_mask, pl, defval);
strbuf_setlen(prefix, prefix->len - len - 1);
continue;
}
@@ -1382,7 +1382,7 @@ static int clear_ce_flags_1(struct index_state *istate,
/* Non-directory */
dtype = ce_to_dtype(ce);
ret = is_excluded_from_list(ce->name, ce_namelen(ce),
- name, &dtype, el, istate);
+ name, &dtype, pl, istate);
if (ret < 0)
ret = defval;
if (ret > 0)
@@ -1394,7 +1394,7 @@ static int clear_ce_flags_1(struct index_state *istate,
static int clear_ce_flags(struct index_state *istate,
int select_mask, int clear_mask,
- struct exclude_list *el)
+ struct pattern_list *pl)
{
static struct strbuf prefix = STRBUF_INIT;
@@ -1405,13 +1405,13 @@ static int clear_ce_flags(struct index_state *istate,
istate->cache_nr,
&prefix,
select_mask, clear_mask,
- el, 0);
+ pl, 0);
}
/*
* Set/Clear CE_NEW_SKIP_WORKTREE according to $GIT_DIR/info/sparse-checkout
*/
-static void mark_new_skip_worktree(struct exclude_list *el,
+static void mark_new_skip_worktree(struct pattern_list *pl,
struct index_state *istate,
int select_flag, int skip_wt_flag)
{
@@ -1437,7 +1437,7 @@ static void mark_new_skip_worktree(struct exclude_list *el,
* 2. Widen worktree according to sparse-checkout file.
* Matched entries will have skip_wt_flag cleared (i.e. "in")
*/
- clear_ce_flags(istate, select_flag, skip_wt_flag, el);
+ clear_ce_flags(istate, select_flag, skip_wt_flag, pl);
}
static int verify_absent(const struct cache_entry *,
@@ -1453,21 +1453,21 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
{
int i, ret;
static struct cache_entry *dfc;
- struct exclude_list el;
+ struct pattern_list pl;
if (len > MAX_UNPACK_TREES)
die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
trace_performance_enter();
- memset(&el, 0, sizeof(el));
+ memset(&pl, 0, sizeof(pl));
if (!core_apply_sparse_checkout || !o->update)
o->skip_sparse_checkout = 1;
if (!o->skip_sparse_checkout) {
char *sparse = git_pathdup("info/sparse-checkout");
- if (add_excludes_from_file_to_list(sparse, "", 0, &el, NULL) < 0)
+ if (add_excludes_from_file_to_list(sparse, "", 0, &pl, NULL) < 0)
o->skip_sparse_checkout = 1;
else
- o->el = ⪙
+ o->pl = &pl;
free(sparse);
}
@@ -1498,7 +1498,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
* Sparse checkout loop #1: set NEW_SKIP_WORKTREE on existing entries
*/
if (!o->skip_sparse_checkout)
- mark_new_skip_worktree(o->el, o->src_index, 0, CE_NEW_SKIP_WORKTREE);
+ mark_new_skip_worktree(o->pl, o->src_index, 0, CE_NEW_SKIP_WORKTREE);
if (!dfc)
dfc = xcalloc(1, cache_entry_size(0));
@@ -1563,7 +1563,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
* If the will have NEW_SKIP_WORKTREE, also set CE_SKIP_WORKTREE
* so apply_sparse_checkout() won't attempt to remove it from worktree
*/
- mark_new_skip_worktree(o->el, &o->result, CE_ADDED, CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);
+ mark_new_skip_worktree(o->pl, &o->result, CE_ADDED, CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);
ret = 0;
for (i = 0; i < o->result.cache_nr; i++) {
@@ -1631,7 +1631,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
done:
trace_performance_leave("unpack_trees");
- clear_exclude_list(&el);
+ clear_exclude_list(&pl);
return ret;
return_failed:
diff --git a/unpack-trees.h b/unpack-trees.h
index d344d7d296..f2eee0c7c5 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -10,7 +10,7 @@
struct cache_entry;
struct unpack_trees_options;
-struct exclude_list;
+struct pattern_list;
typedef int (*merge_fn_t)(const struct cache_entry * const *src,
struct unpack_trees_options *options);
@@ -83,7 +83,7 @@ struct unpack_trees_options {
struct index_state *src_index;
struct index_state result;
- struct exclude_list *el; /* for internal use */
+ struct pattern_list *pl; /* for internal use */
};
int unpack_trees(unsigned n, struct tree_desc *t,
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] treewide: rename 'EXCL_FLAG_' to 'PATTERN_FLAG_'
2019-09-03 18:04 [PATCH 0/5] Refactor excludes library Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 1/5] treewide: rename 'struct exclude' to 'struct path_pattern' Derrick Stolee via GitGitGadget
@ 2019-09-03 18:04 ` Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 2/5] treewide: rename 'struct exclude_list' to 'struct pattern_list' Derrick Stolee via GitGitGadget
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2019-09-03 18:04 UTC (permalink / raw)
To: git; +Cc: newren, pclouds, peff, jon, matvore, Junio C Hamano,
Derrick Stolee
From: Derrick Stolee <dstolee@microsoft.com>
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!
It would be clearer to rename this entire library as a "pattern
matching" library, and the callers apply exclusion/inclusion
logic accordingly based on their needs.
This commit replaces 'EXCL_FLAG_' to 'PATTERN_FLAG_' in the
names of the flags used on 'struct path_pattern'.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
attr.c | 8 ++++----
builtin/check-ignore.c | 4 ++--
dir.c | 22 +++++++++++-----------
dir.h | 10 +++++-----
4 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/attr.c b/attr.c
index 93dc16b59c..d90239869a 100644
--- a/attr.c
+++ b/attr.c
@@ -259,7 +259,7 @@ struct pattern {
const char *pattern;
int patternlen;
int nowildcardlen;
- unsigned flags; /* EXC_FLAG_* */
+ unsigned flags; /* PATTERN_FLAG_* */
};
/*
@@ -404,7 +404,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
&res->u.pat.patternlen,
&res->u.pat.flags,
&res->u.pat.nowildcardlen);
- if (res->u.pat.flags & EXC_FLAG_NEGATIVE) {
+ if (res->u.pat.flags & PATTERN_FLAG_NEGATIVE) {
warning(_("Negative patterns are ignored in git attributes\n"
"Use '\\!' for literal leading exclamation."));
goto fail_return;
@@ -991,10 +991,10 @@ static int path_matches(const char *pathname, int pathlen,
int prefix = pat->nowildcardlen;
int isdir = (pathlen && pathname[pathlen - 1] == '/');
- if ((pat->flags & EXC_FLAG_MUSTBEDIR) && !isdir)
+ if ((pat->flags & PATTERN_FLAG_MUSTBEDIR) && !isdir)
return 0;
- if (pat->flags & EXC_FLAG_NODIR) {
+ if (pat->flags & PATTERN_FLAG_NODIR) {
return match_basename(pathname + basename_offset,
pathlen - basename_offset - isdir,
pattern, prefix,
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 97108ccb9c..28b8f14999 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -34,8 +34,8 @@ static const struct option check_ignore_options[] = {
static void output_pattern(const char *path, struct path_pattern *pattern)
{
- char *bang = (pattern && pattern->flags & EXC_FLAG_NEGATIVE) ? "!" : "";
- char *slash = (pattern && pattern->flags & EXC_FLAG_MUSTBEDIR) ? "/" : "";
+ char *bang = (pattern && pattern->flags & PATTERN_FLAG_NEGATIVE) ? "!" : "";
+ char *slash = (pattern && pattern->flags & PATTERN_FLAG_MUSTBEDIR) ? "/" : "";
if (!nul_term_line) {
if (!verbose) {
write_name_quoted(path, stdout, '\n');
diff --git a/dir.c b/dir.c
index b522d61ee0..640f10973e 100644
--- a/dir.c
+++ b/dir.c
@@ -571,20 +571,20 @@ void parse_exclude_pattern(const char **pattern,
*flags = 0;
if (*p == '!') {
- *flags |= EXC_FLAG_NEGATIVE;
+ *flags |= PATTERN_FLAG_NEGATIVE;
p++;
}
len = strlen(p);
if (len && p[len - 1] == '/') {
len--;
- *flags |= EXC_FLAG_MUSTBEDIR;
+ *flags |= PATTERN_FLAG_MUSTBEDIR;
}
for (i = 0; i < len; i++) {
if (p[i] == '/')
break;
}
if (i == len)
- *flags |= EXC_FLAG_NODIR;
+ *flags |= PATTERN_FLAG_NODIR;
*nowildcardlen = simple_length(p);
/*
* we should have excluded the trailing slash from 'p' too,
@@ -594,7 +594,7 @@ void parse_exclude_pattern(const char **pattern,
if (*nowildcardlen > len)
*nowildcardlen = len;
if (*p == '*' && no_wildcard(p + 1))
- *flags |= EXC_FLAG_ENDSWITH;
+ *flags |= PATTERN_FLAG_ENDSWITH;
*pattern = p;
*patternlen = len;
}
@@ -608,7 +608,7 @@ void add_exclude(const char *string, const char *base,
int nowildcardlen;
parse_exclude_pattern(&string, &patternlen, &flags, &nowildcardlen);
- if (flags & EXC_FLAG_MUSTBEDIR) {
+ if (flags & PATTERN_FLAG_MUSTBEDIR) {
FLEXPTR_ALLOC_MEM(pattern, pattern, string, patternlen);
} else {
pattern = xmalloc(sizeof(*pattern));
@@ -940,7 +940,7 @@ int match_basename(const char *basename, int basenamelen,
if (patternlen == basenamelen &&
!fspathncmp(pattern, basename, basenamelen))
return 1;
- } else if (flags & EXC_FLAG_ENDSWITH) {
+ } else if (flags & PATTERN_FLAG_ENDSWITH) {
/* "*literal" matching against "fooliteral" */
if (patternlen - 1 <= basenamelen &&
!fspathncmp(pattern + 1,
@@ -1039,14 +1039,14 @@ static struct path_pattern *last_exclude_matching_from_list(const char *pathname
const char *exclude = pattern->pattern;
int prefix = pattern->nowildcardlen;
- if (pattern->flags & EXC_FLAG_MUSTBEDIR) {
+ if (pattern->flags & PATTERN_FLAG_MUSTBEDIR) {
if (*dtype == DT_UNKNOWN)
*dtype = get_dtype(NULL, istate, pathname, pathlen);
if (*dtype != DT_DIR)
continue;
}
- if (pattern->flags & EXC_FLAG_NODIR) {
+ if (pattern->flags & PATTERN_FLAG_NODIR) {
if (match_basename(basename,
pathlen - (basename - pathname),
exclude, prefix, pattern->patternlen,
@@ -1083,7 +1083,7 @@ int is_excluded_from_list(const char *pathname,
pattern = last_exclude_matching_from_list(pathname, pathlen, basename,
dtype, pl, istate);
if (pattern)
- return pattern->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
+ return pattern->flags & PATTERN_FLAG_NEGATIVE ? 0 : 1;
return -1; /* undecided */
}
@@ -1198,7 +1198,7 @@ static void prep_exclude(struct dir_struct *dir,
dir->basebuf.buf + current, &dt);
dir->basebuf.buf[stk->baselen - 1] = '/';
if (dir->pattern &&
- dir->pattern->flags & EXC_FLAG_NEGATIVE)
+ dir->pattern->flags & PATTERN_FLAG_NEGATIVE)
dir->pattern = NULL;
if (dir->pattern) {
dir->exclude_stack = stk;
@@ -1298,7 +1298,7 @@ int is_excluded(struct dir_struct *dir, struct index_state *istate,
struct path_pattern *pattern =
last_exclude_matching(dir, istate, pathname, dtype_p);
if (pattern)
- return pattern->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
+ return pattern->flags & PATTERN_FLAG_NEGATIVE ? 0 : 1;
return 0;
}
diff --git a/dir.h b/dir.h
index 4114d6bf78..87eb10662f 100644
--- a/dir.h
+++ b/dir.h
@@ -11,10 +11,10 @@ struct dir_entry {
char name[FLEX_ARRAY]; /* more */
};
-#define EXC_FLAG_NODIR 1
-#define EXC_FLAG_ENDSWITH 4
-#define EXC_FLAG_MUSTBEDIR 8
-#define EXC_FLAG_NEGATIVE 16
+#define PATTERN_FLAG_NODIR 1
+#define PATTERN_FLAG_ENDSWITH 4
+#define PATTERN_FLAG_MUSTBEDIR 8
+#define PATTERN_FLAG_NEGATIVE 16
struct path_pattern {
/*
@@ -28,7 +28,7 @@ struct path_pattern {
int nowildcardlen;
const char *base;
int baselen;
- unsigned flags; /* EXC_FLAG_* */
+ unsigned flags; /* PATTERN_FLAG_* */
/*
* Counting starts from 1 for line numbers in ignore files,
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/5] treewide: rename 'exclude' methods to 'pattern'
2019-09-03 18:04 [PATCH 0/5] Refactor excludes library Derrick Stolee via GitGitGadget
` (2 preceding siblings ...)
2019-09-03 18:04 ` [PATCH 2/5] treewide: rename 'struct exclude_list' to 'struct pattern_list' Derrick Stolee via GitGitGadget
@ 2019-09-03 18:04 ` Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 5/5] unpack-trees: rename 'is_excluded_from_list()' Derrick Stolee via GitGitGadget
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2019-09-03 18:04 UTC (permalink / raw)
To: git; +Cc: newren, pclouds, peff, jon, matvore, Junio C Hamano,
Derrick Stolee
From: Derrick Stolee <dstolee@microsoft.com>
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!
It would be clearer to rename this entire library as a "pattern
matching" library, and the callers apply exclusion/inclusion
logic accordingly based on their needs.
This commit renames several methods defined in dir.h to make
more sense with the renamed 'struct exclude_list' to 'struct
pattern_list' and 'struct exclude' to 'struct path_pattern':
* last_exclude_matching() -> last_matching_pattern()
* parse_exclude() -> parse_path_pattern()
In addition, the word 'exclude' was replaced with 'pattern'
in the methods below:
* add_exclude_list()
* add_excludes_from_file_to_list()
* add_excludes_from_file()
* add_excludes_from_blob_to_list()
* add_exclude()
* clear_exclude_list()
A few methods with the word "exclude" remain. These will
be handled seperately. In particular, the method
"is_excluded()" is concretely about the .gitignore file
relative to a specific directory. This is the important
boundary between library and consumer: is_excluded() cares
about .gitignore, but is_excluded() calls
last_matching_pattern() to make that decision.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/RelNotes/2.7.1.txt | 2 +-
Documentation/RelNotes/2.8.0.txt | 2 +-
.../technical/api-directory-listing.txt | 6 +-
attr.c | 2 +-
builtin/check-ignore.c | 2 +-
builtin/clean.c | 8 +-
builtin/ls-files.c | 6 +-
dir.c | 78 +++++++++----------
dir.h | 22 +++---
list-objects-filter.c | 2 +-
unpack-trees.c | 4 +-
11 files changed, 67 insertions(+), 67 deletions(-)
diff --git a/Documentation/RelNotes/2.7.1.txt b/Documentation/RelNotes/2.7.1.txt
index 6553d69e33..6323feaf64 100644
--- a/Documentation/RelNotes/2.7.1.txt
+++ b/Documentation/RelNotes/2.7.1.txt
@@ -10,7 +10,7 @@ Fixes since v2.7
setting GIT_WORK_TREE environment themselves.
* The "exclude_list" structure has the usual "alloc, nr" pair of
- fields to be used by ALLOC_GROW(), but clear_exclude_list() forgot
+ fields to be used by ALLOC_GROW(), but clear_pattern_list() forgot
to reset 'alloc' to 0 when it cleared 'nr' to discard the managed
array.
diff --git a/Documentation/RelNotes/2.8.0.txt b/Documentation/RelNotes/2.8.0.txt
index 25079710fa..5fbe1b86ee 100644
--- a/Documentation/RelNotes/2.8.0.txt
+++ b/Documentation/RelNotes/2.8.0.txt
@@ -270,7 +270,7 @@ notes for details).
setting GIT_WORK_TREE environment themselves.
* The "exclude_list" structure has the usual "alloc, nr" pair of
- fields to be used by ALLOC_GROW(), but clear_exclude_list() forgot
+ fields to be used by ALLOC_GROW(), but clear_pattern_list() forgot
to reset 'alloc' to 0 when it cleared 'nr' to discard the managed
array.
diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
index 5abb8e8b1f..76b6e4f71b 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -111,11 +111,11 @@ marked. If you to exclude files, make sure you have loaded index first.
* Prepare `struct dir_struct dir` and clear it with `memset(&dir, 0,
sizeof(dir))`.
-* To add single exclude pattern, call `add_exclude_list()` and then
- `add_exclude()`.
+* To add single exclude pattern, call `add_pattern_list()` and then
+ `add_pattern()`.
* To add patterns from a file (e.g. `.git/info/exclude`), call
- `add_excludes_from_file()` , and/or set `dir.exclude_per_dir`. A
+ `add_patterns_from_file()` , and/or set `dir.exclude_per_dir`. A
short-hand function `setup_standard_excludes()` can be used to set
up the standard set of exclude settings.
diff --git a/attr.c b/attr.c
index d90239869a..d02d081e28 100644
--- a/attr.c
+++ b/attr.c
@@ -400,7 +400,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
char *p = (char *)&(res->state[num_attr]);
memcpy(p, name, namelen);
res->u.pat.pattern = p;
- parse_exclude_pattern(&res->u.pat.pattern,
+ parse_path_pattern(&res->u.pat.pattern,
&res->u.pat.patternlen,
&res->u.pat.flags,
&res->u.pat.nowildcardlen);
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 28b8f14999..5a4f92395f 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -106,7 +106,7 @@ static int check_ignore(struct dir_struct *dir,
pattern = NULL;
if (!seen[i]) {
int dtype = DT_UNKNOWN;
- pattern = last_exclude_matching(dir, &the_index,
+ pattern = last_matching_pattern(dir, &the_index,
full_path, &dtype);
}
if (!quiet && (pattern || show_non_matching))
diff --git a/builtin/clean.c b/builtin/clean.c
index d8c847d9fd..4920c92593 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -670,7 +670,7 @@ static int filter_by_patterns_cmd(void)
break;
memset(&dir, 0, sizeof(dir));
- pl = add_exclude_list(&dir, EXC_CMDL, "manual exclude");
+ pl = add_pattern_list(&dir, EXC_CMDL, "manual exclude");
ignore_list = strbuf_split_max(&confirm, ' ', 0);
for (i = 0; ignore_list[i]; i++) {
@@ -678,7 +678,7 @@ static int filter_by_patterns_cmd(void)
if (!ignore_list[i]->len)
continue;
- add_exclude(ignore_list[i]->buf, "", 0, pl, -(i+1));
+ add_pattern(ignore_list[i]->buf, "", 0, pl, -(i+1));
}
changed = 0;
@@ -957,9 +957,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
if (!ignored)
setup_standard_excludes(&dir);
- pl = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
+ pl = add_pattern_list(&dir, EXC_CMDL, "--exclude option");
for (i = 0; i < exclude_list.nr; i++)
- add_exclude(exclude_list.items[i].string, "", 0, pl, -(i+1));
+ add_pattern(exclude_list.items[i].string, "", 0, pl, -(i+1));
parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_CWD,
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index df8918a128..5acc396949 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -492,7 +492,7 @@ static int option_parse_exclude_from(const struct option *opt,
BUG_ON_OPT_NEG(unset);
exc_given = 1;
- add_excludes_from_file(dir, arg);
+ add_patterns_from_file(dir, arg);
return 0;
}
@@ -594,9 +594,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
- pl = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
+ pl = add_pattern_list(&dir, EXC_CMDL, "--exclude option");
for (i = 0; i < exclude_list.nr; i++) {
- add_exclude(exclude_list.items[i].string, "", 0, pl, --exclude_args);
+ add_pattern(exclude_list.items[i].string, "", 0, pl, --exclude_args);
}
if (show_tag || show_valid_bit || show_fsmonitor_bit) {
tag_cached = "H ";
diff --git a/dir.c b/dir.c
index 640f10973e..b057bd3d95 100644
--- a/dir.c
+++ b/dir.c
@@ -561,7 +561,7 @@ int no_wildcard(const char *string)
return string[simple_length(string)] == '\0';
}
-void parse_exclude_pattern(const char **pattern,
+void parse_path_pattern(const char **pattern,
int *patternlen,
unsigned *flags,
int *nowildcardlen)
@@ -599,7 +599,7 @@ void parse_exclude_pattern(const char **pattern,
*patternlen = len;
}
-void add_exclude(const char *string, const char *base,
+void add_pattern(const char *string, const char *base,
int baselen, struct pattern_list *pl, int srcpos)
{
struct path_pattern *pattern;
@@ -607,7 +607,7 @@ void add_exclude(const char *string, const char *base,
unsigned flags;
int nowildcardlen;
- parse_exclude_pattern(&string, &patternlen, &flags, &nowildcardlen);
+ parse_path_pattern(&string, &patternlen, &flags, &nowildcardlen);
if (flags & PATTERN_FLAG_MUSTBEDIR) {
FLEXPTR_ALLOC_MEM(pattern, pattern, string, patternlen);
} else {
@@ -646,7 +646,7 @@ static int read_skip_worktree_file_from_index(const struct index_state *istate,
* Frees memory within pl which was allocated for exclude patterns and
* the file buffer. Does not free pl itself.
*/
-void clear_exclude_list(struct pattern_list *pl)
+void clear_pattern_list(struct pattern_list *pl)
{
int i;
@@ -762,7 +762,7 @@ static void invalidate_directory(struct untracked_cache *uc,
dir->dirs[i]->recurse = 0;
}
-static int add_excludes_from_buffer(char *buf, size_t size,
+static int add_patterns_from_buffer(char *buf, size_t size,
const char *base, int baselen,
struct pattern_list *pl);
@@ -772,10 +772,10 @@ static int add_excludes_from_buffer(char *buf, size_t size,
* exclude rules in "pl".
*
* If "ss" is not NULL, compute SHA-1 of the exclude file and fill
- * stat data from disk (only valid if add_excludes returns zero). If
+ * stat data from disk (only valid if add_patterns returns zero). If
* ss_valid is non-zero, "ss" must contain good value as input.
*/
-static int add_excludes(const char *fname, const char *base, int baselen,
+static int add_patterns(const char *fname, const char *base, int baselen,
struct pattern_list *pl, struct index_state *istate,
struct oid_stat *oid_stat)
{
@@ -837,11 +837,11 @@ static int add_excludes(const char *fname, const char *base, int baselen,
}
}
- add_excludes_from_buffer(buf, size, base, baselen, pl);
+ add_patterns_from_buffer(buf, size, base, baselen, pl);
return 0;
}
-static int add_excludes_from_buffer(char *buf, size_t size,
+static int add_patterns_from_buffer(char *buf, size_t size,
const char *base, int baselen,
struct pattern_list *pl)
{
@@ -860,7 +860,7 @@ static int add_excludes_from_buffer(char *buf, size_t size,
if (entry != buf + i && entry[0] != '#') {
buf[i - (i && buf[i-1] == '\r')] = 0;
trim_trailing_spaces(entry);
- add_exclude(entry, base, baselen, pl, lineno);
+ add_pattern(entry, base, baselen, pl, lineno);
}
lineno++;
entry = buf + i + 1;
@@ -869,14 +869,14 @@ static int add_excludes_from_buffer(char *buf, size_t size,
return 0;
}
-int add_excludes_from_file_to_list(const char *fname, const char *base,
+int add_patterns_from_file_to_list(const char *fname, const char *base,
int baselen, struct pattern_list *pl,
struct index_state *istate)
{
- return add_excludes(fname, base, baselen, pl, istate, NULL);
+ return add_patterns(fname, base, baselen, pl, istate, NULL);
}
-int add_excludes_from_blob_to_list(
+int add_patterns_from_blob_to_list(
struct object_id *oid,
const char *base, int baselen,
struct pattern_list *pl)
@@ -889,11 +889,11 @@ int add_excludes_from_blob_to_list(
if (r != 1)
return r;
- add_excludes_from_buffer(buf, size, base, baselen, pl);
+ add_patterns_from_buffer(buf, size, base, baselen, pl);
return 0;
}
-struct pattern_list *add_exclude_list(struct dir_struct *dir,
+struct pattern_list *add_pattern_list(struct dir_struct *dir,
int group_type, const char *src)
{
struct pattern_list *pl;
@@ -910,7 +910,7 @@ struct pattern_list *add_exclude_list(struct dir_struct *dir,
/*
* Used to set up core.excludesfile and .git/info/exclude lists.
*/
-static void add_excludes_from_file_1(struct dir_struct *dir, const char *fname,
+static void add_patterns_from_file_1(struct dir_struct *dir, const char *fname,
struct oid_stat *oid_stat)
{
struct pattern_list *pl;
@@ -921,15 +921,15 @@ static void add_excludes_from_file_1(struct dir_struct *dir, const char *fname,
*/
if (!dir->untracked)
dir->unmanaged_exclude_files++;
- pl = add_exclude_list(dir, EXC_FILE, fname);
- if (add_excludes(fname, "", 0, pl, NULL, oid_stat) < 0)
+ pl = add_pattern_list(dir, EXC_FILE, fname);
+ if (add_patterns(fname, "", 0, pl, NULL, oid_stat) < 0)
die(_("cannot use %s as an exclude file"), fname);
}
-void add_excludes_from_file(struct dir_struct *dir, const char *fname)
+void add_patterns_from_file(struct dir_struct *dir, const char *fname)
{
dir->unmanaged_exclude_files++; /* see validate_untracked_cache() */
- add_excludes_from_file_1(dir, fname, NULL);
+ add_patterns_from_file_1(dir, fname, NULL);
}
int match_basename(const char *basename, int basenamelen,
@@ -1021,7 +1021,7 @@ int match_pathname(const char *pathname, int pathlen,
* any, determines the fate. Returns the exclude_list element which
* matched, or NULL for undecided.
*/
-static struct path_pattern *last_exclude_matching_from_list(const char *pathname,
+static struct path_pattern *last_matching_pattern_from_list(const char *pathname,
int pathlen,
const char *basename,
int *dtype,
@@ -1080,14 +1080,14 @@ int is_excluded_from_list(const char *pathname,
struct pattern_list *pl, struct index_state *istate)
{
struct path_pattern *pattern;
- pattern = last_exclude_matching_from_list(pathname, pathlen, basename,
+ pattern = last_matching_pattern_from_list(pathname, pathlen, basename,
dtype, pl, istate);
if (pattern)
return pattern->flags & PATTERN_FLAG_NEGATIVE ? 0 : 1;
return -1; /* undecided */
}
-static struct path_pattern *last_exclude_matching_from_lists(
+static struct path_pattern *last_matching_pattern_from_lists(
struct dir_struct *dir, struct index_state *istate,
const char *pathname, int pathlen,
const char *basename, int *dtype_p)
@@ -1098,7 +1098,7 @@ static struct path_pattern *last_exclude_matching_from_lists(
for (i = EXC_CMDL; i <= EXC_FILE; i++) {
group = &dir->exclude_list_group[i];
for (j = group->nr - 1; j >= 0; j--) {
- pattern = last_exclude_matching_from_list(
+ pattern = last_matching_pattern_from_list(
pathname, pathlen, basename, dtype_p,
&group->pl[j], istate);
if (pattern)
@@ -1137,7 +1137,7 @@ static void prep_exclude(struct dir_struct *dir,
dir->exclude_stack = stk->prev;
dir->pattern = NULL;
free((char *)pl->src); /* see strbuf_detach() below */
- clear_exclude_list(pl);
+ clear_pattern_list(pl);
free(stk);
group->nr--;
}
@@ -1184,7 +1184,7 @@ static void prep_exclude(struct dir_struct *dir,
stk->baselen = cp - base;
stk->exclude_ix = group->nr;
stk->ucd = untracked;
- pl = add_exclude_list(dir, EXC_DIRS, NULL);
+ pl = add_pattern_list(dir, EXC_DIRS, NULL);
strbuf_add(&dir->basebuf, base + current, stk->baselen - current);
assert(stk->baselen == dir->basebuf.len);
@@ -1192,7 +1192,7 @@ static void prep_exclude(struct dir_struct *dir,
if (stk->baselen) {
int dt = DT_DIR;
dir->basebuf.buf[stk->baselen - 1] = 0;
- dir->pattern = last_exclude_matching_from_lists(dir,
+ dir->pattern = last_matching_pattern_from_lists(dir,
istate,
dir->basebuf.buf, stk->baselen - 1,
dir->basebuf.buf + current, &dt);
@@ -1228,28 +1228,28 @@ static void prep_exclude(struct dir_struct *dir,
* need fname to remain unchanged to ensure the src
* member of each struct path_pattern correctly
* back-references its source file. Other invocations
- * of add_exclude_list provide stable strings, so we
+ * of add_pattern_list provide stable strings, so we
* strbuf_detach() and free() here in the caller.
*/
struct strbuf sb = STRBUF_INIT;
strbuf_addbuf(&sb, &dir->basebuf);
strbuf_addstr(&sb, dir->exclude_per_dir);
pl->src = strbuf_detach(&sb, NULL);
- add_excludes(pl->src, pl->src, stk->baselen, pl, istate,
+ add_patterns(pl->src, pl->src, stk->baselen, pl, istate,
untracked ? &oid_stat : NULL);
}
/*
* NEEDSWORK: when untracked cache is enabled, prep_exclude()
* will first be called in valid_cached_dir() then maybe many
- * times more in last_exclude_matching(). When the cache is
- * used, last_exclude_matching() will not be called and
+ * times more in last_matching_pattern(). When the cache is
+ * used, last_matching_pattern() will not be called and
* reading .gitignore content will be a waste.
*
* So when it's called by valid_cached_dir() and we can get
* .gitignore SHA-1 from the index (i.e. .gitignore is not
* modified on work tree), we could delay reading the
* .gitignore content until we absolutely need it in
- * last_exclude_matching(). Be careful about ignore rule
+ * last_matching_pattern(). Be careful about ignore rule
* order, though, if you do that.
*/
if (untracked &&
@@ -1269,7 +1269,7 @@ static void prep_exclude(struct dir_struct *dir,
* Returns the exclude_list element which matched, or NULL for
* undecided.
*/
-struct path_pattern *last_exclude_matching(struct dir_struct *dir,
+struct path_pattern *last_matching_pattern(struct dir_struct *dir,
struct index_state *istate,
const char *pathname,
int *dtype_p)
@@ -1283,7 +1283,7 @@ struct path_pattern *last_exclude_matching(struct dir_struct *dir,
if (dir->pattern)
return dir->pattern;
- return last_exclude_matching_from_lists(dir, istate, pathname, pathlen,
+ return last_matching_pattern_from_lists(dir, istate, pathname, pathlen,
basename, dtype_p);
}
@@ -1296,7 +1296,7 @@ int is_excluded(struct dir_struct *dir, struct index_state *istate,
const char *pathname, int *dtype_p)
{
struct path_pattern *pattern =
- last_exclude_matching(dir, istate, pathname, dtype_p);
+ last_matching_pattern(dir, istate, pathname, dtype_p);
if (pattern)
return pattern->flags & PATTERN_FLAG_NEGATIVE ? 0 : 1;
return 0;
@@ -1811,7 +1811,7 @@ static int valid_cached_dir(struct dir_struct *dir,
/*
* prep_exclude will be called eventually on this directory,
- * but it's called much later in last_exclude_matching(). We
+ * but it's called much later in last_matching_pattern(). We
* need it now to determine the validity of the cache for this
* path. The next calls will be nearly no-op, the way
* prep_exclude() is designed.
@@ -2491,14 +2491,14 @@ void setup_standard_excludes(struct dir_struct *dir)
if (!excludes_file)
excludes_file = xdg_config_home("ignore");
if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
- add_excludes_from_file_1(dir, excludes_file,
+ add_patterns_from_file_1(dir, excludes_file,
dir->untracked ? &dir->ss_excludes_file : NULL);
/* per repository user preference */
if (startup_info->have_repository) {
const char *path = git_path_info_exclude();
if (!access_or_warn(path, R_OK, 0))
- add_excludes_from_file_1(dir, path,
+ add_patterns_from_file_1(dir, path,
dir->untracked ? &dir->ss_info_exclude : NULL);
}
}
@@ -2539,7 +2539,7 @@ void clear_directory(struct dir_struct *dir)
pl = &group->pl[j];
if (i == EXC_DIRS)
free((char *)pl->src);
- clear_exclude_list(pl);
+ clear_pattern_list(pl);
}
free(group->pl);
}
diff --git a/dir.h b/dir.h
index 87eb10662f..7babf31d88 100644
--- a/dir.h
+++ b/dir.h
@@ -18,7 +18,7 @@ struct dir_entry {
struct path_pattern {
/*
- * This allows callers of last_exclude_matching() etc.
+ * This allows callers of last_matching_pattern() etc.
* to determine the origin of the matching pattern.
*/
struct pattern_list *pl;
@@ -248,26 +248,26 @@ int match_pathname(const char *, int,
const char *, int,
const char *, int, int, unsigned);
-struct path_pattern *last_exclude_matching(struct dir_struct *dir,
- struct index_state *istate,
- const char *name, int *dtype);
+struct path_pattern *last_matching_pattern(struct dir_struct *dir,
+ struct index_state *istate,
+ const char *name, int *dtype);
int is_excluded(struct dir_struct *dir,
struct index_state *istate,
const char *name, int *dtype);
-struct pattern_list *add_exclude_list(struct dir_struct *dir,
+struct pattern_list *add_pattern_list(struct dir_struct *dir,
int group_type, const char *src);
-int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,
+int add_patterns_from_file_to_list(const char *fname, const char *base, int baselen,
struct pattern_list *pl, struct index_state *istate);
-void add_excludes_from_file(struct dir_struct *, const char *fname);
-int add_excludes_from_blob_to_list(struct object_id *oid,
+void add_patterns_from_file(struct dir_struct *, const char *fname);
+int add_patterns_from_blob_to_list(struct object_id *oid,
const char *base, int baselen,
struct pattern_list *pl);
-void parse_exclude_pattern(const char **string, int *patternlen, unsigned *flags, int *nowildcardlen);
-void add_exclude(const char *string, const char *base,
+void parse_path_pattern(const char **string, int *patternlen, unsigned *flags, int *nowildcardlen);
+void add_pattern(const char *string, const char *base,
int baselen, struct pattern_list *pl, int srcpos);
-void clear_exclude_list(struct pattern_list *pl);
+void clear_pattern_list(struct pattern_list *pl);
void clear_directory(struct dir_struct *dir);
int repo_file_exists(struct repository *repo, const char *path);
diff --git a/list-objects-filter.c b/list-objects-filter.c
index a1fedf8bd8..ccd58e61c3 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -482,7 +482,7 @@ static void filter_sparse_oid__init(
struct filter *filter)
{
struct filter_sparse_data *d = xcalloc(1, sizeof(*d));
- if (add_excludes_from_blob_to_list(filter_options->sparse_oid_value,
+ if (add_patterns_from_blob_to_list(filter_options->sparse_oid_value,
NULL, 0, &d->pl) < 0)
die("could not load filter specification");
diff --git a/unpack-trees.c b/unpack-trees.c
index c4dc21affb..902a799aeb 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1464,7 +1464,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
o->skip_sparse_checkout = 1;
if (!o->skip_sparse_checkout) {
char *sparse = git_pathdup("info/sparse-checkout");
- if (add_excludes_from_file_to_list(sparse, "", 0, &pl, NULL) < 0)
+ if (add_patterns_from_file_to_list(sparse, "", 0, &pl, NULL) < 0)
o->skip_sparse_checkout = 1;
else
o->pl = &pl;
@@ -1631,7 +1631,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
done:
trace_performance_leave("unpack_trees");
- clear_exclude_list(&pl);
+ clear_pattern_list(&pl);
return ret;
return_failed:
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/5] unpack-trees: rename 'is_excluded_from_list()'
2019-09-03 18:04 [PATCH 0/5] Refactor excludes library Derrick Stolee via GitGitGadget
` (3 preceding siblings ...)
2019-09-03 18:04 ` [PATCH 4/5] treewide: rename 'exclude' methods to 'pattern' Derrick Stolee via GitGitGadget
@ 2019-09-03 18:04 ` Derrick Stolee via GitGitGadget
2019-09-04 20:25 ` Elijah Newren
2019-09-04 20:28 ` [PATCH 0/5] Refactor excludes library Elijah Newren
2019-09-06 20:34 ` Junio C Hamano
6 siblings, 1 reply; 11+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2019-09-03 18:04 UTC (permalink / raw)
To: git; +Cc: newren, pclouds, peff, jon, matvore, Junio C Hamano,
Derrick Stolee
From: Derrick Stolee <dstolee@microsoft.com>
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>
---
dir.c | 25 +++++++++++++++++--------
dir.h | 21 +++++++++++++++++----
list-objects-filter.c | 29 +++++++++++++++--------------
unpack-trees.c | 39 +++++++++++++++++++++++----------------
4 files changed, 72 insertions(+), 42 deletions(-)
diff --git a/dir.c b/dir.c
index b057bd3d95..34972abdaf 100644
--- a/dir.c
+++ b/dir.c
@@ -1072,19 +1072,28 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname
}
/*
- * Scan the list and let the last match determine the fate.
- * Return 1 for exclude, 0 for include and -1 for undecided.
+ * Scan the list of patterns to determine if the ordered list
+ * of patterns matches on 'pathname'.
+ *
+ * Return 1 for a match, 0 for not matched and -1 for undecided.
*/
-int is_excluded_from_list(const char *pathname,
- int pathlen, const char *basename, int *dtype,
- struct pattern_list *pl, struct index_state *istate)
+enum pattern_match_result path_matches_pattern_list(
+ const char *pathname, int pathlen,
+ const char *basename, int *dtype,
+ struct pattern_list *pl,
+ struct index_state *istate)
{
struct path_pattern *pattern;
pattern = last_matching_pattern_from_list(pathname, pathlen, basename,
dtype, pl, istate);
- if (pattern)
- return pattern->flags & PATTERN_FLAG_NEGATIVE ? 0 : 1;
- return -1; /* undecided */
+ if (pattern) {
+ if (pattern->flags & PATTERN_FLAG_NEGATIVE)
+ return NOT_MATCHED;
+ else
+ return MATCHED;
+ }
+
+ return UNDECIDED;
}
static struct path_pattern *last_matching_pattern_from_lists(
diff --git a/dir.h b/dir.h
index 7babf31d88..608696c958 100644
--- a/dir.h
+++ b/dir.h
@@ -230,10 +230,23 @@ int read_directory(struct dir_struct *, struct index_state *istate,
const char *path, int len,
const struct pathspec *pathspec);
-int is_excluded_from_list(const char *pathname, int pathlen,
- const char *basename, int *dtype,
- struct pattern_list *pl,
- struct index_state *istate);
+enum pattern_match_result {
+ UNDECIDED = -1,
+ NOT_MATCHED = 0,
+ MATCHED = 1,
+};
+
+/*
+ * Scan the list of patterns to determine if the ordered list
+ * of patterns matches on 'pathname'.
+ *
+ * Return 1 for a match, 0 for not matched and -1 for undecided.
+ */
+enum pattern_match_result path_matches_pattern_list(const char *pathname,
+ int pathlen,
+ const char *basename, int *dtype,
+ struct pattern_list *pl,
+ struct index_state *istate);
struct dir_entry *dir_add_ignored(struct dir_struct *dir,
struct index_state *istate,
const char *pathname, int len);
diff --git a/list-objects-filter.c b/list-objects-filter.c
index ccd58e61c3..d624f1c898 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -328,12 +328,12 @@ static void filter_blobs_limit__init(
*/
struct frame {
/*
- * defval is the usual default include/exclude value that
+ * default_match is the usual default include/exclude value that
* should be inherited as we recurse into directories based
* upon pattern matching of the directory itself or of a
* containing directory.
*/
- int defval;
+ enum pattern_match_result default_match;
/*
* 1 if the directory (recursively) contains any provisionally
@@ -363,8 +363,9 @@ static enum list_objects_filter_result filter_sparse(
void *filter_data_)
{
struct filter_sparse_data *filter_data = filter_data_;
- int val, dtype;
+ int dtype;
struct frame *frame;
+ enum pattern_match_result match;
switch (filter_situation) {
default:
@@ -373,15 +374,15 @@ static enum list_objects_filter_result filter_sparse(
case LOFS_BEGIN_TREE:
assert(obj->type == OBJ_TREE);
dtype = DT_DIR;
- val = is_excluded_from_list(pathname, strlen(pathname),
- filename, &dtype, &filter_data->pl,
- r->index);
- if (val < 0)
- val = filter_data->array_frame[filter_data->nr - 1].defval;
+ match = path_matches_pattern_list(pathname, strlen(pathname),
+ filename, &dtype, &filter_data->pl,
+ r->index);
+ if (match == UNDECIDED)
+ match = filter_data->array_frame[filter_data->nr - 1].default_match;
ALLOC_GROW(filter_data->array_frame, filter_data->nr + 1,
filter_data->alloc);
- filter_data->array_frame[filter_data->nr].defval = val;
+ filter_data->array_frame[filter_data->nr].default_match = match;
filter_data->array_frame[filter_data->nr].child_prov_omit = 0;
filter_data->nr++;
@@ -435,12 +436,12 @@ static enum list_objects_filter_result filter_sparse(
frame = &filter_data->array_frame[filter_data->nr - 1];
dtype = DT_REG;
- val = is_excluded_from_list(pathname, strlen(pathname),
+ match = path_matches_pattern_list(pathname, strlen(pathname),
filename, &dtype, &filter_data->pl,
r->index);
- if (val < 0)
- val = frame->defval;
- if (val > 0) {
+ if (match == UNDECIDED)
+ match = frame->default_match;
+ if (match == MATCHED) {
if (omits)
oidset_remove(omits, &obj->oid);
return LOFR_MARK_SEEN | LOFR_DO_SHOW;
@@ -487,7 +488,7 @@ static void filter_sparse_oid__init(
die("could not load filter specification");
ALLOC_GROW(d->array_frame, d->nr + 1, d->alloc);
- d->array_frame[d->nr].defval = 0; /* default to include */
+ d->array_frame[d->nr].default_match = 0; /* default to include */
d->array_frame[d->nr].child_prov_omit = 0;
d->nr++;
diff --git a/unpack-trees.c b/unpack-trees.c
index 902a799aeb..cd548f4fa2 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1265,7 +1265,8 @@ static int clear_ce_flags_1(struct index_state *istate,
struct cache_entry **cache, int nr,
struct strbuf *prefix,
int select_mask, int clear_mask,
- struct pattern_list *pl, int defval);
+ struct pattern_list *pl,
+ enum pattern_match_result default_match);
/* Whole directory matching */
static int clear_ce_flags_dir(struct index_state *istate,
@@ -1273,19 +1274,21 @@ static int clear_ce_flags_dir(struct index_state *istate,
struct strbuf *prefix,
char *basename,
int select_mask, int clear_mask,
- struct pattern_list *pl, int defval)
+ struct pattern_list *pl,
+ enum pattern_match_result default_match)
{
struct cache_entry **cache_end;
int dtype = DT_DIR;
- int ret = is_excluded_from_list(prefix->buf, prefix->len,
- basename, &dtype, pl, istate);
int rc;
+ enum pattern_match_result ret;
+ ret = path_matches_pattern_list(prefix->buf, prefix->len,
+ basename, &dtype, pl, istate);
strbuf_addch(prefix, '/');
/* If undecided, use matching result of parent dir in defval */
- if (ret < 0)
- ret = defval;
+ if (ret == UNDECIDED)
+ ret = default_match;
for (cache_end = cache; cache_end != cache + nr; cache_end++) {
struct cache_entry *ce = *cache_end;
@@ -1298,7 +1301,7 @@ static int clear_ce_flags_dir(struct index_state *istate,
* with ret (iow, we know in advance the incl/excl
* decision for the entire directory), clear flag here without
* calling clear_ce_flags_1(). That function will call
- * the expensive is_excluded_from_list() on every entry.
+ * the expensive path_matches_pattern_list() on every entry.
*/
rc = clear_ce_flags_1(istate, cache, cache_end - cache,
prefix,
@@ -1327,7 +1330,8 @@ static int clear_ce_flags_1(struct index_state *istate,
struct cache_entry **cache, int nr,
struct strbuf *prefix,
int select_mask, int clear_mask,
- struct pattern_list *pl, int defval)
+ struct pattern_list *pl,
+ enum pattern_match_result default_match)
{
struct cache_entry **cache_end = cache + nr;
@@ -1338,7 +1342,8 @@ static int clear_ce_flags_1(struct index_state *istate,
while(cache != cache_end) {
struct cache_entry *ce = *cache;
const char *name, *slash;
- int len, dtype, ret;
+ int len, dtype;
+ enum pattern_match_result ret;
if (select_mask && !(ce->ce_flags & select_mask)) {
cache++;
@@ -1362,7 +1367,7 @@ static int clear_ce_flags_1(struct index_state *istate,
prefix,
prefix->buf + prefix->len - len,
select_mask, clear_mask,
- pl, defval);
+ pl, default_match);
/* clear_c_f_dir eats a whole dir already? */
if (processed) {
@@ -1374,18 +1379,20 @@ static int clear_ce_flags_1(struct index_state *istate,
strbuf_addch(prefix, '/');
cache += clear_ce_flags_1(istate, cache, cache_end - cache,
prefix,
- select_mask, clear_mask, pl, defval);
+ select_mask, clear_mask, pl,
+ default_match);
strbuf_setlen(prefix, prefix->len - len - 1);
continue;
}
/* Non-directory */
dtype = ce_to_dtype(ce);
- ret = is_excluded_from_list(ce->name, ce_namelen(ce),
- name, &dtype, pl, istate);
- if (ret < 0)
- ret = defval;
- if (ret > 0)
+ ret = path_matches_pattern_list(ce->name,
+ ce_namelen(ce),
+ name, &dtype, pl, istate);
+ if (ret == UNDECIDED)
+ ret = default_match;
+ if (ret == MATCHED)
ce->ce_flags &= ~clear_mask;
cache++;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] unpack-trees: rename 'is_excluded_from_list()'
2019-09-03 18:04 ` [PATCH 5/5] unpack-trees: rename 'is_excluded_from_list()' Derrick Stolee via GitGitGadget
@ 2019-09-04 20:25 ` Elijah Newren
0 siblings, 0 replies; 11+ messages in thread
From: Elijah Newren @ 2019-09-04 20:25 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: Git Mailing List, Junio C Hamano, Derrick Stolee
On Tue, Sep 3, 2019 at 11:04 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> 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
s/manes/means/
> 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,
s/retur/return/
> and provides more type checking for handling those values.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> dir.c | 25 +++++++++++++++++--------
> dir.h | 21 +++++++++++++++++----
> list-objects-filter.c | 29 +++++++++++++++--------------
> unpack-trees.c | 39 +++++++++++++++++++++++----------------
> 4 files changed, 72 insertions(+), 42 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index b057bd3d95..34972abdaf 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1072,19 +1072,28 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname
> }
>
> /*
> - * Scan the list and let the last match determine the fate.
> - * Return 1 for exclude, 0 for include and -1 for undecided.
> + * Scan the list of patterns to determine if the ordered list
> + * of patterns matches on 'pathname'.
> + *
> + * Return 1 for a match, 0 for not matched and -1 for undecided.
Maybe drop this last sentence since it's misleading, and since the
function signature already specifies that it returns an enum and the
enums have obvious meanings already?
> */
> -int is_excluded_from_list(const char *pathname,
> - int pathlen, const char *basename, int *dtype,
> - struct pattern_list *pl, struct index_state *istate)
> +enum pattern_match_result path_matches_pattern_list(
> + const char *pathname, int pathlen,
> + const char *basename, int *dtype,
> + struct pattern_list *pl,
> + struct index_state *istate)
> {
> struct path_pattern *pattern;
> pattern = last_matching_pattern_from_list(pathname, pathlen, basename,
> dtype, pl, istate);
> - if (pattern)
> - return pattern->flags & PATTERN_FLAG_NEGATIVE ? 0 : 1;
> - return -1; /* undecided */
> + if (pattern) {
> + if (pattern->flags & PATTERN_FLAG_NEGATIVE)
> + return NOT_MATCHED;
> + else
> + return MATCHED;
> + }
> +
> + return UNDECIDED;
> }
>
> static struct path_pattern *last_matching_pattern_from_lists(
> diff --git a/dir.h b/dir.h
> index 7babf31d88..608696c958 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -230,10 +230,23 @@ int read_directory(struct dir_struct *, struct index_state *istate,
> const char *path, int len,
> const struct pathspec *pathspec);
>
> -int is_excluded_from_list(const char *pathname, int pathlen,
> - const char *basename, int *dtype,
> - struct pattern_list *pl,
> - struct index_state *istate);
> +enum pattern_match_result {
> + UNDECIDED = -1,
> + NOT_MATCHED = 0,
> + MATCHED = 1,
Are we worried about preventing e.g. grep from using these names or
clashing with another lib (block-sha1?) that might choose to define
these?
> +};
> +
> +/*
> + * Scan the list of patterns to determine if the ordered list
> + * of patterns matches on 'pathname'.
> + *
> + * Return 1 for a match, 0 for not matched and -1 for undecided.
Again, I'd drop the last sentence.
> + */
> +enum pattern_match_result path_matches_pattern_list(const char *pathname,
> + int pathlen,
> + const char *basename, int *dtype,
> + struct pattern_list *pl,
> + struct index_state *istate);
> struct dir_entry *dir_add_ignored(struct dir_struct *dir,
> struct index_state *istate,
> const char *pathname, int len);
> diff --git a/list-objects-filter.c b/list-objects-filter.c
> index ccd58e61c3..d624f1c898 100644
> --- a/list-objects-filter.c
> +++ b/list-objects-filter.c
> @@ -328,12 +328,12 @@ static void filter_blobs_limit__init(
> */
> struct frame {
> /*
> - * defval is the usual default include/exclude value that
> + * default_match is the usual default include/exclude value that
> * should be inherited as we recurse into directories based
> * upon pattern matching of the directory itself or of a
> * containing directory.
> */
> - int defval;
> + enum pattern_match_result default_match;
>
> /*
> * 1 if the directory (recursively) contains any provisionally
> @@ -363,8 +363,9 @@ static enum list_objects_filter_result filter_sparse(
> void *filter_data_)
> {
> struct filter_sparse_data *filter_data = filter_data_;
> - int val, dtype;
> + int dtype;
> struct frame *frame;
> + enum pattern_match_result match;
>
> switch (filter_situation) {
> default:
> @@ -373,15 +374,15 @@ static enum list_objects_filter_result filter_sparse(
> case LOFS_BEGIN_TREE:
> assert(obj->type == OBJ_TREE);
> dtype = DT_DIR;
> - val = is_excluded_from_list(pathname, strlen(pathname),
> - filename, &dtype, &filter_data->pl,
> - r->index);
> - if (val < 0)
> - val = filter_data->array_frame[filter_data->nr - 1].defval;
> + match = path_matches_pattern_list(pathname, strlen(pathname),
> + filename, &dtype, &filter_data->pl,
> + r->index);
> + if (match == UNDECIDED)
> + match = filter_data->array_frame[filter_data->nr - 1].default_match;
>
> ALLOC_GROW(filter_data->array_frame, filter_data->nr + 1,
> filter_data->alloc);
> - filter_data->array_frame[filter_data->nr].defval = val;
> + filter_data->array_frame[filter_data->nr].default_match = match;
> filter_data->array_frame[filter_data->nr].child_prov_omit = 0;
> filter_data->nr++;
>
> @@ -435,12 +436,12 @@ static enum list_objects_filter_result filter_sparse(
> frame = &filter_data->array_frame[filter_data->nr - 1];
>
> dtype = DT_REG;
> - val = is_excluded_from_list(pathname, strlen(pathname),
> + match = path_matches_pattern_list(pathname, strlen(pathname),
> filename, &dtype, &filter_data->pl,
> r->index);
> - if (val < 0)
> - val = frame->defval;
> - if (val > 0) {
> + if (match == UNDECIDED)
> + match = frame->default_match;
> + if (match == MATCHED) {
> if (omits)
> oidset_remove(omits, &obj->oid);
> return LOFR_MARK_SEEN | LOFR_DO_SHOW;
> @@ -487,7 +488,7 @@ static void filter_sparse_oid__init(
> die("could not load filter specification");
>
> ALLOC_GROW(d->array_frame, d->nr + 1, d->alloc);
> - d->array_frame[d->nr].defval = 0; /* default to include */
> + d->array_frame[d->nr].default_match = 0; /* default to include */
s/0/MATCHED/ ?
> d->array_frame[d->nr].child_prov_omit = 0;
> d->nr++;
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 902a799aeb..cd548f4fa2 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1265,7 +1265,8 @@ static int clear_ce_flags_1(struct index_state *istate,
> struct cache_entry **cache, int nr,
> struct strbuf *prefix,
> int select_mask, int clear_mask,
> - struct pattern_list *pl, int defval);
> + struct pattern_list *pl,
> + enum pattern_match_result default_match);
>
> /* Whole directory matching */
> static int clear_ce_flags_dir(struct index_state *istate,
> @@ -1273,19 +1274,21 @@ static int clear_ce_flags_dir(struct index_state *istate,
> struct strbuf *prefix,
> char *basename,
> int select_mask, int clear_mask,
> - struct pattern_list *pl, int defval)
> + struct pattern_list *pl,
> + enum pattern_match_result default_match)
> {
> struct cache_entry **cache_end;
> int dtype = DT_DIR;
> - int ret = is_excluded_from_list(prefix->buf, prefix->len,
> - basename, &dtype, pl, istate);
> int rc;
> + enum pattern_match_result ret;
> + ret = path_matches_pattern_list(prefix->buf, prefix->len,
> + basename, &dtype, pl, istate);
>
> strbuf_addch(prefix, '/');
>
> /* If undecided, use matching result of parent dir in defval */
> - if (ret < 0)
> - ret = defval;
> + if (ret == UNDECIDED)
> + ret = default_match;
>
> for (cache_end = cache; cache_end != cache + nr; cache_end++) {
> struct cache_entry *ce = *cache_end;
> @@ -1298,7 +1301,7 @@ static int clear_ce_flags_dir(struct index_state *istate,
> * with ret (iow, we know in advance the incl/excl
> * decision for the entire directory), clear flag here without
> * calling clear_ce_flags_1(). That function will call
> - * the expensive is_excluded_from_list() on every entry.
> + * the expensive path_matches_pattern_list() on every entry.
> */
> rc = clear_ce_flags_1(istate, cache, cache_end - cache,
> prefix,
> @@ -1327,7 +1330,8 @@ static int clear_ce_flags_1(struct index_state *istate,
> struct cache_entry **cache, int nr,
> struct strbuf *prefix,
> int select_mask, int clear_mask,
> - struct pattern_list *pl, int defval)
> + struct pattern_list *pl,
> + enum pattern_match_result default_match)
> {
> struct cache_entry **cache_end = cache + nr;
>
> @@ -1338,7 +1342,8 @@ static int clear_ce_flags_1(struct index_state *istate,
> while(cache != cache_end) {
> struct cache_entry *ce = *cache;
> const char *name, *slash;
> - int len, dtype, ret;
> + int len, dtype;
> + enum pattern_match_result ret;
>
> if (select_mask && !(ce->ce_flags & select_mask)) {
> cache++;
> @@ -1362,7 +1367,7 @@ static int clear_ce_flags_1(struct index_state *istate,
> prefix,
> prefix->buf + prefix->len - len,
> select_mask, clear_mask,
> - pl, defval);
> + pl, default_match);
>
> /* clear_c_f_dir eats a whole dir already? */
> if (processed) {
> @@ -1374,18 +1379,20 @@ static int clear_ce_flags_1(struct index_state *istate,
> strbuf_addch(prefix, '/');
> cache += clear_ce_flags_1(istate, cache, cache_end - cache,
> prefix,
> - select_mask, clear_mask, pl, defval);
> + select_mask, clear_mask, pl,
> + default_match);
> strbuf_setlen(prefix, prefix->len - len - 1);
> continue;
> }
>
> /* Non-directory */
> dtype = ce_to_dtype(ce);
> - ret = is_excluded_from_list(ce->name, ce_namelen(ce),
> - name, &dtype, pl, istate);
> - if (ret < 0)
> - ret = defval;
> - if (ret > 0)
> + ret = path_matches_pattern_list(ce->name,
> + ce_namelen(ce),
> + name, &dtype, pl, istate);
> + if (ret == UNDECIDED)
> + ret = default_match;
> + if (ret == MATCHED)
> ce->ce_flags &= ~clear_mask;
> cache++;
> }
> --
> gitgitgadget
The rest looks good.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/5] Refactor excludes library
2019-09-03 18:04 [PATCH 0/5] Refactor excludes library Derrick Stolee via GitGitGadget
` (4 preceding siblings ...)
2019-09-03 18:04 ` [PATCH 5/5] unpack-trees: rename 'is_excluded_from_list()' Derrick Stolee via GitGitGadget
@ 2019-09-04 20:28 ` Elijah Newren
2019-09-06 20:34 ` Junio C Hamano
6 siblings, 0 replies; 11+ messages in thread
From: Elijah Newren @ 2019-09-04 20:28 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget; +Cc: Git Mailing List, Junio C Hamano
On Tue, Sep 3, 2019 at 11:04 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> The exclude library defined in dir.h was originally written for the
> .gitignore feature, but has since been used for .gitattributes and
> sparse-checkout. In the later applications, these patterns are used for
> inclusion rather than exclusion, so the name is confusing. This gets
> particularly bad when looking at how the sparse-checkout feature uses
> is_excluded_from_list() to really mean "should be included in the working
> directory".
>
> This series performs several renames of structs and methods to generalize
> the exclude library to be a "pattern matching" library. Instead of a list of
> excludes, we have a list of path patterns. It is up to the consumer to
> decide what to do with a match. The .gitignore logic will still treat the
> patterns as a list of exclusions, the sparse-checkout logic treats the
> patterns as a list of inclusions.
Wahoo! Thanks for going through and cleaning these up. The
possibility of negated patterns (or negated logic or both) on top of
using "exclude" when "include" was _sometimes_ meant was just too much
for me to keep track of. This series will make it much easier.
> For this reason, some methods and structs in dir.h retain "exclude" in their
> name. These are limited to things consumed only by the .gitignore feature
> (as far as I can tell).
>
> Most of these changes are mechanical find-and-replaces, with the exception
> of some variable names and the last patch.
I looked through the first four patches with --color-words=. to spot
check them; as you say, looks like straightforward mechanical changes.
I did notice that the first two paragraphs were shared between the
commit messages; it does make sense to help provide the context, but
it seemed a little duplicative. I'm not sure what to suggest; it may
be fine as it is. Just thought I'd flag it.
> The last patch, "unpack-trees: rename 'is_excluded_from_list()'", performs a
> more meaningful refactor. The method is_excluded_from_list() was only used
> by sparse-checkout (inside the clear_ce_flags() methods) to see if a path
> should be included in the working directory. The return value of "1 for
> excluded" was confusing. Instead, use a new enum value. This required
> changing several method prototypes inside unpack-trees.c.
>
> This refactor was inspired by Elijah Newren's feedback [1] on my
> sparse-checkout builtin RFC. I am working on a few other orthogonal changes
> to make to the existing sparse-checkout behavior before I resubmit that RFC.
>
> I had started working on v2.23.0, but found adjacent-diff conflicts with
> md/list-objects-filter-combo [2] and js/partial-clone-sparse-blob [3]. Those
> branches are independent, but the conflicts with
> md/list-objects-filter-combo were more severe (and that branch seems closer
> to merging) so this is based on md/list-object-filter-combo. Hopefully the
> conflicts with js/partial-clone-sparse-blob are clear enough to resolve
> easily.
I posted some comments on patch 5; otherwise, the series looks good to me.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] treewide: rename 'struct exclude' to 'struct path_pattern'
2019-09-03 18:04 ` [PATCH 1/5] treewide: rename 'struct exclude' to 'struct path_pattern' Derrick Stolee via GitGitGadget
@ 2019-09-05 6:55 ` Jeff King
2019-09-05 21:03 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2019-09-05 6:55 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, newren, pclouds, jon, matvore, Junio C Hamano,
Derrick Stolee
On Tue, Sep 03, 2019 at 11:04:55AM -0700, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@microsoft.com>
>
> The first consumer of pattern-matching filenames was the
> .gitignore feature. In that context, storing a list of patterns
> as a list of 'struct exclude' items 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!
>
> It would be clearer to rename this entire library as a "pattern
> matching" library, and the callers apply exclusion/inclusion
> logic accordingly based on their needs.
>
> This commit renames 'struct exclude' to 'struct path_pattern'
> and renames several variable names to match. 'struct pattern'
> was already taken by attr.c, and this more completely describes
> that the patterns are specific to file paths.
I agree that the current name is overly restrictive, and this is a step
in the right direction. However, when I see path_pattern that makes me
think of our command-line pathspecs.
I wonder if there's a name that could more clearly distinguish the two.
Or if it's sufficient to just become Git jargon that "pathspec" is the
command-line one and "path_pattern" is the file-based one (we're at
least pretty consistent about the former already).
I think one could also make an argument that the name collision is a
sign that these two things should actually share both syntax and
implementation, since we're exposing too similar-but-not-quite versions
of the same idea to users. But given the compatibility issues, it's
probably not worth changing the user facing parts at this point (and I
also haven't thought too hard about it; there may be reasons why the two
_should_ differ).
-Peff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] treewide: rename 'struct exclude' to 'struct path_pattern'
2019-09-05 6:55 ` Jeff King
@ 2019-09-05 21:03 ` Junio C Hamano
0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2019-09-05 21:03 UTC (permalink / raw)
To: Jeff King
Cc: Derrick Stolee via GitGitGadget, git, newren, pclouds, jon,
matvore, Derrick Stolee
Jeff King <peff@peff.net> writes:
> I wonder if there's a name that could more clearly distinguish the two.
> Or if it's sufficient to just become Git jargon that "pathspec" is the
> command-line one and "path_pattern" is the file-based one (we're at
> least pretty consistent about the former already).
>
> I think one could also make an argument that the name collision is a
> sign that these two things should actually share both syntax and
> implementation, since we're exposing too similar-but-not-quite versions
> of the same idea to users. But given the compatibility issues, it's
> probably not worth changing the user facing parts at this point (and I
> also haven't thought too hard about it; there may be reasons why the two
> _should_ differ).
Hmph. I did not realize there are so many differences X-<.
A pathspec is relative to $CWD, and there is a syntax, i.e.
prefixing with ":(top)", to make it relative to the root level. An
entry in a .gitignore file will never affect paths outside the
directory the file appears in. And there should never be such a
mechanism to allow it.
An entry without slash in .gitignore is a basename match, and there
is a syntax i.e. prefixing with "/", to anchor it to a single
directory. A pathspec without slash also can be a basename match
(e.g. "*.c" matches "a/b.c" as well as "d.c"). A pathspec with a
slash can be made to tail-match (e.g. "**/*.c" matches "a/b.c",
"a/b/c.c", etc.) but I do not think of a way to make an entry with a
slash in a .gitignore file a tail-match the same way. I do not think
this is intended but merely a missing feature.
So, yes, eventually we may want to make them more similar, but I
suspect that there are some things that should be in one but never
be in the other.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/5] Refactor excludes library
2019-09-03 18:04 [PATCH 0/5] Refactor excludes library Derrick Stolee via GitGitGadget
` (5 preceding siblings ...)
2019-09-04 20:28 ` [PATCH 0/5] Refactor excludes library Elijah Newren
@ 2019-09-06 20:34 ` Junio C Hamano
6 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2019-09-06 20:34 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget; +Cc: git, newren, pclouds, peff, jon, matvore
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> I had started working on v2.23.0, but found adjacent-diff conflicts with
> md/list-objects-filter-combo [2] and js/partial-clone-sparse-blob [3]. Those
> branches are independent, but the conflicts with
> md/list-objects-filter-combo were more severe (and that branch seems closer
> to merging) so this is based on md/list-object-filter-combo. Hopefully the
> conflicts with js/partial-clone-sparse-blob are clear enough to resolve
> easily.
Thanks.
I did screw the resolution up in previous integration, but I think
what we have today is in a good shape.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-09-06 20:34 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 18:04 [PATCH 0/5] Refactor excludes library Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 1/5] treewide: rename 'struct exclude' to 'struct path_pattern' Derrick Stolee via GitGitGadget
2019-09-05 6:55 ` Jeff King
2019-09-05 21:03 ` Junio C Hamano
2019-09-03 18:04 ` [PATCH 3/5] treewide: rename 'EXCL_FLAG_' to 'PATTERN_FLAG_' Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 2/5] treewide: rename 'struct exclude_list' to 'struct pattern_list' Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 4/5] treewide: rename 'exclude' methods to 'pattern' Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 5/5] unpack-trees: rename 'is_excluded_from_list()' Derrick Stolee via GitGitGadget
2019-09-04 20:25 ` Elijah Newren
2019-09-04 20:28 ` [PATCH 0/5] Refactor excludes library Elijah Newren
2019-09-06 20:34 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).