Re: RFC: git bisect should accept "paths-to-be-excluded"

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 17, 2013, 17:02 UTC
Message-ID: <xmqqsix3z8ie.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CACsJy8AEoUUat-1smJ1BmDuDBLseWf8oZ+EJyuadSLncb1UMSw@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 27 quoted lines

> On Tue, Sep 17, 2013 at 4:03 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> On Tue, Sep 17, 2013 at 10:21 AM, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>> Christian Couder <christian.couder@gmail.com> writes:
>>>
>>>> In practice though, as git bisect is a kind of binary search, if what
>>>> you want to exclude is exclusively touched by half the commits, it
>>>> will only add one more bisection step if you don't exclude it.
>>>
>>> Actually, I think the same remark would apply to any other Git command
>>> that deal with a set of revisions. If you want to review code with "git
>>> log -p", but you don't care about a subdirectory, you may want a "git
>>> log -p --ignore-dir foo/" or so, too.
>>
>> Yeah, and there was a patch series about that 2 years ago:
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/182830/
>
> And that's just one of the few attempts if I remember correctly. I
> guess it's time revisit it. A few things to sort out before we get to
> the implementation:
>
> Support flat or nested negation (i.e.include A, ignore A/B, but
> include A/B/C..). Nested thing complicates things so I'm towards the
> flat exclusion (exclude B means all inside B, no buts nor excepts) and
> probably cover most use cases

Yeah, it is easy to say that

	git log -- A ':(exclude)A/B' A/B/C

has two positive (A, A/B/C) and one negative (A/B), and then the most specific one A/B/C matches a path A/B/C/D and hence A/B/C/D is included.

But to actually _design_ it, there are ambiguities that makes understanding and explaining the semantics, especially given pathspecs can have wildcards, icase matches, etc. For example, is ":(exclude,icase)A/B/?" more specific than "A/?/C" or less?

So I tend to agree that we should aim for an easier to explain, if less capable, approach.

> Interaction with "git grep --depth"

I am not sure how that affects anything. Conceptually, isn't "--depth" an independent axis to filter out paths that have too many components after given positive pathspec elements? E.g. given

	git grep --depth=2 pattern -- A B/C

we will grab paths from two levels starting at A and B/C (so A/1/2 and B/C/1/2 may hit but not A/1/2/3 nor B/C/1/2/3). Shouldn't negative pathspecs just filter that depth filtering, i.e. if you have ":(exclude)*/1/*", even though both "A/1/2" and "A/a/b" may pass the --depth=2 filter, the former is excluded while the latter is not.

Show 5 quoted lines

> Syntax. I guess --ignore (or --exclude) is more intuitive than
> ":(exclude)something" but then it might collide with existing options
> (I did not check if --ignore or --exclude is used anywhere though).
> The latter also enables combining with other filters, such as
> case-insensitive matching..

I do not think it is an option to do this with any mechanism other than negative pathspecs.