* [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 2/5] treewide: rename 'struct exclude_list' to 'struct pattern_list' 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* 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
* [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 ` Derrick Stolee via GitGitGadget
2019-09-03 18:04 ` [PATCH 3/5] treewide: rename 'EXCL_FLAG_' to 'PATTERN_FLAG_' 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 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 ` [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 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 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 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 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 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