Improve source pattern handling #730
Conversation
|
By analyzing the blame information on this pull request, we identified @sotojuan, @zhaozhiming and @forresst to be potential reviewers |
| @@ -858,17 +859,46 @@ group('chokidar is installed', function (beforeEach, test, group) { | |||
| }); | |||
| }); | |||
|
|
|||
| test('ignores dependencies outside of the current working directory', function (t) { | |||
| test('allows default exclusion patterns to be overriden', function (t) { | |||
There was a problem hiding this comment.
Once we land the API refactor, I would like to move all the logic of building include/exclude patterns from the config into lib/ava-files, and rewrite many of these tests so they are just:
t.true(avaFiles.shouldWatch('some/path'));There was a problem hiding this comment.
Yea would be good to centralize the pattern matching logic.
|
Trying to understand what we are fixing here: Seems like a few things:
|
|
@jamestalmage yup, perfect score! |
|
It just seems we have a whole lot of code manipulating glob patterns. Ignoring for a moment our intent to refactor much of that to The code looks good, just wondering if we need to rethink some of the design decisions that led us here. |
|
@jamestalmage different use cases I think. Chokidar needs to watch tests and sources, and exclude particular directories. But we also want it to not exclude certain directories. Then we need to see if a changed file was a source or a test which again has different logic… Let me know where it isn't clear and I can add some comments. |
|
Any further feedback aside from refactoring concerns? |
| You can configure patterns for the source files using the [`--source` CLI flag] or in the `ava` section of your `package.json` file. Note that if you specify a negative pattern the directories from [`ignore-by-default`] will no longer be ignored, so you may want to repeat these in your config. | ||
| You can configure patterns for the source files using the [`--source` CLI flag] or in the `ava` section of your `package.json` file. | ||
|
|
||
| You can specify patterns to match files in the folders that would otherwise be ignored, e.g. use `node_modules/some-dependency/*.js` to specify all `.js` files in `node_modules/some-dependency` as a source, even though normally all files in `node_modules` are ignored. Note that you need to specify an exact directory; `{.bower_components,node_modules}/**/*.js` won't work. |
There was a problem hiding this comment.
.bower_components => bower_components
|
LGTM when the nitpick inline feedback is resolved. |
Make sure the path starts with ../ (after platform-specific slashes have been corrected). Clarify existing test and add a case where the source starts with two dots but is still in the current working directory.
Fixes #614. Allow negated source patterns to be specified without unsetting the default negation patterns. Allow source patterns to override the default negation patterns if they start with one of the ignored directories.
novemberborn
force-pushed
the
better-negated-source-patterns
branch
from
4ff70bd to
5c65590
Compare
* Suggest `watch:test` as the npm script * Document how to always enable watch mode using the ava section in package.json * Recommend source patterns are configured through the ava section in package.json * Suggest using the verbose reporter when debugging
novemberborn
force-pushed
the
better-negated-source-patterns
branch
from
5c65590 to
919e5cf
Compare
|
Addressed feedback. Made some other changes to the docs, see the last commit. |
?
|
🎉 Looks good @novemberborn |
Not caught earlier probably because #730 wasn't rebased.
Bugfix where
..file.jssources were treated as being outside the current working directory. But the main change of this PR is that negated source patterns are now handled much better 💥Fixes #614. Allow negated source patterns to be specified without unsetting the default negation patterns.
Allow source patterns to override the default negation patterns if they start with one of the ignored directories.