Re: [PATCH] Support pathspec magic :(exclude) and its short form :-
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:59:17
Nguyễn Thái Ngọc Duy [off-list ref] writes:
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted> --- This is yet another stab at the negative pathspec thing. It's not ready yet (there are a few XXXs) but I could use some feedback regarding the interface, or the behavior. It looks better this time now that pathspec magic is supported (or maybe I'm just biased). For :(glob) or :(icase) you're more likely to enable it for all pathspec, i.e. --glob-pathspecs. But I expect :(exclude) to be typed more often (it does not make sense to add --exclude-pathspecs to exclude everything), which is why I add the short form for it. We don't have many options that say "negative" in short form. Either '!', '-' or '~'. '!' is already used for bash history expansion. ~ looks more like $HOME expansion. Which left me '-'.
I agree with your decision to reject ~, but "!not-this-pattern" is very much consistent with the patterns used in .gitignore (and the "--exclude <pattern>" option), so avoiding "!" and introducing an inconsistent "-" only to appease bash leaves somewhat a funny taste in my mouth.
quoted hunk
Documentation/glossary-content.txt | 5 ++++ builtin/add.c | 5 +++- dir.c | 50 +++++++++++++++++++++++++++++++----- pathspec.c | 9 ++++++- pathspec.h | 4 ++- tree-walk.c | 52 +++++++++++++++++++++++++++++++++++--- 6 files changed, 112 insertions(+), 13 deletions(-)diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index e470661..f7d7d8c 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt@@ -377,6 +377,11 @@ full pathname may have special meaning: - Other consecutive asterisks are considered invalid. + Glob magic is incompatible with literal magic. + +exclude `-`;; + After a path matches any non-exclude pathspec, it will be run + through all exclude pathspec. If it matches, the path is + ignored. -- + Currently only the slash `/` is recognized as the "magic signature",
No longer, no? "magic signature" is a non-alphanumeric that follows the ':' introducer, as opposed to "magic words" that are in ":(...)".
quoted hunk
diff --git a/builtin/add.c b/builtin/add.c index 226f758..0df73ae 100644 --- a/builtin/add.c +++ b/builtin/add.c@@ -540,10 +540,13 @@ int cmd_add(int argc, const char **argv, const char *prefix) PATHSPEC_FROMTOP | PATHSPEC_LITERAL | PATHSPEC_GLOB | - PATHSPEC_ICASE); + PATHSPEC_ICASE | + PATHSPEC_EXCLUDE); for (i = 0; i < pathspec.nr; i++) { const char *path = pathspec.items[i].match; + if (pathspec.items[i].magic & PATHSPEC_EXCLUDE) + continue; if (!seen[i] && ((pathspec.items[i].magic & (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
So "git add ':(exclude)junk/' '*.c'" to add all .c files except for the ones in the 'junk/' directory may find that ':(exclude)junk/' matched nothing (because there is no .c file in there), and that is not an error. It makes sense to me.
quoted hunk
diff --git a/dir.c b/dir.c index 23b6de4..e2df82f 100644 --- a/dir.c +++ b/dir.c@@ -126,10 +126,13 @@ static size_t common_prefix_len(const struct pathspec *pathspec) PATHSPEC_MAXDEPTH | PATHSPEC_LITERAL | PATHSPEC_GLOB | - PATHSPEC_ICASE); + PATHSPEC_ICASE | + PATHSPEC_EXCLUDE); for (n = 0; n < pathspec->nr; n++) { size_t i = 0, len = 0, item_len; + if (pathspec->items[n].magic & PATHSPEC_EXCLUDE) + continue; if (pathspec->items[n].magic & PATHSPEC_ICASE) item_len = pathspec->items[n].prefix; else
Likewise. Exclusion does not participate in the early culling with the common prefix.
quoted hunk
@@ -1375,11 +1407,17 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru PATHSPEC_MAXDEPTH | PATHSPEC_LITERAL | PATHSPEC_GLOB | - PATHSPEC_ICASE); + PATHSPEC_ICASE | + PATHSPEC_EXCLUDE); if (has_symlink_leading_path(path, len)) return dir->nr; + /* + * XXX: exclude patterns are treated like positive ones in + * create_simplify! This is not wrong, but may make path + * filtering less efficient. + */
True, but "git add ':(exclude)a/b/c' a/b" would not suffer. And those who do "git add ':(exclude)a/b' a/b/c" deserve it, no ;-)?
quoted hunk
@@ -427,6 +430,10 @@ void parse_pathspec(struct pathspec *pathspec, pathspec->magic |= item[i].magic; } + if (nr_exclude == n) + die(_("There is nothing to exclude from by :(exclude) patterns.\n" + "Perhaps you forgot to add either ':/' or '.' ?"));
;-).
+enum interesting tree_entry_interesting(const struct name_entry *entry,
+ struct strbuf *base, int base_offset,
+ const struct pathspec *ps)
+{
+ enum interesting positive, negative;
+ positive = tree_entry_interesting_1(entry, base, base_offset, ps, 0);
+
+ /*
+ * # | positive | negative | result
+ * -----+----------+----------+-------
+ * 1..4 | -1 | * | -1
+ * 5..8 | 0 | * | 0
+ * 9 | 1 | -1 | 1
+ * 10 | 1 | 0 | 1
+ * 11 | 1 | 1 | 0
+ * 12 | 1 | 2 | 0
+ * 13 | 2 | -1 | 2
+ * 14 | 2 | 0 | 2
+ * 15 | 2 | 1 | 0
+ * 16 | 2 | 2 | -1
+ */Not sure what this case-table means...
+ if (!(ps->magic & PATHSPEC_EXCLUDE) || + positive <= entry_not_interesting) /* #1..#8 */ + return positive; + + negative = tree_entry_interesting_1(entry, base, base_offset, ps, 1); + + if (negative <= entry_not_interesting) /* #9, #10, #13, #14 */ + return positive; + if ((positive == entry_interesting && + negative >= entry_interesting) || /* #11, #12 */ + (positive == all_entries_interesting && + negative == entry_interesting)) /* #15 */ + return entry_not_interesting; + return all_entries_not_interesting; /* #16 */ +}