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

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

>  - Add a multi-valued configuration variable whose value is the name
>    of an editor program that needs this save/restore; optionally, we
>    may want a way to say "don't do save/restore on this editor",
>    e.g. "!emacs" may countermand an earlier value that would include
>    the editor in the list.
>
>  - Around the program invocation in launch_specified_editor(), check
>    the name of the editor against this list and do the save/restore
>    as necessary;
>
>  - When the variable is not defined in the configuration, pretend
>    that "vi" is on that list (coming up with the list of editors is
>    left as an exercise to readers).
>
> That would give us your flexibility to apply the save/restore on an
> arbitrary editor that is not "vi", Dscho's convenience to special
> case "vi" out of the box when unconfigured, and an escape hatch for
> "vi" users for whom it hurts to do the save/restore on their "vi".
>
> Hmm?

That's an overkill.

A single configuration variable as an escape hatch, that enables the
save/restore around editor invocation, whose default value is
determined by the name of the editor, is probably the right degree
of flexibility.

Something along this line, perhaps?

 editor.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git c/editor.c w/editor.c
index fdd3eeafa9..70d3f80966 100644
--- c/editor.c
+++ w/editor.c
@@ -3,6 +3,7 @@
 #include "strbuf.h"
 #include "run-command.h"
 #include "sigchain.h"
+#include "compat/terminal.h"
 
 #ifndef DEFAULT_EDITOR
 #define DEFAULT_EDITOR "vi"
@@ -47,6 +48,16 @@ const char *git_sequence_editor(void)
 	return editor;
 }
 
+static int prepare_term(const char *editor)
+{
+	int need_saverestore = !strcmp(editor, "vi");
+
+	git_config_get_bool("editor.stty", &need_saverestore);
+	if (need_saverestore)
+		return save_term(1);
+	return 0;
+}
+
 static int launch_specified_editor(const char *editor, const char *path,
 				   struct strbuf *buffer, const char *const *env)
 {
@@ -57,7 +68,7 @@ static int launch_specified_editor(const char *editor, const char *path,
 		struct strbuf realpath = STRBUF_INIT;
 		const char *args[] = { editor, NULL, NULL };
 		struct child_process p = CHILD_PROCESS_INIT;
-		int ret, sig;
+		int ret, sig, need_restore = 0;
 		int print_waiting_for_editor = advice_enabled(ADVICE_WAITING_FOR_EDITOR) && isatty(2);
 
 		if (print_waiting_for_editor) {
@@ -83,7 +94,10 @@ static int launch_specified_editor(const char *editor, const char *path,
 		p.env = env;
 		p.use_shell = 1;
 		p.trace2_child_class = "editor";
+		need_restore = prepare_term(editor);
 		if (start_command(&p) < 0) {
+			if (need_restore)
+				restore_term();
 			strbuf_release(&realpath);
 			return error("unable to start editor '%s'", editor);
 		}
@@ -91,6 +105,8 @@ 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 (need_restore)
+			restore_term();
 		strbuf_release(&realpath);
 		sig = ret - 128;
 		sigchain_pop(SIGINT);