-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Conversation
That's a nice win for API flexibility, lgtm! |
There was a problem hiding this 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` |
There was a problem hiding this comment.
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`. |
There was a problem hiding this comment.
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' || |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
Github Issue: #76
Support custom
unique
filtering through option, or default to 'path' if custom value not provided.