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

Doesn't honor extend-exclude for nested file #3754

Closed
asarkar opened this issue Jun 28, 2023 · 6 comments · Fixed by #3764
Closed

Doesn't honor extend-exclude for nested file #3754

asarkar opened this issue Jun 28, 2023 · 6 comments · Fixed by #3764
Labels
T: bug Something isn't working

Comments

@asarkar
Copy link

asarkar commented Jun 28, 2023

Describe the bug

Given the following configuration in pyproject.toml

[tool.black]
line-length = 120
extend-exclude = [
    '^(?:([^\/]+\/)+(?:test_.*\.py|.*_test\.py))$',
    '.github',
    '.mypy_cache',
    '.pytest_cache',
    'venv'
]

Black fails with the error:

Error: Invalid value for '--extend-exclude': Not a valid regular expression: unbalanced parenthesis at position 17

But this is a valid regex that's supposed to match any files starting or ending in test or files in a directory named test. For example, the following path matches the test directory:
myapp/folder/test/test_something.py

To Reproduce

I guess use the regex in a unit test that parses it?

Expected behavior

It should work. It would be even better if Black could use the extend-exclude list from flake8 or mypy, so that I don't have repeat it. The documentation only shows the max-line-length property.

Environment

  • Black's version: 23.3.0
  • OS: macOS Ventura 13.4.1, M1 chip
  • Python version: (CPython) 3.11.3
@asarkar asarkar added the T: bug Something isn't working label Jun 28, 2023
@asarkar asarkar changed the title Fails to parse extend-exclude regex Fails to parse valid extend-exclude regex Jun 28, 2023
@hauntsaninja
Copy link
Collaborator

The extend-exclude config is meant to be a str, not a list. The error message here could obviously be much better.

@asarkar
Copy link
Author

asarkar commented Jun 28, 2023

@hauntsaninja Thanks, let me try that. Do you've any comment on the other thing I mentioned above?

It would be even better if Black could use the extend-exclude list from flake8 or mypy

@asarkar
Copy link
Author

asarkar commented Jun 28, 2023

The extend-exclude config is meant to be a str

extend-exclude = """
/
^(?:(?:[^/]+/)+(?:test_.*\\.py|.*_test\\.py))$
| .github
/
"""

Nah, this doesn't work too. No error, but the file test_something.py gets formatted. Perhaps related to #3617?

@asarkar asarkar changed the title Fails to parse valid extend-exclude regex Doesn't honor extend-exclude for nested file Jun 28, 2023
@asarkar
Copy link
Author

asarkar commented Jul 5, 2023

@hauntsaninja How does improving the error message fix the issue where the exclude doesn’t work at all?

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jul 5, 2023

You might want to be a little nicer to volunteers. Your regex is probably broken:

import re
exclude = """
/
^(?:(?:[^/]+/)+(?:test_.*\\.py|.*_test\\.py))$
| .github
/
"""
assert re.search(exclude, "something_test.py", flags=re.VERBOSE) is None
assert re.search(exclude, "/something_test.py", flags=re.VERBOSE) is None

Works fine for me:

λ cat pyproject.toml
[tool.black]
extend-exclude=".*_test.py"

λ cat something.py 
x = (
        'asdf'
        )

λ cat something_test.py 
x = (
        'asdf'
        )

λ black --check .
would reformat /Users/shantanu/dev/black/proj/something.py

Oh no! 💥 💔 💥
1 file would be reformatted.

@JelleZijlstra
Copy link
Collaborator

And on this one:

It would be even better if Black could use the extend-exclude list from flake8 or mypy

I don't think that's a good idea. People might exclude files from different tools for different reasons, and it is likely to be confusing if configuration for one tool magically affects another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants