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.

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

Adds Exclusion Support to Gaze. Fixes GH-7 #21

Closed
wants to merge 2 commits into from
Closed

Adds Exclusion Support to Gaze. Fixes GH-7 #21

wants to merge 2 commits into from

Conversation

itsjamie
Copy link

This is the same request as #20, just squashed into one commit. To make it easier to look at/maintain.

If both this and gruntjs/grunt-contrib-watch#43 are applied, gruntjs/grunt-contrib-watch#20 is fixed for me.

Add test case for exclusions in Gaze.

Testcase..

Fix test cases on this branch.

Code Review mchoy. Ran local tests, 42 passes. Yay. Tested locally as well using my project, and it worked the way I expected it too. Yay!

Minor fixes.
@itsjamie
Copy link
Author

@shama should be good for review now, I went through and fixed all the things I saw after I took a second look today.

I've got a version run through JSHint available if you want it. you can see it at...
https://github.com/jamie-stackhouse/gaze/commit/13bfeeb81b73bc3f24590999e025c982b8a883c7
^ also includes some revised edits to the tests that was closed. However, it's gaze.js is definitely better, I'll update this branch with it.

if (this._patterns[i].indexOf('!') === 0) {
//First time a negation is hit, we can hop out of the forEach.
matched = false;
continue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Just reading this to try to help out in reviewing.. If you don't mind.)
Did you mean break here?

Alternatively, you can use the Array.some() method.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hint on array.some, didn't know about it.

Yea, it should be a break. Oops :o

@shama
Copy link
Owner

shama commented Feb 12, 2013

I really appreciate the work you put into this @jamie-stackhouse. Although I think we should upstream this functionality. I found this lib fileset which does exactly what we're trying to accomplish here. Just waiting for a merged PR to get published and will integrate. I have the integration started in the patterns branch.

@chrisirhc
Copy link
Contributor

Thanks for the update, @shama, and I'll be integrating it to the incremental building plugin I'm working on!

@cowboy
Copy link
Collaborator

cowboy commented Feb 12, 2013

FWIW, I'm going to be extrapolating all of grunt.file.expand and grunt.file.match functionality into a separate lib once grunt 0.4.0 has been released.

@itsjamie
Copy link
Author

Hey, seeing as how the issue related to this pull request is going to be solved, I'mma close it.

Thanks!

@itsjamie itsjamie closed this Feb 14, 2013
@shama
Copy link
Owner

shama commented Feb 14, 2013

Thanks again for pinpointing this issue Jamie! Fixed on 5578985.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants