git-plus icon indicating copy to clipboard operation
git-plus copied to clipboard

Handle pre-commit hook failures better as to not lose commit messages

Open xenithorb opened this issue 9 years ago • 4 comments

This is not an enhancement, but a bug.

Currently, if you try and use git-plus with a repository that has a pre-configured git pre-commit hook, and you take the time to write out a nice long commit message, the message is completely lost if the pre-commit hook fails.

Steps to reproduce:

  1. Copy paste this stuff into bash to setup the test case:
    temp=$(mktemp -d) 
    git init "$temp"
    echo -e '#!/bin/bash\necho "ERROR: Simulating failed pre-commit hook"\nexit 1' > "$temp/.git/hooks/pre-commit"
    chmod +x "$temp/.git/hooks/pre-commit"
    touch "$temp/foo"
    atom "$temp/foo"
    
  2. Now type some stuff in atom, and hit CTRL+s to save
  3. Now type CTRL+SHIFT+A a to "Git add all and commit"
  4. Add a commit message to the newly minted COMMIT_EDITMSG pane
  5. Finally, press CTRL+s again to save the message (pane disappears)
  6. Observe the failure and how the commit 1. fails 2. message is not retained anywhere

The way git proper handles this on the commandline is by running the pre-commit hook before it asks for a message.

If you're like me and forget sometimes where you have pre-commit hooks in place, and you're like me and write out lengthy messages - this is a serious bug/annoyance to have to re-write everything you just worked hard to explain.

xenithorb avatar Dec 31 '16 12:12 xenithorb

Git will run the hook before asking for a message if no arguments are passed with git commit.

Commits are handled by the package by passing the file, in which you've written the commit message into, to the commit command in the shell with git commit --file path/to/COMMIT_EDITMSG. After everything finishes, regardless of success, the COMMIT_EDITMSG file gets deleted. Perhaps it is better to keep the file or cache the failed commit message so it is easily retrievable.

akonwi avatar Jan 03 '17 16:01 akonwi

This seems to have made it to 7.0.5, though Is see you left it open still. You're aware that this does not pass the test case?

xenithorb avatar Jan 05 '17 23:01 xenithorb

I typo'd and entered the wrong issue number in that commit message. Those changes were not related to this that's why I left it open

akonwi avatar Jan 05 '17 23:01 akonwi

I understand, no problem. I was just eager to test and a cursory look-over of the changeset seemed to imply a dialog. Thanks!

xenithorb avatar Jan 05 '17 23:01 xenithorb