Read shebangs to discover python files

[florczakraf]

At work we've often found ourselves manually adding python scripts without .py extensions to black calls because they were placed in various locations in repositories that weren't strictly python projects. Such special handling is error-prone and we've come to the conclusion that since black offers python file discovery (includes/excludes) when the directory is provided, it could be extended to also check shebangs.

Thanks to @D0han for the support!

[zsol]

caveat: I haven't looked at the code yet. My main concern with this idea is the performance hit when crawling large directories of non-python code

[emdej]

On *nix systems we could check executable files only.

[florczakraf]

@zsol: I've just checked the difference on some internal repo. I don't see much overhead on the repo with more than 10.5k files where there were ~150 python files to reformat.

Without this patch: 22.3s (avg over the 3 runs)
With: 23.5s (but there were extra files to reformat ;)

Moreover, this feature has been set as optional with an appropriate warning in help.

@emdej: it could produce inconsistent results on exotic platforms like Windows.

[florczakraf]

I see one other problem with the current code -- it doesn't handle OSErrors while opening the files, for example when there's no read permission. I think that we can safely skip such files as black would not reformat them anyway.

[tyilo]

At least #!/usr/bin/python should also be included by default.
Maybe also #!/usr/bin/python3 and #!/usr/bin/python2.

[D0han]

I do not think that we should encourage usage of such explicit path with defaults.
If someone wants, regex is easily customizable with either toml or cli parameter.

[asottile]

fwiw, when run using pre-commit run --all-files the shebang'd executable files will already be passed to black (via identify)

[D0han]

pre-commit is entire different project, that black can, but not have to, be used with.

[asottile]

pre-commit is entire different project, that black can, but not have to, be used with.

sure :) was just offering some potential prior art

[hugovk]

fwiw, when run using pre-commit run --all-files the shebang'd executable files will already be passed to black (via identify)

Yeah, that caught me out, I was surprised when pre-commit run --all-files formatted more files than black .! Here is a workaround.

[florczakraf]

I've just rebased the patch on the upstream and resolved conflicts. Please let me know if you're going to consider pulling this or not.

[florczakraf]

I see that there are problems after the rebase with the new tests. I will have a look at that tomorrow.

[easbarba]

python3
#!/usr/bin/env python3

[florczakraf]

I think that we can safely extend the default regex to include ^#!/usr/bin/python as well

[ambv]

Thanks for your work on this. However, I don't think we're willing to slow down traversal for all users who are not putting Python code in non-.py files. For this use case pre-commit or something similar is a decent workaround.