Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't unstage on failure #14

Open
saferoll opened this issue Nov 17, 2017 · 5 comments
Open

Don't unstage on failure #14

saferoll opened this issue Nov 17, 2017 · 5 comments

Comments

@saferoll
Copy link

saferoll commented Nov 17, 2017

Why git reset all files when the check fails? This is REALLY annoying.

@kbenzie
Copy link
Owner

kbenzie commented Nov 17, 2017

Its been a long time since I've looked at this if I'm honest but I think it was possible to commit the patch even though the hook rejected it. If this is not the case then I'm happy to accept a PR removing this but please do ensure that hook isn't being circumvented. Apologies for the annoyance @saferoll.

@kbenzie
Copy link
Owner

kbenzie commented Nov 17, 2017

Actually I've just remembered why! clang-format is being run on the files on disk not the staged changes, so once the files on disk are formatted correctly the hook will not fail even though the patch contains badly unformatted code which is the opposite of what is desired.

@saferoll
Copy link
Author

But why call this?: subprocess.Popen([Git, "reset", "HEAD", "--", "."])

@kbenzie
Copy link
Owner

kbenzie commented Nov 19, 2017

As I said before, the staged changes contain badly formatted code but after running clang-format the hook will not fail, this relies on the user to stage the newly formatted code but if this is not done then badly formatted code could be committed. I will admit this is a heavy handed approach and am open to suggestions for better solutions.

@upsj
Copy link

upsj commented Dec 3, 2019

Actually I've just remembered why! clang-format is being run on the files on disk not the staged changes, so once the files on disk are formatted correctly the hook will not fail even though the patch contains badly unformatted code which is the opposite of what is desired.

In case you want to resolve this issue: clang-format is actually being executed on the output of git show, i.e., the staged changes only (see L58)
The git reset call can thus be safely removed, as no files are changed by clang-format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants