Clean up the new g:ale_shell code. Use it for Windows

3 files changed, 53 insertions, 58 deletions

diff --git a/autoload/ale/job.vim b/autoload/ale/job.vim
index 1af95049..f9a917e1 100644
--- a/autoload/ale/job.vim
+++ b/autoload/ale/job.vim
@@ -11,12 +11,6 @@
 " A setting for wrapping commands.
 let g:ale_command_wrapper = get(g:, 'ale_command_wrapper', '')
 
-" A setting for the shell used to execute commands
-let g:ale_shell = get(g:, 'ale_shell', v:null)
-
-" A setting for the arguments we pass to the shell when executing commands
-let g:ale_shell_arguments = get(g:, 'ale_shell_arguments', v:null)
-
 if !has_key(s:, 'job_map')
     let s:job_map = {}
 endif
@@ -179,10 +173,6 @@ endfunction
 function! ale#job#PrepareCommand(buffer, command) abort
     let l:wrapper = ale#Var(a:buffer, 'command_wrapper')
 
-    let l:command = !empty(l:wrapper)
-    \ ? s:PrepareWrappedCommand(l:wrapper, a:command)
-    \ : a:command
-
     " The command will be executed in a subshell. This fixes a number of
     " issues, including reading the PATH variables correctly, %PATHEXT%
     " expansion on Windows, etc.
@@ -190,27 +180,26 @@ function! ale#job#PrepareCommand(buffer, command) abort
     " NeoVim handles this issue automatically if the command is a String,
     " but we'll do this explicitly, so we use the same exact command for both
     " versions.
-    if g:ale_shell is v:null
-      if has('win32')
-          return 'cmd /s/c "' . l:command . '"'
-      endif
+    let l:command = !empty(l:wrapper)
+    \ ? s:PrepareWrappedCommand(l:wrapper, a:command)
+    \ : a:command
 
-      if &shell =~? 'fish$\|pwsh$'
-          return ['/bin/sh', '-c', l:command]
-      endif
+    " If a custom shell is specified, use that.
+    if exists('g:ale_shell')
+        let l:shell_arguments = get(g:, 'ale_shell_arguments', &shellcmdflag)
 
-      return split(&shell) + split(&shellcmdflag) + [l:command]
-    else
-      if has('win32')
-          return g:ale_shell . l:command . '"'
-      endif
+        return split(g:ale_shell) + split(l:shell_arguments) + [l:command]
+    endif
 
-      let l:shell_arguments = g:ale_shell_arguments is v:null
-      \ ? &shellcmdflag
-      \ : g:ale_shell_arguments
+    if has('win32')
+        return 'cmd /s/c "' . l:command . '"'
+    endif
 
-      return split(g:ale_shell) + split(l:shell_arguments) + [l:command]
+    if &shell =~? 'fish$\|pwsh$'
+        return ['/bin/sh', '-c', l:command]
     endif
+
+    return split(&shell) + split(&shellcmdflag) + [l:command]
 endfunction
 
 " Start a job with options which are agnostic to Vim and NeoVim.
diff --git a/doc/ale.txt b/doc/ale.txt
index 56669be6..0e658f3c 100644
--- a/doc/ale.txt
+++ b/doc/ale.txt
@@ -1793,6 +1793,33 @@ g:ale_set_signs                                               *g:ale_set_signs*
   To limit the number of signs ALE will set, see |g:ale_max_signs|.
 
 
+g:ale_shell                                                       *g:ale_shell*
+
+  Type: |String|
+  Default: not set
+
+  Override the shell used by ALE for executing commands. ALE uses 'shell' by
+  default, but falls back in `/bin/sh` if the default shell looks like `fish`
+  or `pwsh`, which are not compatible with all of the commands run by ALE. The
+  shell specified with this option will be used even if it might not work in
+  all cases.
+
+  For Windows, ALE uses `cmd` when this option isn't set. Setting this option
+  will apply shell escaping to the command string, even on Windows.
+
+  NOTE: Consider setting |g:ale_shell_arguments| if this option is defined.
+
+
+g:ale_shell_arguments                                   *g:ale_shell_arguments*
+
+  Type: |String|
+  Default: not set
+
+  This option specifies the arguments to use for executing a command with a
+  custom shell, per |g:ale_shell|. If this option is not set, 'shellcmdflag'
+  will be used instead.
+
+
 g:ale_sign_column_always                             *g:ale_sign_column_always*
 
   Type: |Number|
@@ -2000,33 +2027,6 @@ g:ale_windows_node_executable_path         *g:ale_windows_node_executable_path*
   scripts are executed with whatever executable is configured with this
   setting.
 
-g:ale_shell                                                       *g:ale_shell*
-
-  Type: |String|
-  Default: not set
-
-  This variable is used to determine which shell ale will use to execute 
-  commands. By default this variable is undefined, meaning that ALE will use
-  it's default behavior. Which is to run shells via the shell determined by
-  the `&shell` vim variable, with the arguments `&shellcmdflag`. Ale will fall
-  back to using `/bin/sh`if it detects the underlying `&shell`is either `fish`
-  or `pwsh`. However, if you set this variable ALE will no longer fall back to
-  other shells, meaning if you wanted to use `fish` you could do so via this
-  option. For example if `$SHELL == '/bin/bash'`, but you want to use zsh, 
-  set `g:ale_shell = '/bin/zsh'. 
-
-  Please note - if you are using this option you should consider additionally 
-  setting `g:ale``g:ale_shell_arguments` since the default values for that
-  option might be incompatable with the newly set shell. 
-
-g:ale_shell_arguments                                   *g:ale_shell_arguments*
-
-  Type: |String|
-  Default: not set
-
-  This variable is used to determine what commands vim will pass to the shell
-  to execute it's commands. If this command is not set, but g:ale_shell is
-  set, ale will use `&shellcmdflag` as command arguments.
 
 -------------------------------------------------------------------------------
 6.1. Highlights                                                *ale-highlights*
diff --git a/test/test_prepare_command.vader b/test/test_prepare_command.vader
index f90c881b..4a12b9d0 100644
--- a/test/test_prepare_command.vader
+++ b/test/test_prepare_command.vader
@@ -1,10 +1,14 @@
 Before:
   Save &shell
   Save &shellcmdflag
+  Save g:ale_shell
+  Save g:ale_shell_arguments
+
+  unlet! g:ale_shell
+  unlet! g:ale_shell_arguments
 
 After:
   Restore
-  let g:ale_shell = v:null
 
 Execute(sh should be used when the shell is fish):
   if !has('win32')
@@ -58,8 +62,10 @@ Execute(cmd /s/c as a string should be used on Windows):
   endif
 
 Execute(Setting ale_shell should cause ale#job#PrepareCommand to use set shell):
-  if !has('win32')
-    let g:ale_shell = '/foo/bar'
+  let g:ale_shell = '/foo/bar'
 
-    AssertEqual ['/foo/bar', '-c', 'foobar'], ale#job#PrepareCommand(bufnr(''), "foobar")
-  endif
+  AssertEqual ['/foo/bar', '-c', 'foobar'], ale#job#PrepareCommand(bufnr(''), "foobar")
+
+  let g:ale_shell_arguments = '-x'
+
+  AssertEqual ['/foo/bar', '-x', 'foobar'], ale#job#PrepareCommand(bufnr(''), "foobar")