[PATCH] D130108: git-clang-format: format index not worktree when using --staged

[Mészáros Gergely via Phabricator via cfe-commits]

Maetveis added a comment.

In D130108#3706550 <https://reviews.llvm.org/D130108#3706550>, @MyDeveloperDay wrote:

> So trying to understand the problem statement, is it:
>
> 1. you have some files staged
> 2. and then you have to change them locally (review comment)
> 3. but forget to git add them,
> 4. you run git clang-format --staged
> 5. and it formats the new file (not added) based on the line numbers that are changing in the already staged file, which could be a different range to the original (is that correct?)
> 6. You think you've formatted the new file correctly  (but likely missed a line or two)
> 7. You perform git add (as you now notice you've missed adding it)
> 8. You commit, but a commit hook tells you that you didn't clang format.  (or worse still you are able to commit and push but you are pushing a badly formatted file, potentially)
>
> is that correct?

Not exactly, in 5 `git-clang-format --staged` would refuse to change anything because the same file is modified in both the index and the worktree.
But with `--diff`, yes it would run clang-format on the new file, but with the line numbers based on the index, then show the diff with this result.

Also at 8 there would be no problem, as there's no two versions of the file anymore. This is only an issue when there is a file that is different in the index than on disk (Not all changes to it are staged).

I think this captures the original case well:

1. Have a commit hook script that uses `git-clang-format --staged --diff` to reject commits that would be badly formatted
2. Change a file, add changes with incorrect formatting
3. add the file to the index via `git add`
4. Try to commit
5. The commit hook rejects the commit, gives the diff that needs to be applied to fix
6. Fix the formatting, but forget to `git add`
7. Commit
8. Commit succeeds, but it should have failed, the resulting commit has incorrect formatting (because it does not include the formatting changes)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130108/new/

https://reviews.llvm.org/D130108