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

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

Eric Sunshine [off-list ref] writes:

quoted

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