From f42ca31d8d209a43342ea345a52fc0bd43d71cc8 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Nov 2012 17:41:04 -0500 Subject: launch_editor: refactor to use start/finish_command The launch_editor function uses the convenient run_command_* interface. Let's use the more flexible start_command and finish_command functions, which will let us manipulate the parent state while we're waiting for the child to finish. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- editor.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'editor.c') diff --git a/editor.c b/editor.c index d8340031d2..842f7829fc 100644 --- a/editor.c +++ b/editor.c @@ -37,8 +37,16 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; + struct child_process p; - if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env)) + memset(&p, 0, sizeof(p)); + p.argv = args; + p.env = env; + p.use_shell = 1; + if (start_command(&p) < 0) + return error("unable to start editor '%s'", editor); + + if (finish_command(&p)) return error("There was a problem with the editor '%s'.", editor); } -- cgit v1.2.3 From 913ef36093eac3ec78b5fb155cc2beb5843b1ce5 Mon Sep 17 00:00:00 2001 From: Paul Fox Date: Fri, 30 Nov 2012 17:41:26 -0500 Subject: launch_editor: ignore terminal signals while editor has control The user's editor likely catches SIGINT (ctrl-C). but if the user spawns a command from the editor and uses ctrl-C to kill that command, the SIGINT will likely also kill git itself (depending on the editor, this can leave the terminal in an unusable state). Let's ignore it while the editor is running, and do the same for SIGQUIT, which many editors also ignore. This matches the behavior if we were to use system(3) instead of run-command. Signed-off-by: Paul Fox Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- editor.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'editor.c') diff --git a/editor.c b/editor.c index 842f7829fc..c892a81ae3 100644 --- a/editor.c +++ b/editor.c @@ -1,6 +1,7 @@ #include "cache.h" #include "strbuf.h" #include "run-command.h" +#include "sigchain.h" #ifndef DEFAULT_EDITOR #define DEFAULT_EDITOR "vi" @@ -38,6 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; struct child_process p; + int ret; memset(&p, 0, sizeof(p)); p.argv = args; @@ -46,7 +48,12 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (start_command(&p) < 0) return error("unable to start editor '%s'", editor); - if (finish_command(&p)) + sigchain_push(SIGINT, SIG_IGN); + sigchain_push(SIGQUIT, SIG_IGN); + ret = finish_command(&p); + sigchain_pop(SIGINT); + sigchain_pop(SIGQUIT); + if (ret) return error("There was a problem with the editor '%s'.", editor); } -- cgit v1.2.3 From 1250857c6c2695020bce6669a4ff43d57a507d91 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Nov 2012 17:41:50 -0500 Subject: launch_editor: propagate signals from editor to git We block SIGINT and SIGQUIT while the editor runs so that git is not killed accidentally by a stray "^C" meant for the editor or its subprocesses. This works because most editors ignore SIGINT. However, some editor wrappers, like emacsclient, expect to die due to ^C. We detect the signal death in the editor and properly exit, but not before writing a useless error message to stderr. Instead, let's notice when the editor was killed by a terminal signal and just raise the signal on ourselves. This skips the message and looks to our parent like we received SIGINT ourselves. The end effect is that if the user's editor ignores SIGINT, we will, too. And if it does not, then we will behave as if we did not ignore it. That should make all users happy. Note that in the off chance that another part of git has ignored SIGINT while calling launch_editor, we will still properly detect and propagate the failed return code from the editor (i.e., the worst case is that we generate the useless error, not fail to notice the editor's death). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- editor.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'editor.c') diff --git a/editor.c b/editor.c index c892a81ae3..065a7abf2f 100644 --- a/editor.c +++ b/editor.c @@ -39,7 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; struct child_process p; - int ret; + int ret, sig; memset(&p, 0, sizeof(p)); p.argv = args; @@ -51,8 +51,11 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en sigchain_push(SIGINT, SIG_IGN); sigchain_push(SIGQUIT, SIG_IGN); ret = finish_command(&p); + sig = ret + 128; sigchain_pop(SIGINT); sigchain_pop(SIGQUIT); + if (sig == SIGINT || sig == SIGQUIT) + raise(sig); if (ret) return error("There was a problem with the editor '%s'.", editor); -- cgit v1.2.3