Fix exclusions #20
Fix exclusions #20
Conversation
… well using my project, and it worked the way I expected it too. Yay!
| @@ -477,7 +539,7 @@ Gaze.prototype._initWatched = function(done) { | |||
| var relFile = path.join(relDir, file); | |||
| if (_this._isMatch(relFile)) { | |||
| // Add to watch then emit event | |||
| _this.add(relFile, function() { | |||
| _this.add(null, function() { | |||
There was a problem hiding this comment.
Changed to null here because if we are already matching a pattern there is no need to add a single file pattern to our list. This will just make later calls take longer to figure out if they match or not.
|
I'm going to be in and out of meetings the next couple of days. So give me a bit to review. Thanks! |
| @@ -8,7 +8,7 @@ module.exports = function(grunt) { | |||
| } | |||
| }, | |||
| nodeunit: { | |||
| files: ['test/**/*_test.js'], | |||
| files: ['test/**/*_test.js'] | |||
There was a problem hiding this comment.
I had an extra test in nodeunit to run my test in exclusion from the rest initially, must have got committed at some point.
|
@shama Not a problem on the whole meeting thing, work has been slow for me lately, can you tell? Sorry for the mass spam on the commit, I knew there would be places I could clean it up :) Let me know if there is anything I can do. |
|
Also just a note, this is related to #7 |
|
Sorry for making this so hard to maintain. I've gone ahead and updated your testcases so they are a little more standardized. I think it might be easier to close this pull request, and what I'll do is make this a little nicer for you. I'll make the changes to the testcases one commit, and the changes to the lib/gaze.js file one commit. And branch each seperately and submit as two seperate pull requests. |
|
No worries, it looks like you're making good progress. Just let me know when it's ready to review. Separate pull requests that each focus on one issue is a good idea. I'd just clean up the Just let me know when these are ready to review. I change my mind a lot with PRs too :) I actually started for big PRs, to push the commits and then sit on it for a little bit before opening the PR. Forcing myself to slow down gives me a chance to handle those "oh yeah" moments. |
|
oh yeah :) You also dont have to open new pull requests. Just push to the same branch and it will add the commits. Or even better do a |
|
Good to know in the future :) |
After a minor change to grunt-contrib-watch, this would allow passing exclusions through via that task, which in turn gets added events to pop on the right files, not on every file in a directory that initially was added. Test case is included in this pull request which showcases a textfile being included on a rule that was */.js. I'll remove the other pull-request that only contained the test.
I ran the local npm test and had 42 passes on Win7, and I also tested in manually using my project and I was getting added events working as expected etc. The code will need to be reviewed and cleaned up. I'm not sure if it's entirely ready to go into Gaze, but it does what it needs to now, so I figure I might as well let you review / suggest changes. Tomorrow I'll mark up the files changed on GitHub to mark the ideas behind each modification in the hope of cleaning it up some. I'll also remove the console.logs.