patch 9.1.2013: tests: Test_terminal_shell_option fails with conpty

Problem:  tests: When opening a conpty terminal, if process startup
          fails, it will silently exit.  As a result, the
          Test_terminal_shell_option in test_terminal3.vim failed in
          conpty.

          In a winpty terminal, the winpty-provided error message
          "CreateProcess failed" was displayed.  The test is designed to
          catch this error as an exception.

Solution: Make conpty fail with an error messages in the same way as winpty.
          (Muraoka Taro)

In addition, since the GetWin32Error() function can obtain more detailed
error messages, the format has been changed to "CreateProcess failed:
{localized message from the OS}" for conpty.

Also, since the GetWin32Error() function returns errors in ACP (Active
Code Page) encoding, these have been converted to Vim's internal
encoding, enc.  This will prevent messages from being garbled in
Japanese environments, etc.  The output of this function was basically
used by the semsg() function in other places, so this change also fixes
potential similar garbled characters.

The test now errors out immediately in places where it is expected not
to be reached, and comments have been added about the expected content
of the winpty and conpty error messages.

closes: #18998

Signed-off-by: Muraoka Taro <koron.kaoriya@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/os_win32.c b/src/os_win32.c
index ede7a15b299533383a845ccdbb67a7056c916299..8291f45b16922e015f6e738b877ac565a4d09d3a 100644 (file)
--- a/src/os_win32.c
+++ b/src/os_win32.c
@@ -9074,22 +9074,38 @@ resize_console_buf(void)
     char *
 GetWin32Error(void)
 {
-    static char *oldmsg = NULL;
-    char *msg = NULL;
+    static char        *oldmsg = NULL;
+    char       *acp_msg = NULL;
+    DWORD      acp_len;
+    char_u     *enc_msg = NULL;
+    int                enc_len = 0;
+
+    // get formatted message from OS
+    acp_len = FormatMessage(
+           FORMAT_MESSAGE_ALLOCATE_BUFFER|FORMAT_MESSAGE_FROM_SYSTEM,
+           NULL, GetLastError(), 0, (LPSTR)&acp_msg, 0, NULL);
+    if (acp_len == 0 || acp_msg == NULL)
+       return NULL;
 
-    FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER|FORMAT_MESSAGE_FROM_SYSTEM,
-           NULL, GetLastError(), 0, (LPSTR)&msg, 0, NULL);
+    // clean oldmsg if remained.
     if (oldmsg != NULL)
-       LocalFree(oldmsg);
-    if (msg == NULL)
+    {
+       vim_free(oldmsg);
+       oldmsg = NULL;
+    }
+
+    acp_to_enc(acp_msg, (int)acp_len, &enc_msg, &enc_len);
+    LocalFree(acp_msg);
+    if (enc_msg == NULL)
        return NULL;
 
     // remove trailing \r\n
-    char *pcrlf = strstr(msg, "\r\n");
+    char *pcrlf = strstr(enc_msg, "\r\n");
     if (pcrlf != NULL)
        *pcrlf = NUL;
-    oldmsg = msg;
-    return msg;
+
+    oldmsg = enc_msg;
+    return enc_msg;
 }
 
 #if defined(FEAT_RELTIME)

diff --git a/src/terminal.c b/src/terminal.c
index 48b9e06c406ba21d8cfbf2417cfddc0aa6978e37..0e7dc87ddb844790c819d602f52f99f859d64dd2 100644 (file)
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -7048,6 +7048,7 @@ conpty_term_and_job_init(
     HANDLE         o_theirs = NULL;
     HANDLE         i_ours = NULL;
     HANDLE         o_ours = NULL;
+    char           *errmsg = NULL;
 
     ga_init2(&ga_cmd, sizeof(char*), 20);
     ga_init2(&ga_env, sizeof(char*), 20);
@@ -7149,7 +7150,10 @@ conpty_term_and_job_init(
            | CREATE_SUSPENDED | CREATE_DEFAULT_ERROR_MODE,
            env_wchar, cwd_wchar,
            &term->tl_siex.StartupInfo, &proc_info))
+    {
+       errmsg = GetWin32Error();
        goto failed;
+    }
 
     CloseHandle(i_theirs);
     CloseHandle(o_theirs);
@@ -7257,6 +7261,9 @@ failed:
     if (term->tl_conpty != NULL)
        pClosePseudoConsole(term->tl_conpty);
     term->tl_conpty = NULL;
+    // Propagate errors that occur in CreateProcess
+    if (errmsg)
+       semsg("CreateProcess failed: %s", errmsg);
     return FAIL;
 }
 

diff --git a/src/testdir/test_terminal3.vim b/src/testdir/test_terminal3.vim
index 5049f913c809196525a512eff37be425d3c553a4..b332ff4e6dbcde3507e8593f7e089a7cda9e90aa 100644 (file)
--- a/src/testdir/test_terminal3.vim
+++ b/src/testdir/test_terminal3.vim
@@ -46,9 +46,11 @@ func Test_terminal_shell_option()
     " the %PATH%, "term dir" succeeds unintentionally.  Use dir.com instead.
     try
       term dir.com /b runtest.vim
-      call WaitForAssert({-> assert_match('job failed', term_getline(bufnr(), 1))})
-    catch /CreateProcess/
-      " ignore
+      throw 'dir.com without a shell must fail, so you will never get here'
+    catch /CreateProcess failed/
+      " ignore:
+      " winpty simply fails with "CreateProcess failed".
+      " compty fails with "CreateProcess failed: {localized failure reason}".
     endtry
     bwipe!
 

diff --git a/src/version.c b/src/version.c
index 01967fcec395dcf8f35cc020fcb3257b7d7cec19..9ab09606fd504cbcb22245b4afe324913b0c2eb3 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2013,
 /**/
     2012,
 /**/