Re: Fwd: Possibly nicer pathspec syntax?

[Junio C Hamano <gitster@pobox.com>]

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

> People don't expect set theory from their pathspecs. They expect their
> pathspecs to limit the output. They've learnt that within a
> subdirectory, the pathspec limits to that subdirectory. And now it
> suddenly starts showing things outside the subdirectory?
>
> At that point no amount of "but but think about set theory" will
> matter, methinks.

I do not feel too strongly about it, either, but when we invoke
"what would people who go down into subdirectories expect", the
issue becomes a lot bigger.

"git diff/log" without any pathspec in a subdirectory still shows
the whole diff.  "git grep" looks for hits inside that subdirectory,
on the other hand.

I think the former design decision is mostly a historical accident.
"The project tree as the whole is what matters" was one reason, and
another is that historically we didn't have ":/"--defaulting to the
whole tree and telling people to give "." was easier than defaulting
to the current and telling people to give "../.." after counting how
many levels deep you started at.  If we were designing the system
today with "." and ":/", I wouldn't be surprised if we chose "always
limit to cwd if there is no pathspec" for any command for the sake
of consistency.  We can easily say "if you want whole-tree, pass :/"
instead.

But we do not live in that world, and I do not think change them to
make things consistent is what we are discussing in this thread.
Given users accept and expect that "diff" without pathspec is whole
tree, while "grep" without pathspec is limited to cwd, what is the
reasonable definition of "everything from which negative ones are
excluded"?  That is the question I am trying to answer.

As Mike mentioned earlier in the thread, if we used "." (limit to
cwd) for "diff/log" when there are only negative pathspec elements,
the resulting UI would become inconsistent within a single command,
as in my world view, giving no pathspec means "work on everything
you would ordinarily work on", a positive pathspec means "among
everything you would ordinarily work on, only work on the ones that
match this pattern", and giving only a negative one ought to mean
"among everything you would ordinarily work on, only work on the
ones that do NOT match this pattern."

>  pathspec.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/pathspec.c b/pathspec.c
> index 7ababb315..2a91247bc 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -224,6 +224,12 @@ static const char *parse_short_magic(unsigned *magic, const char *elem)
>  		char ch = *pos;
>  		int i;
>  
> +		// Special case alias for '!'

/* style? */

> +		if (ch == '^') {
> +			*magic |= PATHSPEC_EXCLUDE;
> +			continue;
> +		}
> +
>  		if (!is_pathspec_magic(ch))
>  			break;
>  
> +	/*
> +	 * If everything is an exclude pattern, add one positive pattern
> +	 * that matches everyting. We allocated an extra one for this.
> +	 */
> +	if (nr_exclude == n) {
> +		init_pathspec_item(item + n, 0, "", 0, "");
> +		pathspec->nr++;
> +	}

I somehow do not think this is correct.  I expect

	cd t && git grep -e foo -- :^perf/

to look into things in 't' except for things in 't/perf', but the
above will grab hits from ../Documentation/ etc.  We need to pay
attention to PATHSPEC_PREFER_CWD bit in the flags word, I think.

A real change probably wants to become a two-patch series (one for
the new :^ alias, the other is for "everything except...") with log
message ;-)