-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add findUpMultiple()
method
#56
Conversation
If |
index.d.ts
Outdated
|
||
console.log(await findUpMultiple(async directory => { | ||
const hasUnicorns = await pathExists(path.join(directory, 'unicorn.png')); | ||
return hasUnicorns && directory; |
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 tab-indentation.
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.
Thanks. Fixed:
70242f5
Thanks. Fixed: |
Done in: I introduced What do you think? |
index.d.ts
Outdated
Find files or directories by walking up parent directories. | ||
|
||
@param name - The name of the file or directory to find. Can be multiple. | ||
@returns All paths found (by respecting the order of `name`s) or `undefined` if none could be found. |
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.
undefined
is outdated.
index.js
Outdated
|
||
export async function findUp(name, options = {}) { | ||
const matches = await findUpMultiple(name, {...options, limit: 1}); | ||
|
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.
readme.md
Outdated
### findUpMultiple(name, options?) | ||
### findUpMultiple(matcher, options?) | ||
|
||
Returns a `Promise` for either the array of paths or an empty array if it couldn't be found. |
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.
I don't think "it" is correct here. This method is for finding multiple things. The index.d.ts description is correct though.
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.
Thanks! Updated in:
6ee09a3
readme.md
Outdated
### findUpMultiple(name, options?) | ||
### findUpMultiple(matcher, options?) | ||
|
||
Returns a `Promise` for either the array of paths or an empty array if it couldn't be found. |
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.
Returns a `Promise` for either the array of paths or an empty array if it couldn't be found. | |
Returns a `Promise` for either an array of paths or an empty array if it couldn't be found. |
Thanks ;) |
Fixes #27.
I'm a bit concerned about the amount of duplication in code: looks like
findUp
could be a decorated version offindUpMultiple
. On the other hand, duplication is cheaper than wrong abstraction. What do you think?P. S. PR looks huge. Let me know if that's a problem, I'll split it in multiple smaller PRs.
IssueHunt Summary
Referenced issues
This pull request has been submitted to:
findUpMultiple()
method