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

Initial fix for issue 399 #400

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

Conversation

thomasdstewart
Copy link

First look at #399, sorry I've not written any test's, but it still seems to pass al existing ones.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 97.344% when pulling 7a3c6d6 on thomasdstewart:fix-issue-399 into 33ce0fa on adrienverge:master.

@hellt
Copy link

hellt commented Jul 19, 2022

Hi @thomasdstewart
do you have any estimates as to when this is going to be merged? I have a need to exclude broken symlinks as well

@thomasdstewart
Copy link
Author

Hi @hellt,
To be honest, it's not really on my radar. My fix did work for my case, but it subtly broke some other functionality, as showed by the failed tests. I didn't have time to further investigate why.
Tom

@@ -201,6 +201,8 @@ def run(argv=None):

for file in find_files_recursively(args.files, conf):
filepath = file[2:] if file.startswith('./') else file
if(conf.is_file_ignored(filepath) or None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(conf.is_file_ignored(filepath) or None):
if conf.is_file_ignored(filepath):

I don't understand how or None would help.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps change the commit text to:

cli: match symlinks to ignore early on, before following them

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.

None yet

4 participants