[style] automate reformatting with pre-commit hooks
Author: stas00
Pull request: no
Labels: name: wontfix

# 🚀 Feature request

I was just reading how `make style` can be automated with pre-commit hooks. Noticing how often I run and even more often forget to run `make style` before committing, perhaps others are in the same boat - and therefore I thought to propose to the dev community to (mostly) automate this process. The only cons is that each dev will still have to run `pre-commit install` once after cloning the project. This is a security feature of git, so it won't run anything automatically unless you take action to enable such thing.

If I understand it correctly, if an individual dev doesn't run `pre-commit install` inside the repo, things are just as normal as they are now. That dev will just run `make style` manually. i.e. the proposed feature is optional for those who want it.

I read about it [here](https://www.mattlayman.com/blog/2018/python-code-black/), please scroll down to the section: "Black as a Git pre-commit hook". And it links to the whole detailed website: https://pre-commit.com/

Comments:
I personally wouldn't like having a pre-commit hook change all my commits without me being able to see the end result.
On my setup, I have a pre-push hook that aborts a push if make quality fails. I think if we had an install script, we could handle both options?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.


Hi! bring back this because I think in suggest pre-commit instead of `make ...`

With the pre-commit, we can see the results/modifications, like by example:

`git add .`
`git commit -m "any"` **this will run the pre-commit**
- if everything it's ok at the pre-commit pipeline, the commit will be created
- else if he modifies something (like black or style hook) he will not create the commit and change the files
  - when this occurs, we can see with git diff what the pre-commit change, or can just use the `--show-diff-on-failure` flag when running pre-commit.

I think that doesn't need everybody use pre-commit, can use both option (the actual format with running manually `make ...` and also using pre-commit) – but maybe don't make sense because will duplicate things? 

A little setup for pre-commit, i have tested here:

add `.pre-commit-config.yaml` - 
```yml
repos:
-   repo: https://github.com/psf/black
    rev: 22.1.0
    hooks:
    -   id: black
-   repo: https://github.com/pycqa/isort
    rev: 5.10.1
    hooks:
    -   id: isort
        name: isort (python)
-   repo: https://github.com/PyCQA/flake8
    rev: 4.0.1
    hooks:
    -   id: flake8
-   repo: local
    hooks:
    -   id: autogenerate_code
        name: autogenerate_code
        entry: python setup.py deps_table_update
        language: python
        types: [python]
        pass_filenames: false
    -   id: extra_style_checks
        name: extra_style_checks
        entry: make extra_style_checks
        language: system
```
Note:
    - The hooks _autogenerate_code_ and _extra_style_checks_, can be call using the make command or running the python.

Install pre-commit:
`pre-commit install`

Modify src/transformers/activations.py:
```diff
@@ -31,7 +31,8 @@ class NewGELUActivation(nn.Module):
     """
     def forward(self, input: Tensor) -> Tensor:
-        return 0.5 * input * (1.0 + torch.tanh(math.sqrt(2.0 / math.pi) * (input + 0.044715 * torch.pow(input, 3.0))))
+        return 0.5 * input * (1.0 + torch.tanh(math.sqrt(2.0 /
+                                                         math.pi) * (input + 0.044715 * torch.pow(input, 3.0))))
```
```console
$ git add -u
$ git commit -m "test pre-commit pipeline"

black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted src/transformers/activations.py

All done! ✨ 🍰 ✨
1 file reformatted.

isort (python)...........................................................Passed
flake8...................................................................Passed
autogenerate_code........................................................Passed
extra_style_checks.......................................................Passed

$ git status
On branch master
Your branch is up to date with 'origin/master'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   src/transformers/activations.py

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   src/transformers/activations.py

$ git diff
--- a/src/transformers/activations.py
+++ b/src/transformers/activations.py
@@ -31,8 +31,7 @@ class NewGELUActivation(nn.Module):
     """
 
     def forward(self, input: Tensor) -> Tensor:
-        return 0.5 * input * (1.0 + torch.tanh(math.sqrt(2.0 /
-                                                         math.pi) * (input + 0.044715 * torch.pow(input, 3.0))))
+        return 0.5 * input * (1.0 + torch.tanh(math.sqrt(2.0 / math.pi) * (input + 0.044715 * torch.pow(input, 3.0))))
```


to show git diff automatically after the pre-commit can add:
```yml
-   repo: local
    hooks:
    -   id: git-diff
        name: git diff
        entry: git diff --exit-code
        language: system
        pass_filenames: false
        always_run: true
```


Even though I originally created this thread 1.5 years later I now agree with @sgugger, that I don't want format changes done while pushing - I need to see what has been changed since sometimes the autoformatter messes things up badly and I need to rewrite things to make the end result readable.

If this can be done as an option and not a requirement then I'm not against it, but there needs to be a way to validate/reformat files before git is involved.

BTW, `precommit` can be run manually as well and not via git, which doesn't require `pre-commit install`:

```
pre-commit run --all-files
```

And we have 2 ways to reformat files: `fixup` (fast - only modified files) - `style` (slow)

yes use pre-commit don't make sense if does not want to always run the pipeline...

About the `fixup` and  `style`, i think can be done equal... by default pre-commit will run just in modified files (files at the commit) and if wants to run for all files can do as you shows.
For me, by default, i think makes sense always just run at modified files. And if the autoformatter messes things we can see, and if we prefer not to use some hook (like the autoformatter that have messed up something), by example run again with `SKIP=black ...`

And the pre-commit tool will not let the commit be created if something fails, if the dev wants “force” the failed hook will need to add the `SKIP=hook ...` before the commit command

(i personally agree with @sgugger that local hooks are best left as user-level tooling)