Joe Perches <joe@xxxxxxxxxxx> writes: >> > @@ -2996,10 +2996,16 @@ static struct excludes { >> > const char *path; >> > } *excludes; >> > >> > +static struct includes { >> > + struct includes *next; >> > + const char *path; >> > +} *includes; >> >> Now this is ugly. You can just add a new variable "*includes" that is of >> exactly the same type as existing "*excludes" without introducing a new >> type. > > Yes, it's slightly ugly, but it was less work and much easier for > a human to parse. Another consideration is what should happen when you give contradicting excludes and includes list. For example, it is very plausible you might want to say "apply to all but header files, except that you want the part to one specific header file to also get applied). Something like: $ git apply --include='specific-one.h' --exclude='*.h' --include='*' <patch It is easy to declare that all the exclude patterns are processed and used to reject paths, and then only after that include patterns, if any, are used to limit the remainder. But that is describing how the code does it, and may not match what the users expect. For example, the users would expect: $ git apply --include='specific-one.h' --exclude='s*' <patch to apply the part for "specific-one.h" but no other paths that begin with "s". However, that is not what happens. It would be much easier to explain to the end users if the rule were that include and exclude patterns are examined in the order they are specified on the command line, and the first match determines the each path's fate. In order to support that, you do not want two separate lists. Instead, you would want to keep a single "static struct string_list limit_by_name", append both excluded and included items to the list as you encounter with string_list_append(), but in such a way that you can distinguish which one is which later. Also remember if you have seen any included item. Then your use_patch() would: * first check and ignore patches about paths outside of the prefix if any is specified via --directory; * loop over the limit_by_name.items[] array, checking the path with each element in it with fnmatch(). If you find a match, then you know if it is excluded (return 0) or included (return 1); * if no patterns match, return 1 (i.e. modify this path) if you did not see any "include" pattern. If you had any "include" pattern on the command line, return 0 (i.e. do not modify this path). Perhaps something like this, but I did not test it. builtin-apply.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 files changed, 33 insertions(+), 15 deletions(-) diff --git c/builtin-apply.c w/builtin-apply.c index 2216a0b..967ebec 100644 --- c/builtin-apply.c +++ w/builtin-apply.c @@ -2991,29 +2991,45 @@ static int write_out_results(struct patch *list, int skipped_patch) static struct lock_file lock_file; -static struct excludes { - struct excludes *next; - const char *path; -} *excludes; +static struct string_list limit_by_name; +static int has_include; +static void add_name_limit(const char *name, int exclude) +{ + struct string_list_item *it; + + it = string_list_append(name, &limit_by_name); + it->util = exclude ? NULL : (void *) 1; +} static int use_patch(struct patch *p) { const char *pathname = p->new_name ? p->new_name : p->old_name; - struct excludes *x = excludes; - while (x) { - if (fnmatch(x->path, pathname, 0) == 0) - return 0; - x = x->next; - } + int i; + + /* Paths outside are not touched regardless of "--include" */ if (0 < prefix_length) { int pathlen = strlen(pathname); if (pathlen <= prefix_length || memcmp(prefix, pathname, prefix_length)) return 0; } - return 1; + + /* See if it matches any of exclude/include rule */ + for (i = 0; i < limit_by_name.nr; i++) { + struct string_list_item *it = &limit_by_name.items[i]; + if (!fnmatch(it->string, pathname, 0)) + return (it->util != NULL); + } + + /* + * If we had any include, a path that does not match any rule is + * not used. Otherwise, we saw bunch of exclude rules (or none) + * and such a path is used. + */ + return !has_include; } + static void prefix_one(char **name) { char *old_name = *name; @@ -3154,10 +3170,12 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix) continue; } if (!prefixcmp(arg, "--exclude=")) { - struct excludes *x = xmalloc(sizeof(*x)); - x->path = arg + 10; - x->next = excludes; - excludes = x; + add_name_limit(arg + 10, 1); + continue; + } + if (!prefixcmp(arg, "--include=")) { + add_name_limit(arg + 10, 0); + has_include = 1; continue; } if (!prefixcmp(arg, "-p")) { -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html