Print commit message when commit_msg hooks fail #519
Conversation
So the user can copy-paste their commit message when trying to commit again, and not lose it. Removes frustration for users who craft their commit messages with care. The user can always get their failed message from `.git/COMMIT_EDITMSG` (depending on the version of git, I think), but only if they copy before running `git commit` again, which they may forget to do sometimes.
lib/overcommit/hook_runner.rb
Outdated
| hook_failed = @failed || @interrupted | ||
|
|
||
| if hook_failed && @context.respond_to?(:post_fail_message) | ||
| puts @context.post_fail_message |
There was a problem hiding this comment.
The printer should probably be used here, right? I wasn't sure yet how to use the printer for this, so I wanted to commit and get a discussion going first, since this is my first ruby PR.
There was a problem hiding this comment.
I couldn't find a test for run_hooks so I wasn't sure how to test this. Happy to add a test if I could be pointed to where?
There was a problem hiding this comment.
Yes, let's use the printer class (and give it a specific method for this case). Thanks!
lib/overcommit/hook_runner.rb
Outdated
| !(@failed || @interrupted) | ||
| hook_failed = @failed || @interrupted | ||
|
|
||
| if hook_failed && @context.respond_to?(:post_fail_message) |
There was a problem hiding this comment.
I thought it was best to allow such a method to not be implemented on the individual HookContext subclasses, but work here if it was. Right now only HookContext::CommitMsg implements it, to solve #494.
|
Bump! I'd like to know if this would be accepted before I investigate how to improve/fix this PR =) |
There was a problem hiding this comment.
Thanks for the PR, @henrahmagix. My apologies for the delayed response. I know it can be frustrating to put effort into a PR and not get feedback.
Overall this approach is fine. I have some minor comments which once addressed will have me happy to merge. Thanks!
lib/overcommit/hook_runner.rb
Outdated
| hook_failed = @failed || @interrupted | ||
|
|
||
| if hook_failed && @context.respond_to?(:post_fail_message) | ||
| puts @context.post_fail_message |
There was a problem hiding this comment.
Yes, let's use the printer class (and give it a specific method for this case). Thanks!
|
|
||
| def post_fail_message | ||
| "Failed commit message:\n" + commit_message_lines.join | ||
| end |
There was a problem hiding this comment.
Let's define a default post_fail_message method in the base HookContext class so we don't need to do the @context.respond_to?(:post_fail_message) below. We can have a contract where returning nil results in nothing being printed.
|
@sds No worries; thanks for responding! I've pushed the changes =) |
henrahmagix
force-pushed
the
echo-failed-commit-message
branch
from
2b935f5 to
200f6cf
Compare
|
The build failures are out of my hands unfortunately. I think the Rubocop ones are errors on lots of files that haven't been changed here, but I'm also not sure if my dev environment is pulling in more rules than the build environment. |
|
Thanks for making those changed, @henrahmagix. I've merged in 167c13c and a0dafc0 and made some small modifications in 0f40c0d to address the RuboCop warnings. Thanks again! |
|
Thanks so much @sds! Really appreciate it - and your method name choice is way better =D |
Modify on the technique introduced in #519 to instead provide clear instructions on how to re-attempt your commit message starting from the one you previously had. This reduces large amounts of output pasted to the terminal, and is instructional to users who aren't familiar with this feature of Git. Now commit-msg failures will be followed up with: Try again with your existing commit message by running: git commit --edit --file=.git/COMMIT_EDITMSG The reason we don't attempt to simply re-open the editor is that by the time we've failed the hook, Git has already decided the run has failed and there is no means of recovery. Closes #651.
Modify on the technique introduced in #519 to instead provide clear instructions on how to re-attempt your commit message starting from the one you previously had. This is instructional to users who aren't familiar with this feature of Git. Now commit-msg failures will be followed up with: Try again with your existing commit message by running: git commit --edit --file=.git/COMMIT_EDITMSG The reason we don't attempt to simply re-open the editor is that by the time we've failed the hook, Git has already decided the run has failed and there is no means of recovery. Closes #651.
Modify on the technique introduced in #519 to instead provide clear instructions on how to re-attempt your commit message starting from the one you previously had. This is instructional to users who aren't familiar with this feature of Git. Now commit-msg failures will be followed up with: Try again with your existing commit message by running: git commit --edit --file=.git/COMMIT_EDITMSG The reason we don't attempt to simply re-open the editor is that by the time we've failed the hook, Git has already decided the run has failed and there is no means of recovery. Closes #651.
2 participants
Fix #494
I am new to ruby, gems, and this project, so please forgive any "nasties" or other mistakes! I'm happy to change or reimplement this however the maintainers would like =)