Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> 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.
> 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 ":(...)".
> 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.
> 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.
> @@ -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 ;-)?
> @@ -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 */
> +}
--
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