win: default shellxescape, shellxquote to empty

Calling cmd.exe in Windows follows a very different pattern from Vim.
The primary difference is that Vim does a nested call to cmd.exe, e.g.
the following call in Vim

    system('echo a 2>&1')

spawns the following processes

    "C:\Program Files (x86)\Vim\vim80\vimrun" -s C:\Windows\system32\cmd.exe /c (echo a 2^>^&1
        ^>C:\Users\dummy\AppData\Local\Temp\VIoC169.tmp 2^>^&1)
    C:\Windows\system32\cmd.exe /c C:\Windows\system32\cmd.exe /c (echo a 2^>^&1
        ^>C:\Users\dummy\AppData\Local\Temp\VIo3C6C.tmp 2^>^&1)
    C:\Windows\system32\cmd.exe  /c (echo a 2>&1
        >C:\Users\dummy\AppData\Local\Temp\VIo3C6C.tmp 2>&1)

The escaping with ^ is needed because cmd.exe calls itself and needs to
preserve the special metacharacters for the last call. However in nvim
no nested call is made, system('') spawns a single cmd.exe process.
Setting shellxescape to "" disables escaping with ^.

The previous default for shellxquote=( wrapped any command in
parenthesis, in Vim this is more meaningful due to the use of tempfiles
to store the output and redirection (also see &shellquote). There is
a slight benefit in having the default be empty because some expressions
that run in console will not run within parens e.g. due to unbalanced
double quotes

    system('echo "a b')

runtime/doc/options.txt

@@ -5365,18 +5365,14 @@ A jump table for the options with a short description can be found at |Q_op|.
 	|system()| does not respect this option, it always uses pipes.
 
 						*'shellxescape'* *'sxe'*
-'shellxescape' 'sxe'	string	(default: ""; Windows: "\"&|<>()@^")
+'shellxescape' 'sxe'	string	(default: "")
 			global
 	When 'shellxquote' is set to "(" then the characters listed in this
 	option will be escaped with a '^' character.  This makes it possible
 	to execute most external commands with cmd.exe.
 
 						*'shellxquote'* *'sxq'*
-'shellxquote' 'sxq'	string	(default: "";
+'shellxquote' 'sxq'	string	(default: "")
-					for Win32, when 'shell' is cmd.exe: "("
-					for Win32, when 'shell' contains "sh"
-					somewhere: "\""
-					for Unix, when using system(): "\"")
 			global
 	Quoting character(s), put around the command passed to the shell, for
 	the "!" and ":!" commands.  Includes the redirection.  See
@@ -5385,12 +5381,6 @@ A jump table for the options with a short description can be found at |Q_op|.
 	When the value is '(' then ')' is appended. When the value is '"('
 	then ')"' is appended.
 	When the value is '(' then also see 'shellxescape'.
-	This is an empty string by default on most systems, but is known to be
-	useful for on Win32 version, either for cmd.exe which automatically
-	strips off the first and last quote on a command, or 3rd-party shells
-	such as the MKS Korn Shell or bash, where it should be "\"".  The
-	default is adjusted according the value of 'shell', to reduce the need
-	to set this option by the user.
 	This option cannot be set from a |modeline| or in the |sandbox|, for
 	security reasons.
 

src/nvim/options.lua

@@ -2077,11 +2077,7 @@ return {
       secure=true,
       vi_def=true,
       varname='p_sxq',
-      defaults={
+      defaults={if_true={vi=""}}
-        condition='WIN32',
-        if_true={vi="("},
-        if_false={vi=""}
-      }
     },
     {
       full_name='shellxescape', abbreviation='sxe',
@@ -2089,11 +2085,7 @@ return {
       secure=true,
       vi_def=true,
       varname='p_sxe',
-      defaults={
+      defaults={if_true={vi=""}}
-        condition='WIN32',
-        if_true={vi='"&|<>()@^'},
-        if_false={vi=""}
-      }
     },
     {
       full_name='shiftround', abbreviation='sr',

test/functional/eval/system_spec.lua

@@ -4,6 +4,8 @@ local nvim_dir = helpers.nvim_dir
 local eq, call, clear, eval, feed_command, feed, nvim =
   helpers.eq, helpers.call, helpers.clear, helpers.eval, helpers.feed_command,
   helpers.feed, helpers.nvim
+local command = helpers.command
+local iswin = helpers.iswin
 
 local Screen = require('test.functional.ui.screen')
 
@@ -33,8 +35,7 @@ describe('system()', function()
 
   describe('command passed as a List', function()
     local function printargs_path()
-      return nvim_dir..'/printargs-test'
+      return nvim_dir..'/printargs-test' .. (iswin() and '.exe' or '')
-        .. (helpers.os_name() == 'windows' and '.exe' or '')
     end
 
     it('sets v:shell_error if cmd[0] is not executable', function()
@@ -88,14 +89,14 @@ describe('system()', function()
     end)
 
     it('does NOT run in shell', function()
-      if helpers.os_name() ~= 'windows' then
+      if not iswin() then
         eq("* $PATH %PATH%\n", eval("system(['echo', '*', '$PATH', '%PATH%'])"))
       end
     end)
   end)
 
   it('sets v:shell_error', function()
-    if helpers.os_name() == 'windows' then
+    if iswin() then
       eval([[system("cmd.exe /c exit")]])
       eq(0, eval('v:shell_error'))
       eval([[system("cmd.exe /c exit 1")]])
@@ -129,28 +130,29 @@ describe('system()', function()
         screen:detach()
     end)
 
-    if helpers.os_name() == 'windows' then
+    if iswin() then
-      it('with the default cmd.exe shell', function()
+      it('with shell=cmd.exe', function()
+        command('set shell=cmd.exe')
         eq('""\n', eval([[system('echo ""')]]))
         eq('"a b"\n', eval([[system('echo "a b"')]]))
-        -- TODO: consistent with Vim, but it should be fixed
+        eq('a \nb\n', eval([[system('echo a & echo b')]]))
-        eq('a & echo b\n', eval([[system('echo a & echo b')]]))
+        eq('a \n', eval([[system('echo a 2>&1')]]))
         eval([[system('cd "C:\Program Files"')]])
         eq(0, eval('v:shell_error'))
       end)
 
-      it('with set shell="cmd"', function()
+      it('with shell=cmd', function()
-        helpers.source('let &shell="cmd"')
+        command('set shell=cmd')
         eq('"a b"\n', eval([[system('echo "a b"')]]))
       end)
 
-      it('works with cmd.exe from $COMSPEC', function()
+      it('with shell=$COMSPEC', function()
         local comspecshell = eval("fnamemodify($COMSPEC, ':t')")
         if comspecshell == 'cmd.exe' then
-          helpers.source('let &shell=$COMSPEC')
+          command('set shell=$COMSPEC')
           eq('"a b"\n', eval([[system('echo "a b"')]]))
         else
-          pending('$COMSPEC is not cmd.exe ' .. comspecshell)
+          pending('$COMSPEC is not cmd.exe: ' .. comspecshell)
         end
       end)
 
@@ -222,7 +224,7 @@ describe('system()', function()
 
   describe('passing no input', function()
     it('returns the program output', function()
-      if helpers.os_name() == 'windows' then
+      if iswin() then
         eq("echoed\n", eval('system("echo echoed")'))
       else
         eq("echoed", eval('system("echo -n echoed")'))
@@ -327,8 +329,8 @@ describe('systemlist()', function()
   -- Similar to `system()`, but returns List instead of String.
   before_each(clear)
 
-  it('sets the v:shell_error variable', function()
+  it('sets v:shell_error', function()
-    if helpers.os_name() == 'windows' then
+    if iswin() then
       eval([[systemlist("cmd.exe /c exit")]])
       eq(0, eval('v:shell_error'))
       eval([[systemlist("cmd.exe /c exit 1")]])

test/functional/terminal/edit_spec.lua

@@ -8,7 +8,6 @@ local command = helpers.command
 local meths = helpers.meths
 local clear = helpers.clear
 local eq = helpers.eq
-local iswin = helpers.iswin
 
 describe(':edit term://*', function()
   local get_screen = function(columns, lines)
@@ -46,11 +45,8 @@ describe(':edit term://*', function()
     local bufcontents = {}
     local winheight = curwinmeths.get_height()
     local buf_cont_start = rep_size - sb - winheight + 2
-    local function bufline (i)
-      return (iswin() and '%d: (foobar)' or '%d: foobar'):format(i)
-    end
     for i = buf_cont_start,(rep_size - 1) do
-      bufcontents[#bufcontents + 1] = bufline(i)
+      bufcontents[#bufcontents + 1] = ('%d: foobar'):format(i)
     end
     bufcontents[#bufcontents + 1] = ''
     bufcontents[#bufcontents + 1] = '[Process exited 0]'