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

add --force-exclude argument #1032

Merged
merged 15 commits into from
May 8, 2020
Merged

add --force-exclude argument #1032

merged 15 commits into from
May 8, 2020

Conversation

itajaja
Copy link
Contributor

@itajaja itajaja commented Sep 24, 2019

fixes #438

I Have not added or amended tests yet, but if we agree on the approach, I can work on them

@itajaja
Copy link
Contributor Author

itajaja commented Oct 17, 2019

ping, I think this would help a lot of people :)

@itajaja itajaja changed the title honor exclude when specifying file path add --force-exclude argument Mar 3, 2020
@RCM7
Copy link

RCM7 commented Apr 17, 2020

+1 this would definitely help! @ambv Any chance you will find the time to take a look?
Is there anything we can help with?

@yukw777
Copy link
Contributor

yukw777 commented Apr 27, 2020

+1

@itajaja do you need any help with the lint failure?

@itajaja
Copy link
Contributor Author

itajaja commented Apr 27, 2020

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 🤷

@jimmy927
Copy link

I can't understand why this is still pending, without this i basically cant make black work together with pycharm.

@yukw777
Copy link
Contributor

yukw777 commented Apr 28, 2020

@itajaja here's a PR that fixes the lint issue: itajaja#1 Hopefully this will move things forward.

@itajaja
Copy link
Contributor Author

itajaja commented Apr 29, 2020

@ambv any chance you can have a look at this?

#438 (comment) I am holding you accountable! :P

@yukw777
Copy link
Contributor

yukw777 commented May 4, 2020

Ping! @ambv

@yukw777
Copy link
Contributor

yukw777 commented May 4, 2020

I'm not sure why the travis ci job is failing.. coverage run tests/test_black.py passes on this branch on my laptop.

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a 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}")
Copy link
Collaborator

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,
Copy link
Collaborator

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.

@JelleZijlstra JelleZijlstra self-assigned this May 8, 2020
@JelleZijlstra
Copy link
Collaborator

Working on making my own requested changes now

@jimmy927
Copy link

jimmy927 commented May 8, 2020

I can do QA on this one if needed in a more complex devel-environment no ensure desired workflow can be achieved.

@cooperlees
Copy link
Collaborator

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!

@JelleZijlstra
Copy link
Collaborator

@jimmy927 that'd be great! Please let me know if you notice anything amiss.

@jimmy927
Copy link

jimmy927 commented May 8, 2020

@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 ?
Or are there any build artefacts that i need ?

@JelleZijlstra
Copy link
Collaborator

@jimmy927 yes, you can check out the repo and run black.py, although you may have to install dependencies (maybe with pip install -e . in the Black repo).

@JelleZijlstra
Copy link
Collaborator

OK CI is finally green. I ended up making a few more changes:

  • Split off a get_sources() function because flake8 thought main got too complex
  • Make gitignore matching look at the relative path of the file, so we don't get bitten by the fact that Travis runs CI in a directory called build.

@JelleZijlstra JelleZijlstra merged commit 89c87d2 into psf:master May 8, 2020
@itajaja
Copy link
Contributor Author

itajaja commented May 8, 2020

nice!

@itajaja itajaja deleted the fix-438 branch May 8, 2020 14:53
@berkowitzi
Copy link

@JelleZijlstra are there plans to cut a release with this improvement?

@jimmy927
Copy link

jimmy927 commented Jul 6, 2020

@JelleZijlstra are there plans to cut a release with this improvement?

you can do the following in the meantime:

pip install git+git://github.com/psf/black.git

then find your env and run the bin, it should look like this:

(black) ➜  black ./bin/black --version
black, version 19.10b1.dev126+g11f130b

@jimmy927
Copy link

jimmy927 commented Jul 6, 2020

@jimmy927 yes, you can check out the repo and run black.py, although you may have to install dependencies (maybe with pip install -e . in the Black repo).

Works great!
Tested with pycharm now!

see config:

Screenshot 2020-07-06 at 22 00 23

@Saksow
Copy link

Saksow commented Jan 5, 2021

We would be happy to use force-exclude in pyproject.toml using a black release. We are using black with pre-commit and want to consolidate all configs inside pyproject.toml. How do I track this to be released?

@ichard26
Copy link
Collaborator

ichard26 commented Jan 9, 2021

@Saksow if I'm not misremembering, --force-exclude is available in Black version 20.8b1 :)

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.

Black does not honor exclude regex when files explicitly listed on the command line
9 participants