From: Phillip Wood Date: Tue, 23 Nov 2021 11:05:16 GMT Subject: Re: [PATCH] editor: only save (and restore) the terminal if using a tty Message-ID: In-Reply-To: <20211122222850.674-1-carenas@gmail.com> Hi Carlo On 22/11/2021 22:28, Carlo Marcelo Arenas Belón wrote: > If the editor is invoked without a controlling terminal, then > saving the state and restoring it later is not very useful and > could generate signals that the invoking process wouldn't know > how to handle. > > if git's standard output is not connected to a terminal, then > presume there is no need to worry if the invoking terminal could > garble it. Checking if stdout is a terminal fixes the Eclipse case where stdout is a pipe or /dev/null but if git is started in the background from a terminal then calling isatty() will not prevent git from receiving SIGTTOU. For example if the user is using a gui editor then the following used to work GIT_EDITOR=gedit git commit& Now git receives SIGTTOU when the editor exits because we call tcsetattr() from a background process group. One can argue it does not make much sense to be starting git in the background but it did work before these changes. I think a combination of isatty() and tcgetpgrp() is probably the best solution. Best Wishes Phillip > Reported-by: Alexander Veit > Signed-off-by: Carlo Marcelo Arenas Belón > --- > editor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/editor.c b/editor.c > index 674309eed8..214e3834cb 100644 > --- a/editor.c > +++ b/editor.c > @@ -86,7 +86,7 @@ static int launch_specified_editor(const char *editor, const char *path, > p.env = env; > p.use_shell = 1; > p.trace2_child_class = "editor"; > - term_fail = save_term(1); > + term_fail = isatty(1) ? save_term(1) : 1; > if (start_command(&p) < 0) { > if (!term_fail) > restore_term(); >