Re: [PATCH] editor: only save (and restore) the terminal if using a tty
Junio C Hamano <gitster@pobox.com> writes:
Show 20 quoted lines
> - 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);
Alexander Veit
