Re: [PATCH] editor: only save (and restore) the terminal if using a tty

Junio C Hamano [off-list ref] writes:

In any case, I am quite tempted to just revert the offending topic
for now, but later accept a resurrection patch with this isatty
check rolled in (either at this caller, or inside save_term) when
the dust settles.

So, here is that first step, to be hopefully mergeable to 'master'
and also to 'maint' for 2.34.1

----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
Subject: [PATCH] Revert "editor: save and reset terminal after calling EDITOR"

This reverts commit 3d411afabc9a96f41d47c07d6af6edda3d29ec92,
blindly opening /dev/tty and calling tcsetattr() seems to be causing
problems.

cf. https://bugs.eclipse.org/bugs/show_bug.cgi?id=577358
cf. https://lore.kernel.org/git/04ab7301-ea34-476c-eae4-4044fef74b91@gmail.com/

Signed-off-by: Junio C Hamano <redacted>
---
 editor.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/editor.c b/editor.c
index be7441e7e0..6303ae0ab0 100644
--- a/editor.c
+++ b/editor.c

@@ -3,7 +3,6 @@
 #include "strbuf.h"
 #include "run-command.h"
 #include "sigchain.h"
-#include "compat/terminal.h"
 
 #ifndef DEFAULT_EDITOR
 #define DEFAULT_EDITOR "vi"

@@ -51,8 +50,6 @@ const char *git_sequence_editor(void)
 static int launch_specified_editor(const char *editor, const char *path,
 				   struct strbuf *buffer, const char *const *env)
 {
-	int term_fail;
-
 	if (!editor)
 		return error("Terminal is dumb, but EDITOR unset");
 

@@ -86,10 +83,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);
 		if (start_command(&p) < 0) {
-			if (!term_fail)
-				restore_term();
 			strbuf_release(&realpath);
 			return error("unable to start editor '%s'", editor);
 		}

@@ -97,8 +91,6 @@ static int launch_specified_editor(const char *editor, const char *path,
 		sigchain_push(SIGINT, SIG_IGN);
 		sigchain_push(SIGQUIT, SIG_IGN);
 		ret = finish_command(&p);
-		if (!term_fail)
-			restore_term();
 		strbuf_release(&realpath);
 		sig = ret - 128;
 		sigchain_pop(SIGINT);

-- 
2.34.0-222-gceadac38b1

Am 23.11.21 um 00:08 schrieb Junio C Hamano:

So, here is that first step, to be hopefully mergeable to 'master'
and also to 'maint' for 2.34.1

I've applied the patch to 'maint' and it worked. The application does not freeze anymore.

-Alex

On Tue, Nov 23, 2021 at 12:52 AM Alexander Veit [off-list ref] wrote:

Am 23.11.21 um 00:08 schrieb Junio C Hamano:

quoted

So, here is that first step, to be hopefully mergeable to 'master'
and also to 'maint' for 2.34.1

I've applied the patch to 'maint' and it worked. The application does not freeze anymore.

great news; apologies for the disruption.

Carlo