I have used git-clang-format for incremental formatting in some projects adding a git pre-commit hook e.g in rocPRIM.
One of the things the githook does is it gives the user the command to run in case formatting needs to be applied, can be seen here in rocPRIM.
Using this we have discovered that if the command is run which applies the changes, but then they are not added to the index (i.e. developer forgets to do
git add), then
git-clang-format --staged will accept, and an improperly formatted change will be committed.
I have tracked this down to that
git-clang-format compares the index in its
--staged mode but runs then clang-format on the worktree and check if it changed anything. I think it should format the index in this case. I have a patch ready to fix it this way, but I’m unsure if I’m missing anything.
Some additional questions, because I’m completely new to llvm contributions:
- git-clang-format seems to have no tests yet, I can post a small git repository and commands on how I was testing this, but I don’t know If I need to make that into a formal test script, and if yes how
- Should I submit a patch with this to phabricator, any suggested reviewers?