-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 --force-exclude argument #1032
Conversation
ping, I think this would help a lot of people :) |
+1 this would definitely help! @ambv Any chance you will find the time to take a look? |
+1 @itajaja do you need any help with the lint failure? |
if you want to help, you are welcome to. I am honestly tired of trying to get this merged. black team has been pretty unresponsive about this 🤷 |
I can't understand why this is still pending, without this i basically cant make black work together with pycharm. |
@ambv any chance you can have a look at this? #438 (comment) I am holding you accountable! :P |
Ping! @ambv |
I'm not sure why the travis ci job is failing.. |
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.
Sorry for the long wait here! I have a few comments.
black.py
Outdated
re_compile_maybe_verbose(force_exclude) if force_exclude else None | ||
) | ||
except re.error: | ||
err(f"Invalid regular expression for exclude given: {exclude!r}") |
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.
Should say force_exclude
, not exclude
.
black.py
Outdated
p.iterdir(), | ||
root, | ||
include_regex, | ||
exclude_regex, |
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.
I feel like this should also obey the force_exclude_regex. It will be surprising if I force-exclude a file and it gets included anyway because it's in a directory I included.
Working on making my own requested changes now |
can't reproduce these failures locally
I can do QA on this one if needed in a more complex devel-environment no ensure desired workflow can be achieved. |
Please sir. With your confidence we'd look to merge this today! |
@jimmy927 that'd be great! Please let me know if you notice anything amiss. |
Can i just check out the repo and run it as is ? |
@jimmy927 yes, you can check out the repo and run |
OK CI is finally green. I ended up making a few more changes:
|
nice! |
@JelleZijlstra are there plans to cut a release with this improvement? |
you can do the following in the meantime:
then find your env and run the bin, it should look like this:
|
Works great! see config: |
We would be happy to use |
@Saksow if I'm not misremembering, |
fixes #438
I Have not added or amended tests yet, but if we agree on the approach, I can work on them