Open ramsey opened 3 years ago
So the idea is to prevent pre-commit
hooks to commit unwanted changes to the repository.
That would mean that we had to configure an additional pre-commit
hook that cleans up the working tree before any other action just as you described.
If we removed any changes from the working directory we have to make sure no other action changes anything in the index otherwise re-applying the removed changes could potentially fail.
I'm not yet sure how you want to detect those potential changes but if it is doable I think it is a valid safe guard to implement.
Use Case
Developers often use pre-commit hooks to run style-checkers that automatically fix style issues and add them to the staged commit before committing. This is problematic when the index contains changes to a file that also has changes in the working tree (that are not in the index). Unless we first move these changes out of the way, the pre-commit hook will see that changes exist in these files and try to add them to the index.
As a developer, I want any working tree changes to be moved out of the way before applying pre-commit hooks, so that these changes do not get added to the index by pre-commit hooks. After applying all pre-commit hooks, the working tree changes should be moved back to the working tree.
Discussion
You'll note that I didn't use the word "stash" in the use case description. This is because
git stash
does not effectively perform the operation we need.git stash [push]
by itself adds everything in the working tree (including the index) to the stash. When usinggit stash push --keep-index
, the current index remains active, but it is also stored to the stash along with the working tree changes. There's no way to stash only the working tree changes, which is what we need.I've taken a look at various approaches for this, and I've landed on the following two popular tools:
After examining both and discussing pre-commit's approach with the maintainer on Twitter, I've decided to use pre-commit as a model.
In short, pre-commit's approach is to return a non-zero status if any pre-commit hook changes a file.
This may not be what everyone wants to see, however. Some folks might expect these changes to be added to the index and commited as part of the current commit. This is something we can allow, but I think the risk for unwanted changes being added to a commit is too great. It also increases the chances of a conflict occurring when we add the moved working tree changes back into the working tree.
Here's what I propose:
git commit
By failing the commit if any hook makes changes, we require the developer to analyze the reasons for failure, fix them, stage any new changes, and try the commit again.
Thoughts?
Implemention
I've begun stubbing out an implementation, so I can see if this is a workable solution. So far, it looks good. I'm opening this issue to get feedback on the approach before I continue with more development. The Status operator PR (https://github.com/sebastianfeldmann/git/pull/23) on sebastianfeldmann/git is one of the tools I needed to make this work in CaptainHook.