win/pty: log errors

src/nvim/os/pty_process_win.c

@@ -2,7 +2,9 @@
 #include <stdbool.h>
 #include <stdlib.h>
 
-#include "nvim/vim.h"
+#include <winpty_constants.h>
+
+#include "nvim/os/os.h"
 #include "nvim/ascii.h"
 #include "nvim/memory.h"
 #include "nvim/mbyte.h"  // for utf8_to_utf16, utf16_to_utf8
@@ -23,7 +25,7 @@ static void CALLBACK pty_process_finish1(void *context, BOOLEAN unused)
   uv_timer_start(&ptyproc->wait_eof_timer, wait_eof_timer_cb, 200, 200);
 }
 
-/// @returns zero on sucess, or error code of winpty or MultiByteToWideChar.
+/// @returns zero on success, or negative error code.
 int pty_process_spawn(PtyProcess *ptyproc)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -40,27 +42,32 @@ int pty_process_spawn(PtyProcess *ptyproc)
   uv_connect_t *out_req = NULL;
   wchar_t *cmd_line = NULL;
   wchar_t *cwd = NULL;
+  const char *emsg = NULL;
 
   assert(!proc->err);
 
   cfg = winpty_config_new(WINPTY_FLAG_ALLOW_CURPROC_DESKTOP_CREATION, &err);
   if (cfg == NULL) {
+    emsg = "Failed, winpty_config_new.";
     goto cleanup;
   }
 
   winpty_config_set_initial_size(cfg, ptyproc->width, ptyproc->height);
   winpty_object = winpty_open(cfg, &err);
   if (winpty_object == NULL) {
+    emsg = "Failed, winpty_open.";
     goto cleanup;
   }
 
   status = utf16_to_utf8(winpty_conin_name(winpty_object), &in_name);
   if (status != 0) {
+    emsg = "Failed to convert in_name from utf16 to utf8.";
     goto cleanup;
   }
 
   status = utf16_to_utf8(winpty_conout_name(winpty_object), &out_name);
   if (status != 0) {
+    emsg = "Failed to convert out_name from utf16 to utf8.";
     goto cleanup;
   }
 
@@ -85,12 +92,14 @@ int pty_process_spawn(PtyProcess *ptyproc)
   if (proc->cwd != NULL) {
     status = utf8_to_utf16(proc->cwd, &cwd);
     if (status != 0) {
+      emsg = "Failed to convert pwd form utf8 to utf16.";
       goto cleanup;
     }
   }
 
   status = build_cmd_line(proc->argv, &cmd_line);
   if (status != 0) {
+    emsg = "Failed to convert cmd line form utf8 to utf16.";
     goto cleanup;
   }
 
@@ -102,15 +111,23 @@ int pty_process_spawn(PtyProcess *ptyproc)
       NULL,  // Optional environment variables
       &err);
   if (spawncfg == NULL) {
+    emsg = "Failed winpty_spawn_config_new.";
     goto cleanup;
   }
 
+  DWORD win_err = 0;
   if (!winpty_spawn(winpty_object,
                     spawncfg,
                     &process_handle,
                     NULL,  // Optional thread handle
-                    NULL,  // Optional create process error
+                    &win_err,
                     &err)) {
+    if (win_err) {
+      status = (int)win_err;
+      emsg = "Failed spawn process.";
+    } else {
+      emsg = "Failed winpty_spawn.";
+    }
     goto cleanup;
   }
   proc->pid = GetProcessId(process_handle);
@@ -137,6 +154,15 @@ int pty_process_spawn(PtyProcess *ptyproc)
   process_handle = NULL;
 
 cleanup:
+  if (status) {
+    // In the case of an error of MultiByteToWideChar or CreateProcessW.
+    ELOG("%s error code: %d", emsg, status);
+    status = os_translate_sys_error(status);
+  } else if (err != NULL) {
+    status = (int)winpty_error_code(err);
+    ELOG("%s error code: %d", emsg, status);
+    status = translate_winpty_error(status);
+  }
   winpty_error_free(err);
   winpty_config_free(cfg);
   winpty_spawn_config_free(spawncfg);
@@ -198,8 +224,7 @@ static void pty_process_connect_cb(uv_connect_t *req, int status)
 static void wait_eof_timer_cb(uv_timer_t *wait_eof_timer)
   FUNC_ATTR_NONNULL_ALL
 {
-  PtyProcess *ptyproc =
+  PtyProcess *ptyproc = wait_eof_timer->data;
-    (PtyProcess *)((uv_handle_t *)wait_eof_timer->data);
   Process *proc = (Process *)ptyproc;
 
   if (!proc->out || !uv_is_readable(proc->out->uvstream)) {
@@ -229,9 +254,9 @@ static void pty_process_finish2(PtyProcess *ptyproc)
 /// Build the command line to pass to CreateProcessW.
 ///
 /// @param[in]  argv  Array with string arguments.
-/// @param[out]  cmd_line  Location where saved bulded cmd line.
+/// @param[out]  cmd_line  Location where saved builded cmd line.
 ///
-/// @returns zero on sucess, or error code of MultiByteToWideChar function.
+/// @returns zero on success, or error code of MultiByteToWideChar function.
 ///
 static int build_cmd_line(char **argv, wchar_t **cmd_line)
   FUNC_ATTR_NONNULL_ALL
@@ -271,8 +296,6 @@ static int build_cmd_line(char **argv, wchar_t **cmd_line)
   }
 
   int result = utf8_to_utf16(utf8_cmd_line, cmd_line);
-  if (result != 0) {
-  }
   xfree(utf8_cmd_line);
   return result;
 }
@@ -280,33 +303,33 @@ static int build_cmd_line(char **argv, wchar_t **cmd_line)
 /// Emulate quote_cmd_arg of libuv and quotes command line argument.
 /// Most of the code came from libuv.
 ///
-/// @param[out]  dist  Location where saved quotes argument.
+/// @param[out]  dest  Location where saved quotes argument.
-/// @param  dist_remaining  Deistnation buffer size.
+/// @param  dest_remaining  Destination buffer size.
 /// @param[in]  src Pointer to argument.
 ///
-static void quote_cmd_arg(char *dist, size_t dist_remaining, const char *src)
+static void quote_cmd_arg(char *dest, size_t dest_remaining, const char *src)
   FUNC_ATTR_NONNULL_ALL
 {
   size_t src_len = strlen(src);
   bool quote_hit = true;
-  char *start = dist;
+  char *start = dest;
 
   if (src_len == 0) {
     // Need double quotation for empty argument.
-    snprintf(dist, dist_remaining, "\"\"");
+    snprintf(dest, dest_remaining, "\"\"");
     return;
   }
 
   if (NULL == strpbrk(src, " \t\"")) {
     // No quotation needed.
-    xstrlcpy(dist, src, dist_remaining);
+    xstrlcpy(dest, src, dest_remaining);
     return;
   }
 
   if (NULL == strpbrk(src, "\"\\")) {
     // No embedded double quotes or backlashes, so I can just wrap quote marks.
     // around the whole thing.
-    snprintf(dist, dist_remaining, "\"%s\"", src);
+    snprintf(dest, dest_remaining, "\"%s\"", src);
     return;
   }
 
@@ -326,32 +349,57 @@ static void quote_cmd_arg(char *dist, size_t dist_remaining, const char *src)
   //   input : hello world\
   //   output: "hello world\\"
 
-  assert(dist_remaining--);
+  assert(dest_remaining--);
-  *(dist++) = NUL;
+  *(dest++) = NUL;
-  assert(dist_remaining--);
+  assert(dest_remaining--);
-  *(dist++) = '"';
+  *(dest++) = '"';
   for (size_t i = src_len; i > 0; i--) {
-    assert(dist_remaining--);
+    assert(dest_remaining--);
-    *(dist++) = src[i - 1];
+    *(dest++) = src[i - 1];
     if (quote_hit && src[i - 1] == '\\') {
-      assert(dist_remaining--);
+      assert(dest_remaining--);
-      *(dist++) = '\\';
+      *(dest++) = '\\';
     } else if (src[i - 1] == '"') {
       quote_hit = true;
-      assert(dist_remaining--);
+      assert(dest_remaining--);
-      *(dist++) = '\\';
+      *(dest++) = '\\';
     } else {
       quote_hit = false;
     }
   }
-  assert(dist_remaining);
+  assert(dest_remaining);
-  *dist = '"';
+  *dest = '"';
 
-  while (start < dist) {
+  while (start < dest) {
     char tmp = *start;
-    *start = *dist;
+    *start = *dest;
-    *dist = tmp;
+    *dest = tmp;
     start++;
-    dist--;
+    dest--;
+  }
+}
+
+/// Translate winpty error code to libuv error.
+///
+/// @param[in]  winpty_errno  Winpty error code returned by winpty_error_code
+///                           function.
+///
+/// @returns  Error code of libuv error.
+int translate_winpty_error(int winpty_errno)
+{
+  if (winpty_errno <= 0) {
+    return winpty_errno;  // If < 0 then it's already a libuv error.
+  }
+
+  switch (winpty_errno) {
+    case WINPTY_ERROR_OUT_OF_MEMORY:                return UV_ENOMEM;
+    case WINPTY_ERROR_SPAWN_CREATE_PROCESS_FAILED:  return UV_EAI_FAIL;
+    case WINPTY_ERROR_LOST_CONNECTION:              return UV_ENOTCONN;
+    case WINPTY_ERROR_AGENT_EXE_MISSING:            return UV_ENOENT;
+    case WINPTY_ERROR_UNSPECIFIED:                   return UV_UNKNOWN;
+    case WINPTY_ERROR_AGENT_DIED:                   return UV_ESRCH;
+    case WINPTY_ERROR_AGENT_TIMEOUT:                return UV_ETIMEDOUT;
+    case WINPTY_ERROR_AGENT_CREATION_FAILED:        return UV_EAI_FAIL;
+    default:                                        return UV_UNKNOWN;
   }
 }

src/nvim/os/pty_process_win.h

@@ -2,7 +2,6 @@
 #define NVIM_OS_PTY_PROCESS_WIN_H
 
 #include <uv.h>
-
 #include <winpty.h>
 
 #include "nvim/event/process.h"

test/functional/fixtures/tty-test.c

@@ -4,15 +4,34 @@
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
+
 #include <uv.h>
+#ifdef _WIN32
+#include <windows.h>
+#endif
 
 // -V:STRUCT_CAST:641
 #define STRUCT_CAST(Type, obj) ((Type *)(obj))
-
+#define is_terminal(stream) (uv_guess_handle(fileno(stream)) == UV_TTY)
-uv_tty_t tty;
+#define BUF_SIZE 0xfff
+#define CTRL_C 0x03
+#ifdef _WIN32
+#define CTRL_Q 0x11
+#endif
+
+#ifdef _WIN32
+typedef struct screen_size {
+  int width;
+  int height;
+} ScreenSize;
+#endif
+
+uv_tty_t tty;
+#ifdef _WIN32
+ScreenSize screen_rect;
+#endif
 
 #ifdef _WIN32
-#include <windows.h>
 bool owns_tty(void)
 {
   // XXX: We need to make proper detect owns tty
@@ -30,10 +49,8 @@ bool owns_tty(void)
 }
 #endif
 
-#define is_terminal(stream) (uv_guess_handle(fileno(stream)) == UV_TTY)
+static void walk_cb(uv_handle_t *handle, void *arg)
-#define BUF_SIZE 0xfff
+{
-
-static void walk_cb(uv_handle_t *handle, void *arg) {
   if (!uv_is_closing(handle)) {
     uv_close(handle, NULL);
   }
@@ -42,7 +59,7 @@ static void walk_cb(uv_handle_t *handle, void *arg) {
 #ifndef WIN32
 static void sig_handler(int signum)
 {
-  switch(signum) {
+  switch (signum) {
   case SIGWINCH: {
     int width, height;
     uv_tty_get_winsize(&tty, &width, &height);
@@ -82,14 +99,19 @@ static void read_cb(uv_stream_t *stream, ssize_t cnt, const uv_buf_t *buf)
   }
 
   int *interrupted = stream->data;
+#ifdef _WIN32
   bool prsz = false;
-  int width, height;
+  int width;
+  int height;
+#endif
 
   for (int i = 0; i < cnt; i++) {
-    if (buf->base[i] == 3) {
+    if (buf->base[i] == CTRL_C) {
       (*interrupted)++;
-    } else if (buf->base[i] == 17) {
+#ifdef _WIN32
+    } else if (buf->base[i] == CTRL_Q) {
       prsz = true;
+#endif
     }
   }
 
@@ -98,15 +120,23 @@ static void read_cb(uv_stream_t *stream, ssize_t cnt, const uv_buf_t *buf)
   uv_tty_t out;
   uv_tty_init(&write_loop, &out, fileno(stdout), 0);
 
+#ifdef _WIN32
   if (prsz) {
     uv_tty_get_winsize(&out, &width, &height);
-    fprintf(stderr, "rows: %d, cols: %d\n", height, width);
+    if (screen_rect.width != width || screen_rect.height != height) {
+      screen_rect.width = width;
+      screen_rect.height = height;
+      fprintf(stderr, "rows: %d, cols: %d\n", height, width);
+    }
   } else {
+#endif
     uv_write_t req;
     uv_buf_t b = {.base = buf->base, .len = (size_t)cnt};
     uv_write(&req, STRUCT_CAST(uv_stream_t, &out), &b, 1, NULL);
     uv_run(&write_loop, UV_RUN_DEFAULT);
+#ifdef _WIN32
   }
+#endif
 
   uv_close(STRUCT_CAST(uv_handle_t, &out), NULL);
   uv_run(&write_loop, UV_RUN_DEFAULT);
@@ -152,7 +182,7 @@ int main(int argc, char **argv)
 
   if (argc > 1) {
     int count = atoi(argv[1]);
-    for (int i = 0; i < count; ++i) {
+    for (int i = 0; i < count; i++) {
       printf("line%d\n", i);
     }
     fflush(stdout);
@@ -168,6 +198,14 @@ int main(int argc, char **argv)
   uv_tty_init(uv_default_loop(), &tty, fileno(stderr), 1);
 #else
   uv_tty_init(uv_default_loop(), &tty, fileno(stdin), 1);
+  uv_tty_t out;
+  uv_tty_init(uv_default_loop(), &out, fileno(stdout), 0);
+  int width;
+  int height;
+  uv_tty_get_winsize(&out, &width, &height);
+  screen_rect.width = width;
+  screen_rect.height = height;
+  uv_close((uv_handle_t *)&out, NULL);
 #endif
   uv_tty_set_mode(&tty, UV_TTY_MODE_RAW);
   tty.data = &interrupted;

test/functional/terminal/helpers.lua

@@ -30,10 +30,6 @@ local function clear_attrs() feed_termcode('[0;10m') end
 -- mouse
 local function enable_mouse() feed_termcode('[?1002h') end
 local function disable_mouse() feed_termcode('[?1002l') end
-local function print_screen_size()
-  helpers.sleep(1000)
-  nvim('command', 'call jobsend(b:terminal_job_id, "\\<C-q>")')
-end
 
 local default_command = '["'..nvim_dir..'/tty-test'..'"]'
 
@@ -115,6 +111,5 @@ return {
   clear_attrs = clear_attrs,
   enable_mouse = enable_mouse,
   disable_mouse = disable_mouse,
-  print_screen_size = print_screen_size,
   screen_setup = screen_setup
 }

test/functional/terminal/scrollback_spec.lua

@@ -3,7 +3,7 @@ local helpers = require('test.functional.helpers')(after_each)
 local thelpers = require('test.functional.terminal.helpers')
 local clear, eq, curbuf = helpers.clear, helpers.eq, helpers.curbuf
 local feed, nvim_dir, feed_command = helpers.feed, helpers.nvim_dir, helpers.feed_command
-local iswin, print_screen_size = helpers.iswin, thelpers.print_screen_size
+local iswin = helpers.iswin
 local eval = helpers.eval
 local command = helpers.command
 local wait = helpers.wait
@@ -141,10 +141,7 @@ describe('terminal scrollback', function()
     describe('and the height is decreased by 1', function()
       local function will_hide_top_line()
         screen:try_resize(screen._width, screen._height - 1)
-        if iswin() then
+        screen:expect_after_resize([[
-          print_screen_size()
-        end
-        screen:expect([[
           line2                         |
           line3                         |
           line4                         |
@@ -160,13 +157,10 @@ describe('terminal scrollback', function()
         before_each(function()
           will_hide_top_line()
           screen:try_resize(screen._width, screen._height - 2)
-          if iswin() then
-            print_screen_size()
-          end
         end)
 
         it('will hide the top 3 lines', function()
-          screen:expect([[
+          screen:expect_after_resize([[
             rows: 5, cols: 30             |
             rows: 3, cols: 30             |
             {1: }                             |
@@ -189,9 +183,6 @@ describe('terminal scrollback', function()
     describe('and the height is decreased by 2', function()
       before_each(function()
         screen:try_resize(screen._width, screen._height - 2)
-        if iswin() then
-          print_screen_size()
-        end
       end)
 
       local function will_delete_last_two_lines()
@@ -213,9 +204,6 @@ describe('terminal scrollback', function()
         before_each(function()
           will_delete_last_two_lines()
           screen:try_resize(screen._width, screen._height - 1)
-          if iswin() then
-            print_screen_size()
-          end
         end)
 
         it('will delete the last line and hide the first', function()
@@ -258,10 +246,7 @@ describe('terminal scrollback', function()
         {3:-- TERMINAL --}                |
       ]])
       screen:try_resize(screen._width, screen._height - 3)
-      if iswin() then
+      screen:expect_after_resize([[
-        print_screen_size()
-      end
-      screen:expect([[
         line4                         |
         rows: 3, cols: 30             |
         {1: }                             |
@@ -273,10 +258,7 @@ describe('terminal scrollback', function()
     describe('and the height is increased by 1', function()
       local function pop_then_push()
         screen:try_resize(screen._width, screen._height + 1)
-        if iswin() then
+        screen:expect_after_resize([[
-          print_screen_size()
-        end
-        screen:expect([[
           line4                         |
           rows: 3, cols: 30             |
           rows: 4, cols: 30             |
@@ -292,13 +274,10 @@ describe('terminal scrollback', function()
           pop_then_push()
           eq(8, curbuf('line_count'))
           screen:try_resize(screen._width, screen._height + 3)
-          if iswin() then
-            print_screen_size()
-          end
         end)
 
         local function pop3_then_push1()
-          screen:expect([[
+          screen:expect_after_resize([[
             line2                         |
             line3                         |
             line4                         |
@@ -330,9 +309,6 @@ describe('terminal scrollback', function()
             pop3_then_push1()
             feed('Gi')
             screen:try_resize(screen._width, screen._height + 4)
-            if iswin() then
-              print_screen_size()
-            end
           end)
 
           it('will show all lines and leave a blank one at the end', function()

test/functional/ui/screen.lua

@@ -73,6 +73,8 @@
 
 local helpers = require('test.functional.helpers')(nil)
 local request, run, uimeths = helpers.request, helpers.run, helpers.uimeths
+local iswin, nvim, retry = helpers.iswin, helpers.nvim, helpers.retry
+
 local dedent = helpers.dedent
 
 local Screen = {}
@@ -259,6 +261,17 @@ screen:redraw_debug() to show all intermediate screen states.  ]])
   end)
 end
 
+function Screen:expect_after_resize(expected)
+  if iswin() then
+    retry(nil, nil, function()
+      nvim('command', 'call jobsend(b:terminal_job_id, "\\<C-q>")')
+      self:expect(expected)
+    end)
+  else
+    self:expect(expected)
+  end
+end
+
 function Screen:wait(check, timeout)
   local err, checked = false
   local success_seen = false