git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Nov 22, 2021, 23:08 UTC
Message-ID
<xmqq5ysjbxfm.fsf@gitster.g>
In-Reply-To
<xmqqa6hvbxob.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> 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 <gitster@pobox.com>
---
 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
Previous: Junio C HamanoNext: Alexander Veit
Message 6 of 25 in “Update to Git 2.34.0 breaks application”
  1. Alexander VeitNov 22, 2021
  2. Phillip WoodNov 22, 2021
  3. editor: only save (and restore) the terminal if using a ttyCarlo Marcelo Arenas Belón, Nov 22, 2021
  4. Junio C HamanoNov 22, 2021
  5. Junio C HamanoNov 22, 2021
  6. Junio C HamanoNov 22, 2021
  7. Alexander VeitNov 23, 2021
  8. Carlo ArenasNov 23, 2021
  9. Carlo ArenasNov 22, 2021
  10. Junio C HamanoNov 23, 2021
  11. Johannes SchindelinNov 24, 2021
  12. Carlo ArenasNov 24, 2021
  13. Junio C HamanoNov 24, 2021
  14. Carlo ArenasNov 24, 2021
  15. Junio C HamanoNov 24, 2021
  16. Johannes SchindelinNov 29, 2021
  17. Phillip WoodNov 23, 2021
  18. Junio C HamanoNov 23, 2021
  19. Carlo ArenasNov 23, 2021
  20. Phillip WoodNov 30, 2021
  21. Chris TorekDec 1, 2021
  22. Junio C HamanoDec 1, 2021
  23. Junio C HamanoDec 2, 2021
  24. Carlo ArenasDec 2, 2021
  25. Johannes SchindelinDec 2, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.