Re: [PATCH v3 01/10] wildmatch: fix "**" special case

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 25, 2013, 04:55 UTC
Message-ID: <7vip6l7tex.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CACsJy8Bq8aQ69twWtwHyNzez2OwTN1wHxqHwb_AM-Qo4TUuv8Q@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 15 quoted lines

> On Fri, Jan 25, 2013 at 1:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Duy Nguyen <pclouds@gmail.com> writes:
>>
>>> I don't think we need to support two different sets of wildcards in
>>> the long run. I'm thinking of adding ":(glob)" with WM_PATHNAME.
>>> Pathspec without :(glob) still uses the current wildcards (i.e. no
>>> FNM_PATHNAME). At some point, like 2.0, we either switch the behavior
>>> of patterns-without-:(glob) to WM_PATHNAME, or just disable wildcards
>>> when :(glob) is not present.
>>
>> Yeah, I think that is sensible.
>>
>> I am meaning to merge your retire-fnmatch topic to 'master'.
>
> I thought you wanted it to stay in 'next' longer :-)

I only said "a bit longer than other topics", and the topic has been cooking on 'next' for three weeks by now, which is a lot longer. I haven't been keeping exact tallies, but trivial ones graduate from 'next' to 'master' in 3 to 5 days, ones with medium complexity in 7 to 10 days on average, I think.

> That looks ok. You may want to mention that "**" syntax is enabled in
> .gitignore and .gitattributes as well (even without USE_WILDMATCH).

Yeah, I forgot about that behaviour, and it is a bit confusing story. You did that in 237ec6e (Support "**" wildcard in .gitignore and .gitattributes, 2012-10-15).

c41244e (wildmatch: support "no FNM_PATHNAME" mode, 2013-01-01) that is part of the retire-fnmatch topic, changes the behaviour of wildmatch() with respect to /**/ magic drastically, but everything cancels out in the end:

 - With the current master without nd/retire-fnmatch, wildmatch()
   always works in WM_PATHNAME mode wrt '/**/'; match_pathname()
   that is used for attr/ignore matching uses wildmatch() so a
   pattern "**/Makefile" matches "Makefile" at the top, affected by
   the "**/" magic;

 - After merging nd/retire-fnmatch, wildmatch() needs WM_PATHNAME
   passed in order to grok '/**/' magic, but match_pathname() is
   changed in the same commit to pass WM_PATHNAME, so the "**/"
   magic is retained for ignore/attr matching.

> We
> could even stop the behavior change in for-each-ref (FNM_PATHNAME in
> general) but I guess because it's a nice regression, nobody would
> complain.

That is not a "regression" (whose definition is "we inadvertently changed the behaviour and fixed that once, but the breakage came back"). It is a behaviour change that is backward incompatible.

I would agree that it is a nice behaviour change, though ;-)

Thanks.