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

-I supports shell globs #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mgarciaisaia
Copy link

Fixes #1

Copy link
Owner

@foca foca left a comment

Choose a reason for hiding this comment

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

Thanks so much for taking the time and effort to do this! ❤️

Could you add tests for this? If not I'll get around to it probably over the weekend, but would like to merge with tests added.

@mgarciaisaia
Copy link
Author

I'm really not that sure I could write a simple test around this - if you can come up with them, that'd be great 👍

@foca
Copy link
Owner

foca commented Oct 31, 2016

I added some tests and documentation on top of your changes on this branch. Feel free to pull them into the PR and I'll merge it 😄

@foca
Copy link
Owner

foca commented Oct 31, 2016

I also added a tiny performance improvement on top of your change, to avoid adding files to the search path, just directories. This way we pay a tiny penalty (checking if it's a directory) once when adding them to the path, but avoid iterating through a list with potentially lots of files each time you include another one.

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.

2 participants