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

Support custom 'unique' filtering through option #91

Merged
merged 3 commits into from
Feb 25, 2017

Conversation

aashna956
Copy link
Contributor

Github Issue: #76

Support custom unique filtering through option, or default to 'path' if custom value not provided.

@yocontra
Copy link
Member

That's a nice win for API flexibility, lgtm!

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Some comments inline to change.

This also needs another test for using a function as the unique (or uniqueBy) option.

README.md Outdated
@@ -90,6 +90,14 @@ Type: `Boolean`

Default: `false`

##### `options.unique`
Copy link
Member

Choose a reason for hiding this comment

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

After sleeping on it, I think this option should probably be named uniqueBy to properly describe what it is doing. @contra thoughts?

README.md Outdated
@@ -90,6 +90,14 @@ Type: `Boolean`

Default: `false`

##### `options.unique`

Filters stream to remove duplicates based on `unique`.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe something like "based on the string property name or the result of function". It should also be mentioned that when using a function, it receives the streamed data to compare against (we refer to this data above as "objects that contain cwd, base and path properties")

index.js Outdated
@@ -20,6 +20,8 @@ function globStream(globs, opt) {
ourOpt.dot = typeof ourOpt.dot === 'boolean' ? ourOpt.dot : false;
ourOpt.silent = typeof ourOpt.silent === 'boolean' ? ourOpt.silent : true;
ourOpt.cwdbase = typeof ourOpt.cwdbase === 'boolean' ? ourOpt.cwdbase : false;
ourOpt.unique = typeof ourOpt.unique == 'string' ||
Copy link
Member

Choose a reason for hiding this comment

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

use ===, just a styled preference

Copy link
Member

Choose a reason for hiding this comment

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

Needs to be changed to uniqueBy if we decide on that name.

index.js Outdated
@@ -66,7 +68,7 @@ function globStream(globs, opt) {

// Then just pipe them to a single unique stream and return it
var aggregate = new Combine(streams);
var uniqueStream = unique('path');
var uniqueStream = unique(ourOpt.unique);
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be changed to uniqueBy if we decide on that name.

@phated phated merged commit 471a95f into gulpjs:master Feb 25, 2017
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.

None yet

3 participants