search

amasad / sane

sane aims to be fast, small, and reliable filesystem watcher. No bells and whistles, just change events.
387 stars 66 forks source link

Watchman does not understand micromatch globs #145

Open conartist6 opened 5 years ago

conartist6 commented 5 years ago

If a version of watchman is in use that has the wildmatch capability, sane will delegate checking of glob patterns to watchman. Unfortunately watchman's globbing is not micromatch compatible. It seemingly just supports ** and *.

It seems like this issue could be fixed several ways:

  1. Never delegate glob inclusion to watchman. Pros: simple. Cons: could hurt perf in situations where watchman's efficiency is most needed.
  2. Do not delegate to watchman if the glob pattern contains unsupported syntaxes. Perhaps use the glob-parse package to determine which syntaxes are present.
  3. Similar to above, but warn the user if unsupported syntaxes are present.
  4. Where possible attempt to explode globs. E.g. convert *.{css,scss} to *.css and *.scss. Where it is unreasonable to do this, either fall back or warn the user.
conartist6 commented 5 years ago

I think the thing to do is to parse the glob, then potentially warn and fall back. I'll work on a PR.

conartist6 commented 5 years ago

Hmm, it's more interesting than that I guess. Here's what watchman actually supports for globbing: https://github.com/facebook/watchman/blob/061e6abd75449256e00cd8880b94d030f3360ee4/tests/wildmatch_test.json

conartist6 commented 5 years ago

I opened a PR for option 2 since it's the simplest correct behavior.

conartist6 commented 5 years ago

Also it seems like wildglob supports [!...] and [^...] for negated character classes where ... is characters which should not be part of the class. It seems that micromatch only supports [^...].

conartist6 commented 5 years ago

Fortunately it should be possible to use the parser to translate from [!...] to [^...]

conartist6 commented 5 years ago

er, actually I guess the translation would be from [!...] to [\!...]

conartist6 commented 5 years ago

I'm also going to follow up with watchman and see if it would be possible for them to gain more support by using fnmatch

conartist6 commented 5 years ago

Oh, and it looks like sane can gain at least braces support for watchman using micromatch.braces(glob, { expand: true })

conartist6 commented 5 years ago

Unfortunately I none of the glob parsers I've found (save glob-parse, which is hardly well supported) give me the flexibility to identify the source text of POSIX brackets glob segments, so for the moment I think it's OK to let that smaller issue persist.