51

I'm trying to use pre-commit to manage Black as a Git pre-commit hook, but I must be doing it wrong.

In my pre-commit config file I have:

-   repo: https://github.com/psf/black
    rev: 19.3b0
    hooks:
    -   id: black

What I'm expecting to happen is for Black to just modify the staged file, and for the commit to succeed. Because the whole point of Black is that it auto-enforces Python code style rules, no questions asked.

What actually happens when I stage a (non-black-compliant) file and try to commit: Black goes ahead and modifies the file to make it compliant, as intended... But the problem is that it returns a "fail". So the commit fails. And then I have to unstage the file, then restage it before committing again... and only then does the commit succeed.

This is a huge annoyance and can't possibly be the intended workflow?

What am I doing wrong?

Jean-François Corbett
  • 37,420
  • 30
  • 139
  • 188
  • 3
    I also think that for a tool like black, which is intended specifically for modifying files without the need of human supervision, should commit the files even if it has changed them. I've opened an issue on the `black` repository: https://github.com/psf/black/issues/1857 – Anakhand Dec 12 '20 at 11:17
  • 1
    A pre-commit hook shouldn't modify staged files without the opportunity for inspection as a matter of principle. It sounds like what you want would be better achieved by aliasing/overriding `git add` to run black before staging; leaving the pre-commit hook as a validity check, not a formatting tool. – David258 Aug 31 '23 at 15:48

5 Answers5

48

(author of pre-commit here)

the framework intentionally does not provide a way to auto-commit modifications. Here's a few issues which have asked for such:

A comment from one of those issues:

pre-commit itself will never touch the staging area. These are good ways to silently break commits. In my mind this is one of the worst things that [other frameworks do and suggest] -- hooks are very frequently not perfect and magically changing what's being committed should not be taken lightly.

That said, if you would like to foot gun, your hook can call git add -u and pre-commit won't know any better :) a sketch of that (untested, discouraged)

  - id: yapf
    entry: bash -c 'yapf "$@"; git add -u' --

(note: using bash will potentially reduce portability)

Another comment notes

Fortunately, git add -u && !! is pretty easy to run if you're ok firing from the hip :)

anthony sottile
  • 61,815
  • 15
  • 148
  • 207
  • 15
    Sounds reasonable enough. I'm left struggling to understand what the `black` pre-commit hook is supposed to do / how it's supposed to be used. Because the whole point of black is that it modifies files, no questions asked. I guess that's a questions for the authors of that hook. – Jean-François Corbett Oct 22 '19 at 07:38
  • where do you add this entry? both black and pre-commit-hook say: `yapf` is not present in repository . Typo? Perhaps it is introduced in a newer version? Often `pre-commit autoupdate` fixes this. – michel.iamit Mar 25 '20 at 22:41
  • @michel.iamit `yapf` is just an example for yapf (copied from the ticket), you want to make sure you're configuring the hook you want to configure (`id: black` for instance) – anthony sottile Mar 25 '20 at 22:56
  • 13
    I don't understand how this answer is accepted - I'm left confused just like @Jean-FrançoisCorbett. Whats the use of a pre-commit hook that formats code if it doesn't modify the file in staging? – mchristos Dec 04 '21 at 13:56
  • 2
    The purpose of the pre-commit hook is to ensure that the formatting is done correctly. It's on the individual developers to ensure that they do that using the same code formatters with same settings on their editors. – Afzal S.H. Aug 31 '22 at 11:29
  • https://github.com/psf/black/issues/285 – Linus Fernandes Feb 23 '23 at 10:54
12

One of my developers came with a good tip, in case you have a fail by commit due to black (for example due to the single/double quotes) that's resolved with the pre-commit-hook (like with double-quote-string-fixer). You get in kind of 'nobodies land git situation'. There is a changed file in the staged files, but cannot be committed by the pre-commit-hook, git status won't see any changes, but commit fails (a really black hole in my opinion). You only get a fail on commit, but nothing can be done (excecpt a reset head on this file). Once you are in this situation and run: use commit -m 'Resolving pre-commit-hook changes' --no-verify ..... tada! It's resolved.

Aaron Meese
  • 1,670
  • 3
  • 22
  • 32
michel.iamit
  • 5,788
  • 9
  • 55
  • 74
  • 2
    In this situation, your staged files still have the wrong single-quotes, as black only modifies the *working* files. In order for `git commit` to succeed, **first stage the changes** black made with `git add` otherwise you'll commit the wrong single-quotes. – Carl Walsh Dec 01 '22 at 19:47
  • @CarlWalsh Your comment is a little ambiguous. `"In order for git commit to succeed"` - `git commit` will succeed because the answer suggests using `--no-verify` flag. Maybe would be better to rephrase it to something like `"In order to avoid committing unformatted code"`. – Jakub Kukul Jan 07 '23 at 19:36
6

Looking at black's README, you probably want to use the --check option, which simply exits successfully or unsuccessfully depending on whether the file meets standards. That will cause the commit to fail without modifying the file.

bk2204
  • 64,793
  • 6
  • 84
  • 100
  • 19
    I see what you mean, but that's the opposite of what I'm trying to do. I'd like black to just modify the file, and for the commit to succeed. Now clarified in the question. – Jean-François Corbett Oct 16 '19 at 07:25
1

I'm the same boat as you. From what I've been researching, commits can't be amended by pre-commit hooks.

As far as I can figure out, the best next thing is what bk2204 outlined. We ask black to prevent any commits that include python files that haven't been formatted properly by black. It still makes sure that any commits are formatted, but it is indeed annoying that it won't just automatically format the files for us.

It makes sense. Any changes in a commit have to be staged. If we could do that from the git hook, then our problem would be solved. You got halfway there by modifying the file directly from the git hook. The next half would be staging all the changes of the modified files. But evidently... "You cannot amend a commit in a pre commit hook" which means no staging. https://stackoverflow.com/a/14641656/6032076

I would have commented on bk2204's answer, but I don't have 50 rep yet.

Scratch all that, this answer (https://stackoverflow.com/a/16832764/6032076) claims that commits can be changed in a pre commit hook. In that case files are being added, so I bet in our case files can be restaged/amended.

DavidTriphon
  • 159
  • 1
  • 7
  • Interesting, basically "it's a bad idea to change what's committed during pre-commit hook." It seems like a better solution to the problem is to have a wrapper script that runs black before you run git >.> – Carl Walsh Dec 01 '22 at 19:51
1

In my experience there is no need to unstage any files, either just git add the files that black, or other pre-commit hooks, as modified again which will overwrite them in the stage area and commit with the same message as before unless the error was with a commit message check.

If your commit includes all of your changed files then simply changing the commit command from git commit -m "Your commit message" to git commit -am "Your commit message" will do the job nicely.

Steve Barnes
  • 27,618
  • 6
  • 63
  • 73