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

Duy Nguyen [off-list ref] writes:

On Fri, Jan 25, 2013 at 1:22 AM, Junio C Hamano [off-list ref] wrote:

quoted

Duy Nguyen [off-list ref] writes:

quoted

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.