Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

use exact black version pre-commit uses #31

Closed
RonnyPfannschmidt opened this issue Oct 1, 2018 · 9 comments
Closed

use exact black version pre-commit uses #31

RonnyPfannschmidt opened this issue Oct 1, 2018 · 9 comments

Comments

@RonnyPfannschmidt
Copy link

people combining pre-commit and black tend to pin the black version to control transition and formatting,
as black enhances it sometimes introduces better formatting that will conflict with a projects pinned setup

in order to avoid this dissonance it would be really useful to either use black from pre-commit if available or to take the same version

@jgirardet
Copy link
Owner

I've never used precommit so I do not really understand.
Please tell what you mean with "use black from precommit."

@RonnyPfannschmidt
Copy link
Author

https://github.com/pytest-dev/pytest/blob/5e7d427df1b258b5e4c0f5e1496f0f68545ad4b1/.pre-commit-config.yaml#L4

is an example of the usage - we use it to ensure all commits are up to standard, now if the black the editor uses and the black the commit checking uses differ in version, there is the possibility of different outcomes (as black enhances)

@jgirardet
Copy link
Owner

if I get you, you'd like to run pre-commit run black command instead of black if black in the .pre-commit-config.yaml file ?
What about the config options (line-length, py36...) ? precommit will look in pyproject ? should we ignore sublack options if using precommit ?

@RonnyPfannschmidt
Copy link
Author

@jgirardet i believe the black options should always be taken from black or pre-commit if feasible, its bit of - idealls sublack has no own options and defers everything to the project/its tooling, else developer preference differences will cause havoc

@jgirardet
Copy link
Owner

jgirardet commented Oct 3, 2018

Hi @RonnyPfannschmidt
I discovered pre-commit and thought a bit about it.
Running black via hook on saved file is easy to do but really slows the process.
Black doesn't care much about speed (and does not need) but for sublack it's more a point since you don't want to be waiting in front of your screen waiting for formatting while coding.
a 300 lines file :

jimmy@x-or:~/trash/sublack/tests$ time black test_sublack.py 
reformatted test_sublack.py
All done! ✨ 🍰 ✨
1 file reformatted.

real	0m0,677s
user	0m0,661s
sys	0m0,022s
jimmy@x-or:~/trash/sublack/tests$ time pre-commit run black --files test_sublack.py 
black....................................................................Failed
hookid: black

Files were modified by this hook. Additional output:

reformatted tests/test_sublack.py
All done! ✨ 🍰 ✨
1 file reformatted.

real	0m1,132s
user	0m1,067s
sys	0m0,070s

So i'm not sure about running sublack via precommit by defaut if black's hook is present.

The other matter comes with unsaved file : since I use black - to format via stdin/stdout I can't use it with pre-commit because pre-commit does not accept additional arguments to hook in the command line(or I missed it)
so 2 ways:

  • format via a tempfile,
  • or always save file before applying black wen using pre-commit.
    These 2 ways will probably also slow the process and force more disk access.

Last thing, black_on_save would also force to save the file twice to format.

Please tell if I'm wrong or if you think to another/better implementation.

@RonnyPfannschmidt
Copy link
Author

@jgirardet that's indeed a unfortunate setup - i will open a accompanying issue for pre-commit as its definitively a useful item

@jgirardet
Copy link
Owner

Hi @RonnyPfannschmidt ,
After talking with asotille, for now I'm ok to add it but as an optional behavior.
I'll keep you in touch

@jgirardet
Copy link
Owner

It's on master. you have to use the option "black_use_precommit"

@jgirardet
Copy link
Owner

in v 2.1.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants