Re: [PATCH] gitignore: warn about pointless syntax

[Jeff King <peff@peff.net>]

On Mon, Jan 09, 2012 at 11:43:21AM -0800, Junio C Hamano wrote:

> >> +static inline void check_bogus_wildcard(const char *file, const char *p)
> >> +{
> >> +	if (strstr(p, "**") == NULL)
> >> +		return;
> >> +	warning(_("Pattern \"%s\" from file \"%s\": Double asterisk does not "
> >> +		"have a special meaning and is interpreted just like a single "
> >> +		"asterisk.\n"), file, p);
> >
> > Wouldn't this also match the meaningful "foo\**"?
> 
> Yes.
> 
> But trying to catch that false positive by checking one before "**"
> against a backslash is not a way to do so as it will then turn "foo\\**"
> into a false negative, and you would end up reimplementing fnmatch if you
> really want to avoid false positives nor negatives. At that point, you may
> be better off implementing git_fnmatch() instead that understands the
> double-asterisk that works as some people may expect it to work ;-).

You only have to implement proper backslash decoding, so I think it is
not as hard as reimplementing fnmatch:

  enum { NORMAL, QUOTED, WILDCARD } context = NORMAL;
  for (i = 0; p[i]; i++) {
          if (context == QUOTED)
                  context = NORMAL;
          else if (p[i] == '\\')
                  context = QUOTED;
          else if (p[i] == '*') {
                  if (context == WILDCARD) {
                        warning(...);
                        return;
                  }
                  context = WILDCARD;
          }
          else
                  context = NORMAL;
  }

That being said, if this is such a commonly-requested feature that we
need to be detecting and complaining about its absence, I would be much
more in favor of simply implementing it. Surely fnmatch is not that hard
to write, or we could lift code from glibc or even rsync.

Which perhaps was what you are getting at, but I am happy to say it more
explicitly. :)

-Peff