-
Notifications
You must be signed in to change notification settings - Fork 319
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 support for the --include
flag
#353
base: master
Are you sure you want to change the base?
Conversation
This might be too much to ask: could you please include some tests to show this PR works as intended? @danburzo |
No worries, I'll be glad to include some tests when I find a window of time! In regards to your proposal to mirror
I'll rethink the names of the CLI options to see if there's something better available. |
I agree. The current One difference I noticed is the glob-pattern and RegExp. I can't decide which one is better:
Maybe we can first keep everything the same to finish this PR? We welcome all to discuss about the new |
If I understand correctly (not a TS user), In contrast, |
|
||
function regExpFilter(id) { | ||
return regExpList.findIndex((regexp) => regexp.test(id)) < 0; | ||
return (!includeList || includeList.findIndex((re) => re.test(id)) >= 0) && |
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.
This logic means, only filenames declared in --include
are included, other files, even those not --exclude
-d, won't be checked?
@danburzo, sorry 😅 I was too stupid. I just realized: in What I wanted to say at first was to give priorities to |
Fixes #219. This is a redo of a previous PR:
--include
flag #220