Re: [PATCH] editor: only save (and restore) the terminal if using a tty
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Nov 23, 2021, 11:05 UTC
- Message-ID
- <b1f2257a-044c-17bb-2737-42b8026421eb@gmail.com>
- In-Reply-To
- <20211122222850.674-1-carenas@gmail.com>
Hi Carlo
On 22/11/2021 22:28, Carlo Marcelo Arenas Belón wrote:
Show 8 quoted lines
> 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
Show 20 quoted lines
> Reported-by: Alexander Veit <alexander.veit@gmx.net>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> 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();
>