From: Junio C Hamano <gitster@pobox•com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Cc: git@vger•kernel.org, christian.couder@gmail•com,
Matthieu.Moy@grenoble-inp•fr, toralf.foerster@gmx•de
Subject: Re: [PATCH] Support pathspec magic :(exclude) and its short form :-
Date: Wed, 20 Nov 2013 15:48:24 -0800 [thread overview]
Message-ID: <xmqqhab663ef.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1384911691-11664-1-git-send-email-pclouds@gmail.com> ("Nguyễn Thái Ngọc Duy"'s message of "Wed, 20 Nov 2013 08:41:31 +0700")
Nguyễn Thái Ngọc Duy <pclouds@gmail•com> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail•com>
> ---
> 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 */
> +}
next prev parent reply other threads:[~2013-11-20 23:48 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-16 12:39 RFC: git bisect should accept "paths-to-be-excluded" Toralf Förster
2013-09-17 7:26 ` Christian Couder
2013-09-17 8:21 ` Matthieu Moy
2013-09-17 9:03 ` Christian Couder
2013-09-17 11:45 ` Duy Nguyen
2013-09-17 17:02 ` Junio C Hamano
2013-09-17 18:12 ` Piotr Krukowiecki
2013-09-17 19:04 ` Junio C Hamano
2013-09-17 19:41 ` Piotr Krukowiecki
2013-09-17 20:47 ` Junio C Hamano
2013-09-18 2:22 ` Duy Nguyen
2013-11-20 1:41 ` [PATCH] Support pathspec magic :(exclude) and its short form :- Nguyễn Thái Ngọc Duy
2013-11-20 23:48 ` Junio C Hamano [this message]
2013-11-21 2:10 ` Duy Nguyen
2013-11-21 18:43 ` Junio C Hamano
2013-09-17 16:23 ` RFC: git bisect should accept "paths-to-be-excluded" Toralf Förster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqhab663ef.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Matthieu.Moy@grenoble-inp$(echo .)fr \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
--cc=toralf.foerster@gmx$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox