-
Notifications
You must be signed in to change notification settings - Fork 447
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
adding directory_filter arg to customized ignored dirs #447
Conversation
Current coverage is 90.16% (diff: 100%)@@ master #447 diff @@
==========================================
Files 24 24
Lines 3954 3954
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 3565 3565
Misses 389 389
Partials 0 0
|
@@ -128,6 +129,9 @@ def extract_from_dir(dirname=None, method_map=DEFAULT_MAPPING, | |||
positional arguments, in that order | |||
:param strip_comment_tags: a flag that if set to `True` causes all comment | |||
tags to be removed from the collected comments. | |||
:param directory_filter: a list of strings that identify the starting |
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.
To be honest, this might be more useful if it declared a list of fnmatch
compatible patterns.
Also, now that I think of it, it could maybe just accept a callable? After all there's no command-line support for this argument yet, so it needn't be a "command line compatible". The default could be default_directory_filter
, defined as
def default_directory_filter(path):
subdir = os.path.basename(path)
return not (subdir.startswith('.') or subdir.startswith('_'))
and the implementation in the walk loop simply
dirnames[:] = [dirname for dirname in dirnames if directory_filter(os.path.join(root, dirname)]
Also: no rookie mistakes made! :) |
@@ -138,7 +142,7 @@ def extract_from_dir(dirname=None, method_map=DEFAULT_MAPPING, | |||
absname = os.path.abspath(dirname) | |||
for root, dirnames, filenames in os.walk(absname): | |||
for subdir in dirnames: | |||
if subdir.startswith('.') or subdir.startswith('_'): | |||
if any(subdir.startswith(chars) for chars in directory_filter): |
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.
starstwith can also accept a tuple of strings to try .. so if subdir.startswith(directory_filter)
should suffice
Also while at it, extract_messages
command should be modified to accept the directory_filter
argument in order to be passed to extract_from_dir
Is this going to be accepted? If so, what needs to be done? Inside one of my apps there is a huge NPM's |
Per #402, adding directory_filter arg and replacing hard-coded .startswith() calls. I am unsure where to add --ignore-dirs argument.
This is my first time ever doing a pull request, so please forgive any rookie mistakes. :)