From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Tue, 28 Sep 2010 22:31:08 GMT
Subject: Re: [PATCH 0/6] Introduce pathspec struct
Message-ID: <AANLkTinZ=dx1nBXTw0E=nLOmncsgNb7wv1-+ddWzPU+e@mail.gmail.com>
In-Reply-To: <7v7hi6us35.fsf@alter.siamese.dyndns.org>

2010/9/28 Junio C Hamano <gitster@pobox.com>:
> Just a couple of quick notes.
>
>  - I had to eject Bo's "log -L range path" series in order to push this
>   out on 'pu' as the range stuff adds new callsites to the old pathspec
>   API.
>
>   This is tentative and does not mean Bo's series is getting rejected;
>   I'd want to get its command line parsing around the pathnames fixed
>   anyway but I suspect the affected codepath would overlap between the
>   two series.  Help is appreciated.

I'll have a look.

>  - I do not think either !pattern nor ^pattern is particularly a good way
>   to express negative pathspecs.  My gut feeling is (I have not thought
>   this through nor clearly enough; note the time of this message) that it
>   would be the cleanest at the UI level to introduce negative patterns as
>   arguments to a separate command line flag, e.g.
>
>   $ git log --exclude "Doc*" master..pu -- '*.txt'
>   $ git grep --exclude "t/" -e 'test .*-L' -- '*.sh'

I was writing "but you would lose the ability to mix negative and
positive pathspecs together, something like 'exclude Documentation
except Documentation/technical'", but then we can have negative
excludes too:

$ git log --exclude Documentation --exclude "!Documentation/technical"
master..pu -- '*.txt'

does not sound too twisted to understand (I hope).

>  - David's "git grep --exclude-dir D" topic should be able to internally
>   use the same negative pathspec mechanism.  At the command line level,
>   it allows (and needs to allow) only the leading prefix (which is how
>   GNU grep's --exclude-dir works), but it makes tons of sense for us to
>   allow "--exclude $pattern" from the command line, and share the
>   mechanism internally between the two.

Yes, eventually. But
 - tree_entry_interesting() needs (a bit complex) rework to have
wildcard matching capability
 - then I am still not sure how negative pathspecs should be done properly

Both may take me weeks to come up with something sensible. If David
needs "git grep --exclude-dir" now, he should keep working on
builtin/grep.c as he's doing now (maybe change --exclude-dir to
--exclude). If my work on negative pathspec has a result, sure I will
remove pathspec_matches() from builtin/grep.c, but his work on the
command line interface _and tests_ won't be wasted.
-- 
Duy