win: os_shell_is_cmdexe() + tests

runtime/doc/eval.txt

@@ -4855,14 +4855,14 @@ jobstart({cmd}[, {opts}])				{Nvim} *jobstart()*
 <		(Only shows the idea; see |shell-unquoting| for full details.)
 
 		NOTE: on Windows if {cmd} is a List:
-		  - cmd[0] must be executable. If it is in $PATH it can be
-		    called by name, with or without an extension: >
+		  - cmd[0] must be an executable (not a "built-in"). If it is
+		    in $PATH it can be called by name, without an extension: >
 		      :call jobstart(['ping', 'neovim.io'])
-<		    If it is a path (not a name), extension is required: >
+<		    If it is a full or partial path, extension is required: >
 		      :call jobstart(['System32\ping.exe', 'neovim.io'])
-<		  - {cmd} is quoted per the convention expected by
-		    CommandLineToArgvW https://msdn.microsoft.com/bb776391
-		    unless the first argument is some form of "cmd.exe".
+<		  - {cmd} is collapsed to a string of quoted args as expected
+		    by CommandLineToArgvW https://msdn.microsoft.com/bb776391
+		    unless cmd[0] is some form of "cmd.exe".
 
 		{opts} is a dictionary with these keys:
 		  on_stdout: stdout event handler (function name or |Funcref|)

src/nvim/event/libuv_process.c

@@ -8,7 +8,6 @@
 #include "nvim/event/process.h"
 #include "nvim/event/libuv_process.h"
 #include "nvim/log.h"
-#include "nvim/path.h"
 #include "nvim/os/os.h"
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
@@ -27,22 +26,9 @@ int libuv_process_spawn(LibuvProcess *uvproc)
       uvproc->uvopts.flags |= UV_PROCESS_DETACHED;
   }
 #ifdef WIN32
-  // libuv assumes spawned processes follow the convention from
-  // CommandLineToArgvW(), cmd.exe does not. Disable quoting since it will
-  // result in unexpected behaviour, the caller is left with the responsibility
-  // to quote arguments accordingly. system('') has shell* options for this.
-  //
-  // Disable quoting for cmd, cmd.exe and $COMSPEC with a cmd.exe filename
-  bool is_cmd = STRICMP(proc->argv[0], "cmd.exe") == 0
-                || STRICMP(proc->argv[0], "cmd") == 0;
-  if (!is_cmd) {
-    const char_u *comspec = (char_u *)os_getenv("COMSPEC");
-    const char_u *comspecshell = path_tail((char_u *)proc->argv[0]);
-    is_cmd = comspec != NULL && STRICMP(proc->argv[0], comspec) == 0
-             && STRICMP("cmd.exe", (char *)comspecshell) == 0;
-  }
-
-  if (is_cmd) {
+  // libuv collapses the argv to a CommandLineToArgvW()-style string. cmd.exe
+  // expects a different syntax (must be prepared by the caller before now).
+  if (os_shell_is_cmdexe(proc->argv[0])) {
     uvproc->uvopts.flags |= UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS;
   }
 #endif

src/nvim/memory.c

@@ -495,6 +495,13 @@ bool strequal(const char *a, const char *b)
   return (a == NULL && b == NULL) || (a && b && strcmp(a, b) == 0);
 }
 
+/// Case-insensitive `strequal`.
+bool striequal(const char *a, const char *b)
+  FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT
+{
+  return (a == NULL && b == NULL) || (a && b && STRICMP(a, b) == 0);
+}
+
 /*
  * Avoid repeating the error message many times (they take 1 second each).
  * Did_outofmem_msg is reset when a character is read.

src/nvim/os/env.c

@@ -1,11 +1,8 @@
-// env.c -- environment variable access
+// Environment inspection
 
 #include <assert.h>
-
 #include <uv.h>
 
-// vim.h must be included before charset.h (and possibly others) or things
-// blow up
 #include "nvim/vim.h"
 #include "nvim/ascii.h"
 #include "nvim/charset.h"
@@ -919,3 +916,20 @@ bool os_term_is_nice(void)
     || NULL != os_getenv("KONSOLE_DBUS_SESSION");
 #endif
 }
+
+/// Returns true if `sh` looks like it resolves to "cmd.exe".
+bool os_shell_is_cmdexe(const char *sh)
+  FUNC_ATTR_NONNULL_ALL
+{
+  if (*sh == NUL) {
+    return false;
+  }
+  if (striequal(sh, "$COMSPEC")) {
+    const char *comspec = os_getenv("COMSPEC");
+    return striequal("cmd.exe", (char *)path_tail((char_u *)comspec));
+  }
+  if (striequal(sh, "cmd.exe") || striequal(sh, "cmd")) {
+    return true;
+  }
+  return striequal("cmd.exe", (char *)path_tail((char_u *)sh));
+}

test/unit/os/env_spec.lua

@@ -67,12 +67,37 @@ describe('env function', function()
     end)
   end)
 
+  describe('os_shell_is_cmdexe', function()
+    itp('returns true for expected names', function()
+      eq(true, cimp.os_shell_is_cmdexe(to_cstr('cmd.exe')))
+      eq(true, cimp.os_shell_is_cmdexe(to_cstr('cmd')))
+      eq(true, cimp.os_shell_is_cmdexe(to_cstr('CMD.EXE')))
+      eq(true, cimp.os_shell_is_cmdexe(to_cstr('CMD')))
+
+      os_setenv('COMSPEC', '/foo/bar/cmd.exe', 0)
+      eq(true, cimp.os_shell_is_cmdexe(to_cstr('$COMSPEC')))
+      os_setenv('COMSPEC', [[C:\system32\cmd.exe]], 0)
+      eq(true, cimp.os_shell_is_cmdexe(to_cstr('$COMSPEC')))
+    end)
+    itp('returns false for unexpected names', function()
+      eq(false, cimp.os_shell_is_cmdexe(to_cstr('')))
+      eq(false, cimp.os_shell_is_cmdexe(to_cstr('powershell')))
+      eq(false, cimp.os_shell_is_cmdexe(to_cstr(' cmd.exe ')))
+      eq(false, cimp.os_shell_is_cmdexe(to_cstr('cm')))
+      eq(false, cimp.os_shell_is_cmdexe(to_cstr('md')))
+      eq(false, cimp.os_shell_is_cmdexe(to_cstr('cmd.ex')))
+
+      os_setenv('COMSPEC', '/foo/bar/cmd', 0)
+      eq(false, cimp.os_shell_is_cmdexe(to_cstr('$COMSPEC')))
+    end)
+  end)
+
   describe('os_getenv', function()
     itp('reads an env variable', function()
       local name = 'NVIM_UNIT_TEST_GETENV_1N'
       local value = 'NVIM_UNIT_TEST_GETENV_1V'
       eq(NULL, os_getenv(name))
-      -- need to use os_setenv, because lua dosn't have a setenv function
+      -- Use os_setenv because Lua dosen't have setenv.
       os_setenv(name, value, 1)
       eq(value, os_getenv(name))
     end)