Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Watchman does not understand micromatch globs #145

Open
conartist6 opened this issue Jul 2, 2019 · 9 comments · May be fixed by #147
Open

Watchman does not understand micromatch globs #145

conartist6 opened this issue Jul 2, 2019 · 9 comments · May be fixed by #147

Comments

@conartist6
Copy link

conartist6 commented Jul 2, 2019 •
edited

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
Copy link
Author

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

@conartist6
Copy link
Author

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 conartist6 linked a pull request Jul 3, 2019 that will close this issue
Open
@conartist6
Copy link
Author

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

@conartist6
Copy link
Author

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
Copy link
Author

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

@conartist6
Copy link
Author

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

@conartist6
Copy link
Author

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
Copy link
Author

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

@conartist6
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support micromatch patterns in watchman conartist6/sane
1 participant
@conartist6