fix(ui): re-organize tty fd handling and fix issues

- Use the correct fd to replace stdin on windows (CONIN)
- Don't start the TUI if there are no tty fd (not a regression,
  but makes sense regardless)
- De-mythologize "global input fd". it is just STDIN.
This commit is contained in:
bfredl
2023-01-10 14:03:15 +01:00
parent 43feb973e3
commit 160c69b655
10 changed files with 49 additions and 63 deletions

View File

@@ -494,6 +494,9 @@ EXTERN int v_dying INIT(= 0);
EXTERN bool stdin_isatty INIT(= true); EXTERN bool stdin_isatty INIT(= true);
// is stdout a terminal? // is stdout a terminal?
EXTERN bool stdout_isatty INIT(= true); EXTERN bool stdout_isatty INIT(= true);
// is stderr a terminal?
EXTERN bool stderr_isatty INIT(= true);
/// filedesc set by embedder for reading first buffer like `cmd | nvim -` /// filedesc set by embedder for reading first buffer like `cmd | nvim -`
EXTERN int stdin_fd INIT(= -1); EXTERN int stdin_fd INIT(= -1);

View File

@@ -289,7 +289,13 @@ int main(int argc, char **argv)
} }
} }
bool use_builtin_ui = (!headless_mode && !embedded_mode && !silent_mode); #ifdef MSWIN
// on windows we use CONIN special file, thus we don't know this yet.
bool has_term = true;
#else
bool has_term = (stdin_isatty || stdout_isatty || stderr_isatty);
#endif
bool use_builtin_ui = (has_term && !headless_mode && !embedded_mode && !silent_mode);
// don't bind the server yet, if we are using builtin ui. // don't bind the server yet, if we are using builtin ui.
// This will be done when nvim server has been forked from the ui process // This will be done when nvim server has been forked from the ui process
@@ -305,7 +311,7 @@ int main(int argc, char **argv)
bool remote_ui = (ui_client_channel_id != 0); bool remote_ui = (ui_client_channel_id != 0);
if (use_builtin_ui && !remote_ui) { if (use_builtin_ui && !remote_ui) {
ui_client_forward_stdin = !params.input_isatty; ui_client_forward_stdin = !stdin_isatty;
uint64_t rv = ui_client_start_server(params.argc, params.argv); uint64_t rv = ui_client_start_server(params.argc, params.argv);
if (!rv) { if (!rv) {
os_errmsg("Failed to start Nvim server!\n"); os_errmsg("Failed to start Nvim server!\n");
@@ -362,8 +368,8 @@ int main(int argc, char **argv)
debug_break_level = params.use_debug_break_level; debug_break_level = params.use_debug_break_level;
// Read ex-commands if invoked with "-es". // Read ex-commands if invoked with "-es".
if (!params.input_isatty && !params.input_istext && silent_mode && exmode_active) { if (!stdin_isatty && !params.input_istext && silent_mode && exmode_active) {
input_start(STDIN_FILENO); input_start();
} }
if (ui_client_channel_id) { if (ui_client_channel_id) {
@@ -640,8 +646,8 @@ void os_exit(int r)
if (!event_teardown() && r == 0) { if (!event_teardown() && r == 0) {
r = 1; // Exit with error if main_loop did not teardown gracefully. r = 1; // Exit with error if main_loop did not teardown gracefully.
} }
if (input_global_fd() >= 0) { if (used_stdin) {
stream_set_blocking(input_global_fd(), true); // normalize stream (#2598) stream_set_blocking(STDIN_FILENO, true); // normalize stream (#2598)
} }
ILOG("Nvim exit: %d", r); ILOG("Nvim exit: %d", r);
@@ -786,9 +792,9 @@ void preserve_exit(void)
// Prevent repeated calls into this method. // Prevent repeated calls into this method.
if (really_exiting) { if (really_exiting) {
if (input_global_fd() >= 0) { if (used_stdin) {
// normalize stream (#2598) // normalize stream (#2598)
stream_set_blocking(input_global_fd(), true); stream_set_blocking(STDIN_FILENO, true);
} }
exit(2); exit(2);
} }
@@ -964,7 +970,7 @@ static bool edit_stdin(mparm_T *parmp)
bool implicit = !headless_mode bool implicit = !headless_mode
&& !(embedded_mode && stdin_fd <= 0) && !(embedded_mode && stdin_fd <= 0)
&& (!exmode_active || parmp->input_istext) && (!exmode_active || parmp->input_istext)
&& !parmp->input_isatty && !stdin_isatty
&& parmp->scriptin == NULL; // `-s -` was not given. && parmp->scriptin == NULL; // `-s -` was not given.
return parmp->had_stdin_file || implicit; return parmp->had_stdin_file || implicit;
} }
@@ -1450,11 +1456,9 @@ static void init_startuptime(mparm_T *paramp)
static void check_and_set_isatty(mparm_T *paramp) static void check_and_set_isatty(mparm_T *paramp)
{ {
stdin_isatty stdin_isatty = os_isatty(STDIN_FILENO);
= paramp->input_isatty = os_isatty(STDIN_FILENO); stdout_isatty = os_isatty(STDOUT_FILENO);
stdout_isatty stderr_isatty = os_isatty(STDERR_FILENO);
= paramp->output_isatty = os_isatty(STDOUT_FILENO);
paramp->err_isatty = os_isatty(STDERR_FILENO);
TIME_MSG("window checked"); TIME_MSG("window checked");
} }

View File

@@ -30,10 +30,7 @@ typedef struct {
char *tagname; // tag from -t argument char *tagname; // tag from -t argument
char *use_ef; // 'errorfile' from -q argument char *use_ef; // 'errorfile' from -q argument
bool input_isatty; // stdin is a terminal
bool input_istext; // stdin is text, not executable (-E/-Es) bool input_istext; // stdin is text, not executable (-E/-Es)
bool output_isatty; // stdout is a terminal
bool err_isatty; // stderr is a terminal
int no_swap_file; // "-n" argument used int no_swap_file; // "-n" argument used
int use_debug_break_level; int use_debug_break_level;

View File

@@ -44,7 +44,6 @@ typedef enum {
static Stream read_stream = { .closed = true }; // Input before UI starts. static Stream read_stream = { .closed = true }; // Input before UI starts.
static RBuffer *input_buffer = NULL; static RBuffer *input_buffer = NULL;
static bool input_eof = false; static bool input_eof = false;
static int global_fd = -1;
static bool blocking = false; static bool blocking = false;
static int cursorhold_time = 0; ///< time waiting for CursorHold event static int cursorhold_time = 0; ///< time waiting for CursorHold event
static int cursorhold_tb_change_cnt = 0; ///< tb_change_cnt when waiting started static int cursorhold_tb_change_cnt = 0; ///< tb_change_cnt when waiting started
@@ -58,25 +57,14 @@ void input_init(void)
input_buffer = rbuffer_new(INPUT_BUFFER_SIZE + MAX_KEY_CODE_LEN); input_buffer = rbuffer_new(INPUT_BUFFER_SIZE + MAX_KEY_CODE_LEN);
} }
void input_global_fd_init(int fd) void input_start(void)
{
global_fd = fd;
}
/// Global TTY (or pipe for "-es") input stream, before UI starts.
int input_global_fd(void)
{
return global_fd;
}
void input_start(int fd)
{ {
if (!read_stream.closed) { if (!read_stream.closed) {
return; return;
} }
input_global_fd_init(fd); used_stdin = true;
rstream_init_fd(&main_loop, &read_stream, fd, READ_BUFFER_SIZE); rstream_init_fd(&main_loop, &read_stream, STDIN_FILENO, READ_BUFFER_SIZE);
rstream_start(&read_stream, input_read_cb, NULL); rstream_start(&read_stream, input_read_cb, NULL);
} }

View File

@@ -7,6 +7,8 @@
#include "nvim/api/private/defs.h" #include "nvim/api/private/defs.h"
#include "nvim/event/multiqueue.h" #include "nvim/event/multiqueue.h"
EXTERN bool used_stdin INIT(= false);
#ifdef INCLUDE_GENERATED_DECLARATIONS #ifdef INCLUDE_GENERATED_DECLARATIONS
# include "os/input.h.generated.h" # include "os/input.h.generated.h"
#endif #endif

View File

@@ -14,7 +14,7 @@ static HWND hWnd = NULL;
static HICON hOrigIconSmall = NULL; static HICON hOrigIconSmall = NULL;
static HICON hOrigIcon = NULL; static HICON hOrigIcon = NULL;
int os_get_conin_fd(void) int os_open_conin_fd(void)
{ {
const HANDLE conin_handle = CreateFile("CONIN$", const HANDLE conin_handle = CreateFile("CONIN$",
GENERIC_READ | GENERIC_WRITE, GENERIC_READ | GENERIC_WRITE,
@@ -30,7 +30,7 @@ int os_get_conin_fd(void)
void os_replace_stdin_to_conin(void) void os_replace_stdin_to_conin(void)
{ {
close(STDIN_FILENO); close(STDIN_FILENO);
const int conin_fd = os_get_conin_fd(); const int conin_fd = os_open_conin_fd();
assert(conin_fd == STDIN_FILENO); assert(conin_fd == STDIN_FILENO);
} }

View File

@@ -1064,7 +1064,7 @@ func Test_io_not_a_terminal()
\ 'Vim: Warning: Input is not from a terminal'], l) \ 'Vim: Warning: Input is not from a terminal'], l)
endfunc endfunc
" Test for --not-a-term avoiding escape codes. " Test for not being a term avoiding escape codes.
func Test_not_a_term() func Test_not_a_term()
CheckUnix CheckUnix
CheckNotGui CheckNotGui
@@ -1075,18 +1075,14 @@ func Test_not_a_term()
let redir = &shellredir .. ' Xvimout' let redir = &shellredir .. ' Xvimout'
endif endif
" Without --not-a-term there are a few escape sequences. " As nvim checks the environment by itself there will be no escape sequences
" This will take 2 seconds because of the missing --not-a-term " This will also happen to take two (2) seconds.
let cmd = GetVimProg() .. ' --cmd quit ' .. redir let cmd = GetVimProg() .. ' --cmd quit ' .. redir
exe "silent !" . cmd exe "silent !" . cmd
call assert_match("\<Esc>", readfile('Xvimout')->join()) call assert_notmatch("\e", readfile('Xvimout')->join())
call delete('Xvimout') call delete('Xvimout')
" With --not-a-term there are no escape sequences. " --not-a-term flag has thus been deleted
let cmd = GetVimProg() .. ' --not-a-term --cmd quit ' .. redir
exe "silent !" . cmd
call assert_notmatch("\<Esc>", readfile('Xvimout')->join())
call delete('Xvimout')
endfunc endfunc

View File

@@ -153,19 +153,7 @@ void tinput_init(TermInput *input, Loop *loop)
kitty_key_map_entry[i].name); kitty_key_map_entry[i].name);
} }
// If stdin is not a pty, switch to stderr. For cases like: input->in_fd = STDIN_FILENO;
// echo q | nvim -es
// ls *.md | xargs nvim
#ifdef MSWIN
if (!os_isatty(input->in_fd)) {
input->in_fd = os_get_conin_fd();
}
#else
if (!os_isatty(input->in_fd) && os_isatty(STDERR_FILENO)) {
input->in_fd = STDERR_FILENO;
}
#endif
input_global_fd_init(input->in_fd);
const char *term = os_getenv("TERM"); const char *term = os_getenv("TERM");
if (!term) { if (!term) {
@@ -174,7 +162,7 @@ void tinput_init(TermInput *input, Loop *loop)
input->tk = termkey_new_abstract(term, input->tk = termkey_new_abstract(term,
TERMKEY_FLAG_UTF8 | TERMKEY_FLAG_NOSTART); TERMKEY_FLAG_UTF8 | TERMKEY_FLAG_NOSTART);
termkey_hook_terminfo_getstr(input->tk, input->tk_ti_hook_fn, NULL); termkey_hook_terminfo_getstr(input->tk, input->tk_ti_hook_fn, input);
termkey_start(input->tk); termkey_start(input->tk);
int curflags = termkey_get_canonflags(input->tk); int curflags = termkey_get_canonflags(input->tk);

View File

@@ -1342,7 +1342,7 @@ static void suspend_event(void **argv)
TUIData *tui = argv[0]; TUIData *tui = argv[0];
bool enable_mouse = tui->mouse_enabled; bool enable_mouse = tui->mouse_enabled;
tui_terminal_stop(tui); tui_terminal_stop(tui);
stream_set_blocking(input_global_fd(), true); // normalize stream (#2598) stream_set_blocking(tui->input.in_fd, true); // normalize stream (#2598)
kill(0, SIGTSTP); kill(0, SIGTSTP);
@@ -1351,7 +1351,7 @@ static void suspend_event(void **argv)
if (enable_mouse) { if (enable_mouse) {
tui_mouse_on(tui); tui_mouse_on(tui);
} }
stream_set_blocking(input_global_fd(), false); // libuv expects this stream_set_blocking(tui->input.in_fd, false); // libuv expects this
} }
#endif #endif
@@ -2238,12 +2238,12 @@ static void flush_buf(TUIData *tui)
/// ///
/// @see tmux/tty-keys.c fe4e9470bb504357d073320f5d305b22663ee3fd /// @see tmux/tty-keys.c fe4e9470bb504357d073320f5d305b22663ee3fd
/// @see https://bugzilla.redhat.com/show_bug.cgi?id=142659 /// @see https://bugzilla.redhat.com/show_bug.cgi?id=142659
static const char *tui_get_stty_erase(void) static const char *tui_get_stty_erase(int fd)
{ {
static char stty_erase[2] = { 0 }; static char stty_erase[2] = { 0 };
#if defined(HAVE_TERMIOS_H) #if defined(HAVE_TERMIOS_H)
struct termios t; struct termios t;
if (tcgetattr(input_global_fd(), &t) != -1) { if (tcgetattr(fd, &t) != -1) {
stty_erase[0] = (char)t.c_cc[VERASE]; stty_erase[0] = (char)t.c_cc[VERASE];
stty_erase[1] = '\0'; stty_erase[1] = '\0';
DLOG("stty/termios:erase=%s", stty_erase); DLOG("stty/termios:erase=%s", stty_erase);
@@ -2256,9 +2256,10 @@ static const char *tui_get_stty_erase(void)
/// @see TermInput.tk_ti_hook_fn /// @see TermInput.tk_ti_hook_fn
static const char *tui_tk_ti_getstr(const char *name, const char *value, void *data) static const char *tui_tk_ti_getstr(const char *name, const char *value, void *data)
{ {
TermInput *input = data;
static const char *stty_erase = NULL; static const char *stty_erase = NULL;
if (stty_erase == NULL) { if (stty_erase == NULL) {
stty_erase = tui_get_stty_erase(); stty_erase = tui_get_stty_erase(input->in_fd);
} }
if (strequal(name, "key_backspace")) { if (strequal(name, "key_backspace")) {

View File

@@ -48,9 +48,16 @@ uint64_t ui_client_start_server(int argc, char **argv)
on_err, CALLBACK_NONE, on_err, CALLBACK_NONE,
false, true, true, false, kChannelStdinPipe, false, true, true, false, kChannelStdinPipe,
NULL, 0, 0, NULL, &exit_status); NULL, 0, 0, NULL, &exit_status);
// If stdin is not a pty, it is forwarded to the client.
// Replace stdin in the TUI process with the tty fd.
if (ui_client_forward_stdin) { if (ui_client_forward_stdin) {
close(0); close(0);
dup(2); #ifdef MSWIN
os_open_conin_fd();
#else
dup(stderr_isatty ? STDERR_FILENO : STDOUT_FILENO);
#endif
} }
return channel->id; return channel->id;