jendrikseipp / vulture

Find dead Python code
MIT License
3.38k stars 148 forks source link

Add the ability to use the full path for exclude folder/file #310

Open mirecl opened 1 year ago

mirecl commented 1 year ago

Description

AS-IS:

vulture . --exclude=scripts,app.py  # run command

Currently, there’s no way to specify concrete path in exclude section, this is due to Vulture will always add * to both start and end of the string: https://github.com/jendrikseipp/vulture/blob/e2e84d0a63201742ccee4bfcbe7ab065ed100f2c/vulture/core.py#L263-L266

TO-BE:

vulture . --exclude=./scripts,./app.py  # run command

Now, prefixing exclude value with ./ will resolve to full specific path:

['path/to/project/scripts/*', '/path/to/project/app.py']

Checklist:

codecov-commenter commented 1 year ago

Codecov Report

Merging #310 (068185c) into main (e2e84d0) will decrease coverage by 0.59%. The diff coverage is 33.33%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #310      +/-   ##
==========================================
- Coverage   98.94%   98.36%   -0.59%     
==========================================
  Files          21       21              
  Lines         665      671       +6     
==========================================
+ Hits          658      660       +2     
- Misses          7       11       +4     
Impacted Files Coverage Δ
vulture/core.py 97.40% <33.33%> (-1.13%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

mirecl commented 1 year ago

@jendrikseipp, what do you think? will it be useful for the project?

jendrikseipp commented 1 year ago

Thanks for the PR! It'll take me a while until I can review it properly. Two quick notes:

If you can rework the PR in these directions, that'll help getting it merged. If you already check both of these, all the better :-)

mirecl commented 1 year ago

@jendrikseipp, since this is a new feature - backwards compatibility should remain in full for option exclude files.

Option exclude files with ./ must not conflict with flake8 and black. But the current option exclude files approach definitely contradict flake8 and black: https://github.com/jendrikseipp/vulture/blob/e2e84d0a63201742ccee4bfcbe7ab065ed100f2c/vulture/core.py#L263-L266

mirecl commented 1 year ago

@jendrikseipp, pls tell us how the progress on this PR is going:)

mirecl commented 1 year ago

@jendrikseipp, what is the status of PR?

jendrikseipp commented 1 year ago

@mirecl Are you still planning to work on this?