search

Fakerr / git-recall

An interactive way to peruse your git history from the terminal
MIT License
2.12k stars 47 forks source link

Handle Ctrl-C interrupt #34

Closed ryanc414 closed 7 years ago

ryanc414 commented 7 years ago

Out of habit I'd often exit this script with Ctrl-C instead of by hitting q, but this would cause problems with the cursor never being unhidden and hitting Ctrl-C in diff view would cause the terminal to fill with junk. Fixed by trapping Ctrl-C and resetting cursor and colours before exiting.

Fakerr commented 7 years ago

Hey @ryanc414, thanks for your contribution, I find this one interesting, however it doesn't seem to work for me. I'm on ubuntu 14.04 with bash 4.3. What platform did you use to test it ? For me whenever I try to Ctrl-c, the cursor appears again but the terminal gets completely broken (can no longer see commands, pressing enter will print the prompt on the right instead in a new line, ...).

ryanc414 commented 7 years ago

No problem. Ah that's unfortunate, it worked OK for me testing on Arch Linux 4.10.1 with Bash 4.4.12, running in xfce4-terminal 0.8.4. I've also been using it on Centos 7 without a problem. Maybe there's a Ubuntu-specific problem I could look into at some point...

ryanc414 commented 7 years ago

I was able to reproduce the problem on Ubuntu 14.04. My most recent commit fixes the problem for me - the stty settings were wrong after exiting so I made sure to restore the original settings in the cleanup function. Let me know if this still doesn't work for you!

Fakerr commented 7 years ago

Hey @ryanc414, indeed, restoring the original settings fixed the problem, thanks for that :+1: , however, there's still something that I'm not convinced about. Hitting Ctrl-C while using less won't do anything, so I don't see the point to trap Ctrl-C within less and cleaning up everything after quitting. I think it would be better if we untrap Ctrl-C before using less (this command should do the job: trap - INT), and then trap again. What do you think about that ? (sorry for the late response, busy week ...)

Fakerr commented 7 years ago

I'm gonna go ahead and merge it, but I'll add some changes, mainly the point that described above.

ryanc414 commented 7 years ago

For me, hitting Ctrl-C when in the diff view using less does exit the program uncleanly. I think it depends on whether the interrupt is delivered to the git-recall script or the parent git process (not sure what determines this exactly). It's not a major problem though so I'm happy for this merge to be closed.