git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
* [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns
@ 2017-02-08  5:14 Linus Torvalds
  2017-02-08 13:23 ` Cornelius Weig
  2017-02-08 21:59 ` Junio C Hamano
  0 siblings, 2 replies; 4+ messages in thread
From: Linus Torvalds @ 2017-02-08  5:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List


From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 7 Feb 2017 21:08:15 -0800
Subject: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns

Instead of erroring out and telling the user that they should add a 
positive pattern that covers everything else, just _do_ that.

For commands where we honor the current cwd by default (ie grep, ls-files 
etc), we make that default positive pathspec be the current working 
directory.  And for commands that default to the whole project (ie diff, 
log, etc), the default positive pathspec is the whole project.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 pathspec.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index ecad03406..d8f78088c 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -522,7 +522,7 @@ void parse_pathspec(struct pathspec *pathspec,
 	}
 
 	pathspec->nr = n;
-	ALLOC_ARRAY(pathspec->items, n);
+	ALLOC_ARRAY(pathspec->items, n+1);
 	item = pathspec->items;
 	prefixlen = prefix ? strlen(prefix) : 0;
 
@@ -546,10 +546,16 @@ 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 '.' ?"));
-
+	/*
+	 * If everything is an exclude pattern, add one positive pattern
+	 * that matches everyting. We allocated an extra one for this.
+	 */
+	if (nr_exclude == n) {
+		if (!(flags & PATHSPEC_PREFER_CWD))
+			prefixlen = 0;
+		init_pathspec_item(item + n, 0, prefix, prefixlen, "");
+		pathspec->nr++;
+	}
 
 	if (pathspec->magic & PATHSPEC_MAXDEPTH) {
 		if (flags & PATHSPEC_KEEP_ORDER)
-- 
2.12.0.rc0.1.g02555c1b2.dirty


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns
  2017-02-08  5:14 [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns Linus Torvalds
@ 2017-02-08 13:23 ` Cornelius Weig
  2017-02-08 21:59 ` Junio C Hamano
  1 sibling, 0 replies; 4+ messages in thread
From: Cornelius Weig @ 2017-02-08 13:23 UTC (permalink / raw)
  To: Linus Torvalds, Junio C Hamano; +Cc: Git Mailing List

Again, as Duy pointed out this should be documented.

How about something like this:

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index f127fe9..781cde3 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -387,7 +387,9 @@ Glob magic is incompatible with literal magic.
 exclude;;
        After a path matches any non-exclude pathspec, it will be run
        through all exclude pathspec (magic signature: `!` or `^`). If it
-       matches, the path is ignored.
+       matches, the path is ignored. If only exclude pathspec are given,
+       the exclusion is applied to the result set as if invoked without any
+       pathspec.
 --
 
 [[def_parent]]parent::

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns
  2017-02-08  5:14 [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns Linus Torvalds
  2017-02-08 13:23 ` Cornelius Weig
@ 2017-02-08 21:59 ` Junio C Hamano
  2017-02-08 22:16   ` Linus Torvalds
  1 sibling, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2017-02-08 21:59 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List

Linus Torvalds <torvalds@linux-foundation.org> writes:

> @@ -546,10 +546,16 @@ 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 '.' ?"));
> -
> +	/*
> +	 * If everything is an exclude pattern, add one positive pattern
> +	 * that matches everyting. We allocated an extra one for this.
> +	 */
> +	if (nr_exclude == n) {
> +		if (!(flags & PATHSPEC_PREFER_CWD))
> +			prefixlen = 0;
> +		init_pathspec_item(item + n, 0, prefix, prefixlen, "");
> +		pathspec->nr++;
> +	}
>  
>  	if (pathspec->magic & PATHSPEC_MAXDEPTH) {
>  		if (flags & PATHSPEC_KEEP_ORDER)

Thanks.  Even though the current code does not refer to the original
prefixlen after the added hunk, I'd prefer not to destroy it to
avoid future troubles, so I'll queue with a bit of tweak there,
perhaps like the attached.

Also this has an obvious fallout to the tests, whose (minimum) fix
is rather trivial.

Thanks.

 pathspec.c                  | 7 +++----
 t/t6132-pathspec-exclude.sh | 6 ++++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index d8f78088c8..b961f00c8c 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -522,7 +522,7 @@ void parse_pathspec(struct pathspec *pathspec,
 	}
 
 	pathspec->nr = n;
-	ALLOC_ARRAY(pathspec->items, n+1);
+	ALLOC_ARRAY(pathspec->items, n + 1);
 	item = pathspec->items;
 	prefixlen = prefix ? strlen(prefix) : 0;
 
@@ -551,9 +551,8 @@ void parse_pathspec(struct pathspec *pathspec,
 	 * that matches everyting. We allocated an extra one for this.
 	 */
 	if (nr_exclude == n) {
-		if (!(flags & PATHSPEC_PREFER_CWD))
-			prefixlen = 0;
-		init_pathspec_item(item + n, 0, prefix, prefixlen, "");
+		int plen = (!(flags & PATHSPEC_PREFER_CWD)) ? 0 : prefixlen;
+		init_pathspec_item(item + n, 0, prefix, plen, "");
 		pathspec->nr++;
 	}
 
diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh
index d51595cf6b..9dd5cde5fc 100755
--- a/t/t6132-pathspec-exclude.sh
+++ b/t/t6132-pathspec-exclude.sh
@@ -25,8 +25,10 @@ EOF
 	test_cmp expect actual
 '
 
-test_expect_success 'exclude only should error out' '
-	test_must_fail git log --oneline --format=%s -- ":(exclude)sub"
+test_expect_success 'exclude only no longer errors out' '
+	git log --oneline --format=%s -- . ":(exclude)sub" >expect &&
+	git log --oneline --format=%s -- ":(exclude)sub" >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 't_e_i() exclude sub' '

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns
  2017-02-08 21:59 ` Junio C Hamano
@ 2017-02-08 22:16   ` Linus Torvalds
  0 siblings, 0 replies; 4+ messages in thread
From: Linus Torvalds @ 2017-02-08 22:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

On Wed, Feb 8, 2017 at 1:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Thanks.  Even though the current code does not refer to the original
> prefixlen after the added hunk, I'd prefer not to destroy it to
> avoid future troubles, so I'll queue with a bit of tweak there,
> perhaps like the attached.

Yeah, I considered that. Along with just passing in a NULL prefix
string too for that case. Not that it matters.

So ack on that change,

             Linus

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-08  5:14 [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns Linus Torvalds
2017-02-08 13:23 ` Cornelius Weig
2017-02-08 21:59 ` Junio C Hamano
2017-02-08 22:16   ` Linus Torvalds

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror http://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox