Linus Torvalds [off-list ref] writes:
No. The thing is, "git diff" is relative too - for path
specifications. And the negative entries are pathspecs - and they act
as relative ones.
IOW, that whole
cd drivers
git diff A..B -- net/
will actually show the diff for drivers/net - so the pathspec very
much acts as relative to the cwd.
But that is not what I was talking about. Let's simplify. I'd say
for any command that acts on "everything" when pathspec is not
given, the two sets of actual paths affected by these two:
git cmd -- "net/"
git cmd -- ":!net/"
should have no overlap (obviously) and when you take union of the
two sets, that should equal to
git cmd --
i.e. no pathspecs.
quoted
2. I am not sure what ctype.c change is about. Care to elaborate?
I didn't see the need for it either until I made the rest of the
patch, and it didn't work at all.
The pathspec.c code uses "if (is_pathspec_magic(..))" to test whether
a character is a short magiic pathspec character. But '^' wasn't in
that set, because it was already marked as being (only) in the regex
set.
Does that whole is_pathspec_magic() thing make any sense when we have
an array that specifies the special characters we react to? No it does
not.
But it is what the code does, and I just made that code work.
Ah, OK.
On Tue, Feb 7, 2017 at 7:12 PM, Junio C Hamano [off-list ref] wrote:
But that is not what I was talking about. Let's simplify. I'd say
for any command that acts on "everything" when pathspec is not
given, the two sets of actual paths affected by these two:
git cmd -- "net/"
git cmd -- ":!net/"
should have no overlap (obviously) and when you take union of the
two sets, that should equal to
git cmd --
i.e. no pathspecs.
Well, as mentioned, I won't ever care. I'm certainly ok with the "make
the default positive entry be everything".
I just suspect that from a user perspective that actually delves into
the subdirectories, the much bigger question will be: "I gave you a
pathspec, and suddenly you start giving me stuff from outside the area
entirely".
And then you can say "well, just add '.' to the pathspec", and you'd
be right, but I still think it's not what a naive user would expect.
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.
But I really don't feel strongly about it. The path I sent out (and
the slightly modified version attached in this email) actually acts
the way you suggest. It's certainly the simplest implementation. I
just suspect it's not the implementation people who go down into
subdirectories would want/expect.
quoted
quoted
2. I am not sure what ctype.c change is about. Care to elaborate?
I didn't see the need for it either until I made the rest of the
patch, and it didn't work at all.
The pathspec.c code uses "if (is_pathspec_magic(..))" to test whether
a character is a short magiic pathspec character. But '^' wasn't in
that set, because it was already marked as being (only) in the regex
set.
Does that whole is_pathspec_magic() thing make any sense when we have
an array that specifies the special characters we react to? No it does
not.
But it is what the code does, and I just made that code work.
Ah, OK.
Side note: here's an alternative patch that avoids that issue
entirely, and also avoids a problem with prefix_magic() being uphappy
when one bit shows up multiple times in the array.
It's slightly hacky in parse_short_magic(), but it's smaller and
simpler, and avoids the two subtle cases. No need for ctype changes,
and no issues with prefix_magic() being somewhat stupid.
Linus