Added --include and --exclude cli options #281

autophagy · 2018-05-30T21:08:56Z

These 2 options allow you to pass in regular expressions that determine whether files/directories are included or excluded in the recursive search.

This should address #270. I put the change log entry for this under 18.6b0 since I don't think it has been released yet - but let me know if this was incorrect! ✨

autophagy · 2018-05-30T21:19:38Z

Also, I genuinely do not know how I would check the types of exclude and include here https://github.com/ambv/black/pull/281/files#diff-a75051ebf8e3452bf3d590c186944252R2791 since I am passing in compiled regular expressions which have a type of _sre.SRE_Pattern, but apparently AttributeError: module '_sre' has no attribute 'SRE_Pattern' 😅

JelleZijlstra · 2018-05-30T21:21:46Z

Try typing.Pattern[str].

coveralls · 2018-05-30T21:22:54Z

Pull Request Test Coverage Report for Build 447

53 of 53 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.2%) to 96.203%

Totals
Change from base Build 437:	0.2%
Covered Lines:	2280
Relevant Lines:	2370

💛 - Coveralls

autophagy · 2018-05-30T21:29:52Z

@JelleZijlstra Ah! Thank you a bunch.

ambv

This is almost there! See review comments, and more importantly, add tests for this.

ambv · 2018-05-31T17:54:49Z

black.py

@@ -46,6 +46,10 @@

 __version__ = "18.5b1"
 DEFAULT_LINE_LENGTH = 88
+DEFAULT_EXCLUDES = (
+    "build\/|buck-out\/|dist\/|_build\/|.git\/|.hg\/|.mypy_cache\/|.tox\/|.venv\/"


Use r-strings for patterns.

Escaping slashes is not necessary.

But escaping periods is.

Pro-tip: run Python with warnings, this will make Python tell you things like:

black.py:50: DeprecationWarning: invalid escape sequence \/ "build\/|buck-out\/|dist\/|_build\/|.git\/|.hg\/|.mypy_cache\/|.tox\/|.venv\/" black.py:52: DeprecationWarning: invalid escape sequence \. DEFAULT_INCLUDES = "\.pyi?$"

ambv · 2018-05-31T17:55:00Z

black.py

+DEFAULT_EXCLUDES = (
+    "build\/|buck-out\/|dist\/|_build\/|.git\/|.hg\/|.mypy_cache\/|.tox\/|.venv\/"
+)
+DEFAULT_INCLUDES = "\.pyi?$"


Ditto about r-strings.

ambv · 2018-05-31T18:15:31Z

black.py

    """
    for child in path.iterdir():
        if child.is_dir():
-            if child.name in BLACKLISTED_DIRECTORIES:
+            if not exclude.search(str(child) + "/"):


$ mkdir -p a/b/c/d $ touch a/b/c/d/file.py $ cd a $ black . No paths given. Nothing to do 😴

(This works on master.)

These 2 options allow you to pass in regular expressions that determine whether files/directories are included or excluded in the recursive file search.

ambv · 2018-05-31T20:30:14Z

black.py

+                raise SyntaxError(
+                    "Invalid regular expression for exclude given: {}".format(exclude)
+                )
+            sources.extend(gen_python_files_in_dir(p, include_regex, exclude_regex))


Sorry, just noticed this now.

Why are we compiling the regex again for every entry in src?

Could we use a better exception type than Exception?

Use an f-string for the exception message?

Use the !r modifier so that the error is less likely to be confusing.

We actually don't want to raise an exception but rather just use err() followed by ctx.exit(2) (1 is already used by --check and 123 is internal error). It's a tool, we don't want to trouble people with unnecessary tracebacks.

ambv · 2018-05-31T20:39:37Z

black.py

@@ -46,6 +46,10 @@

 __version__ = "18.5b1"
 DEFAULT_LINE_LENGTH = 88
+DEFAULT_EXCLUDES = (
+    r"build/|buck-out/|dist/|_build/|\.git/|\.hg/|\.mypy_cache/|\.tox/|\.venv/"


There's a problem to solve here. In the current model, "not_a_build/" is also matching. It would be more robust to prepend all those directory names with slashes. However, if the user specified a relative directory, we might not have a leading slash always. If they said black ., it's fine. If they said black *, it's not.

So up to you how we solve this but it will have to be addressed before we land this.

Good point, I think prepending the directories with slashes would be the best route. I'll play around with it and see if I can come up with an elegant way of doing this.

psf#281 (review)

autophagy · 2018-05-31T22:50:51Z

@ambv Added some logic to allow for prepended directories in the filter, and confirmed it works well with both * and .. I'm building up the path to search from using the children, rather than the path, as it's a little easier when the path itself is something like .

Do you think it would be best to mention in the help/documentation that the include/exclude filters only support POSIX formats? Windows' use of \ instead of / was causing a few problems, and supporting /|\ in the defaults seems like it could be a little fragile (for example, if someone on a system with POSIX paths decides to call a directory /build\me/, the exclude filter would match on the /build\)

I'd also like to add a couple more test cases too.

psf#281 (review)

ambv · 2018-06-01T00:38:05Z

Makes sense to just mention: "use forward-slashes on Windows, too."

ambv · 2018-06-01T00:54:48Z

Awesome! ✨ 🍰 ✨

autophagy force-pushed the include-exclude branch from 384ef08 to 1b28854 Compare May 30, 2018 21:09

autophagy force-pushed the include-exclude branch from 92b08ea to 35062ae Compare May 30, 2018 21:54

ambv requested changes May 31, 2018

View reviewed changes

Added --include and --exclude cli options

b185331

These 2 options allow you to pass in regular expressions that determine whether files/directories are included or excluded in the recursive file search.

autophagy force-pushed the include-exclude branch from d217dca to b185331 Compare May 31, 2018 20:10

ambv reviewed May 31, 2018

View reviewed changes

Addressing review comments

9fa5d9c

psf#281 (review)

autophagy added a commit to autophagy/black that referenced this pull request May 31, 2018

Addressing review comments

7257646

psf#281 (review)

autophagy added a commit to autophagy/black that referenced this pull request May 31, 2018

Addressing review comments

ab209ba

psf#281 (review)

autophagy added a commit to autophagy/black that referenced this pull request May 31, 2018

fixup! Addressing review comments psf#281 (review)

6fe431f

Addressing review comments

88c1c50

psf#281 (review)

autophagy force-pushed the include-exclude branch from b6f3e1d to 1021da5 Compare June 1, 2018 00:29

Add tests for invalid and empty include/excludes

57e86ab

autophagy force-pushed the include-exclude branch from 1021da5 to 57e86ab Compare June 1, 2018 00:37

Update help for windows systems

7d008a4

ambv approved these changes Jun 1, 2018

View reviewed changes

ambv merged commit 51756a4 into psf:master Jun 1, 2018

autophagy deleted the include-exclude branch June 1, 2018 00:55

jgirardet mentioned this pull request Jun 1, 2018

Can I configure black to ignore specific directories, such as venv #286

Closed

charliermarsh mentioned this pull request Apr 12, 2023

Allow users to extend the set of included files via include astral-sh/ruff#3914

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added --include and --exclude cli options #281

Added --include and --exclude cli options #281

autophagy commented May 30, 2018 •

edited

Loading

autophagy commented May 30, 2018

JelleZijlstra commented May 30, 2018

coveralls commented May 30, 2018 •

edited

Loading

autophagy commented May 30, 2018

ambv left a comment

ambv May 31, 2018

ambv May 31, 2018

ambv May 31, 2018

ambv May 31, 2018

ambv May 31, 2018

autophagy May 31, 2018

autophagy commented May 31, 2018 •

edited

Loading

ambv commented Jun 1, 2018

ambv commented Jun 1, 2018

Added --include and --exclude cli options #281

Added --include and --exclude cli options #281

Conversation

autophagy commented May 30, 2018 • edited Loading

autophagy commented May 30, 2018

JelleZijlstra commented May 30, 2018

coveralls commented May 30, 2018 • edited Loading

Pull Request Test Coverage Report for Build 447

💛 - Coveralls

autophagy commented May 30, 2018

ambv left a comment

Choose a reason for hiding this comment

ambv May 31, 2018

Choose a reason for hiding this comment

ambv May 31, 2018

Choose a reason for hiding this comment

ambv May 31, 2018

Choose a reason for hiding this comment

ambv May 31, 2018

Choose a reason for hiding this comment

ambv May 31, 2018

Choose a reason for hiding this comment

autophagy May 31, 2018

Choose a reason for hiding this comment

autophagy commented May 31, 2018 • edited Loading

ambv commented Jun 1, 2018

ambv commented Jun 1, 2018

autophagy commented May 30, 2018 •

edited

Loading

coveralls commented May 30, 2018 •

edited

Loading

autophagy commented May 31, 2018 •

edited

Loading