Re: [PATCH v2 1/1] pathspec: warn for a no-glob entry that contains `**`

[Jeff King <peff@peff.net>]

On Thu, Mar 25, 2021 at 08:02:31PM -0700, Junio C Hamano wrote:

> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > I haven't been following the discussion, but is there a reason we need
> > to penalize the user with a warning rather than helping, for instance
> > by inferring ":(glob)" in the presence of `/**/` if not otherwise
> > countermanded by ":(literal)" or whatnot?
> 
> Two reasons I can think of offhand are
> 
>  - How /**/ is interpreted is not the only thing that is different
>    between the normal mode and the glob magic mode.  IIRC, an
>    asterisk * or a question mark ? matches slash in normal mode (it
>    started out as fnmatch() without FNM_PATHNAME).  Should we warn
>    about ":(glob)" if somebody asks for "foo*", "*foo", or
>    "foo*bar".  If not, why shouldn't?
> 
>  - Thers is no explicit magic that says "there is no magic" to
>    countermand such a DWIM.

I do wonder if this distinction creates more harm than good.

As somebody who has never used ":(glob)" myself, I was confused about
what it even does (and it was not easy to find the documentation; I
ended up finding the original commit in the history first!).

We have three modes:

  - no globbing

  - globbing with fnmatch(), with FNM_PATHNAME according to the docs

  - globbing with wildmatch

You may notice that I would call both of those latter two "globbing",
but only one of them is triggered by the ":(glob)" magic. :)

This just seems really confusing, and I wonder if anybody would be that
sad if we just used wildmatch everywhere. The original bd30c2e484
(pathspec: support :(glob) syntax, 2013-07-14) even says:

  The old fnmatch behavior is considered deprecated and discouraged to
  use.

but I guess it would be backwards-incompatible.

Maybe it would be less confusing if we named the three states
explicitly:

  :(literal)
  :(fnmatch)
  :(wildmatch)

(and keeping :(glob) as a synonym for compatibility).

-Peff