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

From: Duy Nguyen <pclouds@gmail.com>
Date: Jan 23, 2013, 01:04 UTC
Message-ID: <CACsJy8DiVy7tcG_t2JENKoPSFWV24Tneh4q=upPPJML4VESMag@mail.gmail.com>
In-Reply-To: <7v1udcn84w.fsf@alter.siamese.dyndns.org>

On Wed, Jan 23, 2013 at 5:51 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 8 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> We obviously do not want to set FNM_PATHNAME when we are not
>> substituting fnmatch() with wildmatch(), but I wonder if it may make
>> sense to unconditionally use WM_PATHNAME semantics when we build the
>> system with USE_WILDMATCH and calling wildmatch() in this codepath.
>> Users can always use "*/**/*" in place of "*" in their patterns
>> where they want to ignore directory boundaries.

If we do that, we need to do the same in tree_entry_interesting(). In other words, pathspec learns the new glob syntax. It's fine for an experimental flag like USE_WILDMATCH. But after fnmatch is replaced by wildmatch unconditionally (thus USE_WILDMATCH becomes obsolete), then what? Should people who expects new syntax with USE_WILDMATCH continue to have new syntax? How does a user know which syntax may be used in his friend's "git" binary?

On a related subject, I've been meaning to write a mail about the other use of patterns in git (e.g. in git-branch, git-tag, git-apply...) but never got around to. Some use without FNM_PATHNAME, some do and the document does not go into details about the differences. We might want to unify the syntax there too. It'll probably break backward compatibility so we can do the same when we switch pathspec syntax.

Show 5 quoted lines

> Another possibility, which _might_ make more practical sense, is to
> update dowild() so that any pattern that has (^|/)**(/|$) in it
> implicitly turns the WM_PATHNAME flag on.  There is no reason for
> the user to feed it a double-asterisk unless it is an attempt to
> defeat some directory boundaries,

They may also put "**" by mistake (or realize they just put "**" but too lazy to remove one asterisk).

> so we already know that the user
> expects WM_PATHNAME behaviour at that point.  Otherwise, the user
> would have simply said '*' instead, wouldn't he?

The only problem I see is, without the version string, there's no way to know if "**" is supported. Old git versions will happily take "**" and interpret as "*". When you advise someone to use "**" you might need to add "check if you have this version of git". This problem does not exist with pathspec magic like :(glob)

> I said "_might_" because it sounds a bit too magical to my taste.
> I dunno.

-- 
Duy