Merge pull request #21831 from bfredl/nofd

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

10 files changed, 49 insertions, 63 deletions

diff --git a/src/nvim/globals.h b/src/nvim/globals.h
index aa8dbf6331..0d3862a1ec 100644
--- a/src/nvim/globals.h
+++ b/src/nvim/globals.h
@@ -494,6 +494,9 @@ EXTERN int v_dying INIT(= 0);
 EXTERN bool stdin_isatty INIT(= true);
 // is stdout a terminal?
 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 -`
 EXTERN int stdin_fd INIT(= -1);
 
diff --git a/src/nvim/main.c b/src/nvim/main.c
index f00d09b748..4dfb00f2db 100644
--- a/src/nvim/main.c
+++ b/src/nvim/main.c
@@ -286,7 +286,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.
   // This will be done when nvim server has been forked from the ui process
@@ -302,7 +308,7 @@ int main(int argc, char **argv)
   bool remote_ui = (ui_client_channel_id != 0);
 
   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);
     if (!rv) {
       os_errmsg("Failed to start Nvim server!\n");
@@ -359,8 +365,8 @@ int main(int argc, char **argv)
   debug_break_level = params.use_debug_break_level;
 
   // Read ex-commands if invoked with "-es".
-  if (!params.input_isatty && !params.input_istext && silent_mode && exmode_active) {
-    input_start(STDIN_FILENO);
+  if (!stdin_isatty && !params.input_istext && silent_mode && exmode_active) {
+    input_start();
   }
 
   if (ui_client_channel_id) {
@@ -639,8 +645,8 @@ void os_exit(int r)
   if (!event_teardown() && r == 0) {
     r = 1;  // Exit with error if main_loop did not teardown gracefully.
   }
-  if (input_global_fd() >= 0) {
-    stream_set_blocking(input_global_fd(), true);  // normalize stream (#2598)
+  if (used_stdin) {
+    stream_set_blocking(STDIN_FILENO, true);  // normalize stream (#2598)
   }
 
   ILOG("Nvim exit: %d", r);
@@ -785,9 +791,9 @@ void preserve_exit(void)
 
   // Prevent repeated calls into this method.
   if (really_exiting) {
-    if (input_global_fd() >= 0) {
+    if (used_stdin) {
       // normalize stream (#2598)
-      stream_set_blocking(input_global_fd(), true);
+      stream_set_blocking(STDIN_FILENO, true);
     }
     exit(2);
   }
@@ -963,7 +969,7 @@ static bool edit_stdin(mparm_T *parmp)
   bool implicit = !headless_mode
                   && !(embedded_mode && stdin_fd <= 0)
                   && (!exmode_active || parmp->input_istext)
-                  && !parmp->input_isatty
+                  && !stdin_isatty
                   && parmp->scriptin == NULL;  // `-s -` was not given.
   return parmp->had_stdin_file || implicit;
 }
@@ -1449,11 +1455,9 @@ static void init_startuptime(mparm_T *paramp)
 
 static void check_and_set_isatty(mparm_T *paramp)
 {
-  stdin_isatty
-    = paramp->input_isatty = os_isatty(STDIN_FILENO);
-  stdout_isatty
-    = paramp->output_isatty = os_isatty(STDOUT_FILENO);
-  paramp->err_isatty = os_isatty(STDERR_FILENO);
+  stdin_isatty = os_isatty(STDIN_FILENO);
+  stdout_isatty = os_isatty(STDOUT_FILENO);
+  stderr_isatty = os_isatty(STDERR_FILENO);
   TIME_MSG("window checked");
 }
 
diff --git a/src/nvim/main.h b/src/nvim/main.h
index 46d7217364..2d54837872 100644
--- a/src/nvim/main.h
+++ b/src/nvim/main.h
@@ -30,10 +30,7 @@ typedef struct {
   char *tagname;                        // tag from -t 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 output_isatty;                   // stdout is a terminal
-  bool err_isatty;                      // stderr is a terminal
 
   int no_swap_file;                     // "-n" argument used
   int use_debug_break_level;
diff --git a/src/nvim/os/input.c b/src/nvim/os/input.c
index 5d2ac1e102..51cabfbcbf 100644
--- a/src/nvim/os/input.c
+++ b/src/nvim/os/input.c
@@ -44,7 +44,6 @@ typedef enum {
 static Stream read_stream = { .closed = true };  // Input before UI starts.
 static RBuffer *input_buffer = NULL;
 static bool input_eof = false;
-static int global_fd = -1;
 static bool blocking = false;
 static int cursorhold_time = 0;  ///< time waiting for CursorHold event
 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);
 }
 
-void input_global_fd_init(int fd)
-{
-  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)
+void input_start(void)
 {
   if (!read_stream.closed) {
     return;
   }
 
-  input_global_fd_init(fd);
-  rstream_init_fd(&main_loop, &read_stream, fd, READ_BUFFER_SIZE);
+  used_stdin = true;
+  rstream_init_fd(&main_loop, &read_stream, STDIN_FILENO, READ_BUFFER_SIZE);
   rstream_start(&read_stream, input_read_cb, NULL);
 }
 
diff --git a/src/nvim/os/input.h b/src/nvim/os/input.h
index 7026781407..6f25efdc7b 100644
--- a/src/nvim/os/input.h
+++ b/src/nvim/os/input.h
@@ -7,6 +7,8 @@
 #include "nvim/api/private/defs.h"
 #include "nvim/event/multiqueue.h"
 
+EXTERN bool used_stdin INIT(= false);
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "os/input.h.generated.h"
 #endif
diff --git a/src/nvim/os/os_win_console.c b/src/nvim/os/os_win_console.c
index ec0f03a1dc..18f6e8b37b 100644
--- a/src/nvim/os/os_win_console.c
+++ b/src/nvim/os/os_win_console.c
@@ -14,7 +14,7 @@ static HWND hWnd = NULL;
 static HICON hOrigIconSmall = NULL;
 static HICON hOrigIcon = NULL;
 
-int os_get_conin_fd(void)
+int os_open_conin_fd(void)
 {
   const HANDLE conin_handle = CreateFile("CONIN$",
                                          GENERIC_READ | GENERIC_WRITE,
@@ -30,7 +30,7 @@ int os_get_conin_fd(void)
 void os_replace_stdin_to_conin(void)
 {
   close(STDIN_FILENO);
-  const int conin_fd = os_get_conin_fd();
+  const int conin_fd = os_open_conin_fd();
   assert(conin_fd == STDIN_FILENO);
 }
 
diff --git a/src/nvim/testdir/test_startup.vim b/src/nvim/testdir/test_startup.vim
index b294d62ab4..1ee1d0dfe3 100644
--- a/src/nvim/testdir/test_startup.vim
+++ b/src/nvim/testdir/test_startup.vim
@@ -1043,7 +1043,7 @@ func Test_io_not_a_terminal()
         \ 'Vim: Warning: Input is not from a terminal'], l)
 endfunc
 
-" Test for --not-a-term avoiding escape codes.
+" Test for not being a term avoiding escape codes.
 func Test_not_a_term()
   CheckUnix
   CheckNotGui
@@ -1054,18 +1054,14 @@ func Test_not_a_term()
     let redir = &shellredir .. ' Xvimout'
   endif
 
-  " Without --not-a-term there are a few escape sequences.
-  " This will take 2 seconds because of the missing --not-a-term
+  " As nvim checks the environment by itself there will be no escape sequences
+  " This will also happen to take two (2) seconds.
   let cmd = GetVimProg() .. ' --cmd quit ' .. redir
   exe "silent !" . cmd
-  call assert_match("\<Esc>", readfile('Xvimout')->join())
+  call assert_notmatch("\e", readfile('Xvimout')->join())
   call delete('Xvimout')
 
-  " With --not-a-term there are no escape sequences.
-  let cmd = GetVimProg() .. ' --not-a-term --cmd quit ' .. redir
-  exe "silent !" . cmd
-  call assert_notmatch("\<Esc>", readfile('Xvimout')->join())
-  call delete('Xvimout')
+  " --not-a-term flag has thus been deleted
 endfunc
 
 
diff --git a/src/nvim/tui/input.c b/src/nvim/tui/input.c
index 55a8b3666e..5325ae3e4d 100644
--- a/src/nvim/tui/input.c
+++ b/src/nvim/tui/input.c
@@ -149,19 +149,7 @@ void tinput_init(TermInput *input, Loop *loop)
                               kitty_key_map_entry[i].name);
   }
 
-  // If stdin is not a pty, switch to stderr. For cases like:
-  //    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);
+  input->in_fd = STDIN_FILENO;
 
   const char *term = os_getenv("TERM");
   if (!term) {
@@ -170,7 +158,7 @@ void tinput_init(TermInput *input, Loop *loop)
 
   input->tk = termkey_new_abstract(term,
                                    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);
 
   int curflags = termkey_get_canonflags(input->tk);
diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c
index 5232bcad19..44b99f6c84 100644
--- a/src/nvim/tui/tui.c
+++ b/src/nvim/tui/tui.c
@@ -1338,7 +1338,7 @@ static void suspend_event(void **argv)
   TUIData *tui = argv[0];
   bool enable_mouse = tui->mouse_enabled;
   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);
 
@@ -1347,7 +1347,7 @@ static void suspend_event(void **argv)
   if (enable_mouse) {
     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
 
@@ -2234,12 +2234,12 @@ static void flush_buf(TUIData *tui)
 ///
 /// @see tmux/tty-keys.c fe4e9470bb504357d073320f5d305b22663ee3fd
 /// @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 };
 #if defined(HAVE_TERMIOS_H)
   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[1] = '\0';
     DLOG("stty/termios:erase=%s", stty_erase);
@@ -2252,9 +2252,10 @@ static const char *tui_get_stty_erase(void)
 /// @see TermInput.tk_ti_hook_fn
 static const char *tui_tk_ti_getstr(const char *name, const char *value, void *data)
 {
+  TermInput *input = data;
   static const char *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")) {
diff --git a/src/nvim/ui_client.c b/src/nvim/ui_client.c
index b80b250cec..222ba3d5dd 100644
--- a/src/nvim/ui_client.c
+++ b/src/nvim/ui_client.c
@@ -51,9 +51,16 @@ uint64_t ui_client_start_server(int argc, char **argv)
                                        on_err, CALLBACK_NONE,
                                        false, true, true, false, kChannelStdinPipe,
                                        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) {
     close(0);
-    dup(2);
+#ifdef MSWIN
+    os_open_conin_fd();
+#else
+    dup(stderr_isatty ? STDERR_FILENO : STDOUT_FILENO);
+#endif
   }
 
   return channel->id;