man.vim, provider.vim: Avoid shell syntax. #5734

- Improves compatibility with shell=tcsh.
- man.vim: split read_page into get_page, put_page. Do not split the
  window until we know there is going to be output.

2 files changed, 74 insertions, 30 deletions

diff --git a/runtime/autoload/health/provider.vim b/runtime/autoload/health/provider.vim
index cb32dab376..9cf540ba09 100644
--- a/runtime/autoload/health/provider.vim
+++ b/runtime/autoload/health/provider.vim
@@ -36,12 +36,15 @@ endfunction
 " Run a system command and timeout after 30 seconds.
 function! s:system(cmd, ...) abort
   let stdin = a:0 ? a:1 : ''
+  let ignore_stderr = a:0 > 1 ? a:2 : 0
   let opts = {
         \ 'output': '',
         \ 'on_stdout': function('s:system_handler'),
-        \ 'on_stderr': function('s:system_handler'),
         \ 'on_exit': function('s:system_handler'),
         \ }
+  if !ignore_stderr
+    let opts.on_stderr = function('s:system_handler')
+  endif
   let jobid = jobstart(a:cmd, opts)
 
   if jobid < 1
@@ -258,8 +261,7 @@ function! s:check_python(version) abort
         call health#report_ok(printf('pyenv found: "%s"', pyenv))
       endif
 
-      let python_bin = s:trim(s:system(
-            \ printf('"%s" which %s 2>/dev/null', pyenv, python_bin_name)))
+      let python_bin = s:trim(s:system([pyenv, 'which', python_bin_name], '', 1))
 
       if empty(python_bin)
         call health#report_warn(printf('pyenv couldn''t find %s.', python_bin_name))
@@ -416,9 +418,8 @@ function! s:check_ruby() abort
     let prog_vers = 'not found'
     call health#report_error('Missing Neovim RubyGem', suggestions)
   else
-    silent let latest_gem = get(s:systemlist("gem list -ra '^neovim$' 2>/dev/null | " .
-          \ "awk -F'[()]' '{print $2}' | " .
-          \ 'cut -d, -f1'), 0, 'not found')
+    silent let latest_gem = get(split(s:system(['gem', 'list', '-ra', '^neovim$']), 
+          \ ' (\|, \|)$' ), 1, 'not found')
     let latest_desc = ' (latest: ' . latest_gem . ')'
 
     silent let prog_vers = s:systemlist(ruby_prog . ' --version')[0]
diff --git a/runtime/autoload/man.vim b/runtime/autoload/man.vim
index 320dd84263..665839a80e 100644
--- a/runtime/autoload/man.vim
+++ b/runtime/autoload/man.vim
@@ -1,11 +1,5 @@
 " Maintainer: Anmol Sethi <anmol@aubble.com>
 
-if &shell =~# 'fish$'
-  let s:man_cmd = 'man ^/dev/null'
-else
-  let s:man_cmd = 'man 2>/dev/null'
-endif
-
 let s:man_find_arg = "-w"
 
 " TODO(nhooyr) Completion may work on SunOS; I'm not sure if `man -l` displays
@@ -45,10 +39,12 @@ function! man#open_page(count, count1, mods, ...) abort
       let sect = string(a:count)
     endif
     let [sect, name, path] = s:verify_exists(sect, name)
+    let page = s:get_page(path)
   catch
     call s:error(v:exception)
     return
   endtry
+
   call s:push_tag()
   let bufname = 'man://'.name.(empty(sect)?'':'('.sect.')')
   if a:mods !~# 'tab' && s:find_man()
@@ -57,30 +53,77 @@ function! man#open_page(count, count1, mods, ...) abort
     noautocmd execute 'silent' a:mods 'split' fnameescape(bufname)
   endif
   let b:man_sect = sect
-  call s:read_page(path)
+  call s:put_page(page)
 endfunction
 
 function! man#read_page(ref) abort
   try
     let [sect, name] = man#extract_sect_and_name_ref(a:ref)
     let [b:man_sect, name, path] = s:verify_exists(sect, name)
+    let page = s:get_page(path)
   catch
     " call to s:error() is unnecessary
     return
   endtry
-  call s:read_page(path)
+  call s:put_page(page)
 endfunction
 
-function! s:read_page(path) abort
-  setlocal modifiable
-  setlocal noreadonly
-  silent keepjumps %delete _
+" Handler for s:system() function.
+function! s:system_handler(jobid, data, event) dict abort
+  if a:event == 'stdout'
+    let self.stdout .= join(a:data, "\n")
+  elseif a:event == 'stderr'
+    let self.stderr .= join(a:data, "\n")
+  else
+    let self.exit_code = a:data
+  endif
+endfunction
+
+" Run a system command and timeout after 30 seconds.
+function! s:system(cmd, ...) abort
+  let opts = {
+        \ 'stdout': '',
+        \ 'stderr': '',
+        \ 'exit_code': 0,
+        \ 'on_stdout': function('s:system_handler'),
+        \ 'on_stderr': function('s:system_handler'),
+        \ 'on_exit': function('s:system_handler'),
+        \ }
+  let jobid = jobstart(a:cmd, opts)
+
+  if jobid < 1
+    throw printf('command error %d: %s', jobid, join(a:cmd))
+  endif
+
+  let res = jobwait([jobid], 30000)
+  if res[0] == -1
+    try
+      call jobstop(jobid)
+      throw printf('command timed out: %s', join(a:cmd))
+    catch /^Vim\%((\a\+)\)\=:E900/
+    endtry
+  elseif res[0] == -2
+    throw printf('command interrupted: %s', join(a:cmd))
+  endif
+  if opts.exit_code != 0
+    throw printf("command error (%d) %s: %s", jobid, join(a:cmd), opts.stderr)
+  endif
+
+  return opts.stdout
+endfunction
+
+function! s:get_page(path) abort
   " Force MANPAGER=cat to ensure Vim is not recursively invoked (by man-db).
   " http://comments.gmane.org/gmane.editors.vim.devel/29085
   " Respect $MANWIDTH, or default to window width.
-  let cmd  = 'env MANPAGER=cat'.(empty($MANWIDTH) ? ' MANWIDTH='.winwidth(0) : '')
-  let cmd .= ' '.s:man_cmd.' '.shellescape(a:path)
-  silent put =system(cmd)
+  return s:system(['env', 'MANPAGER=cat', (empty($MANWIDTH) ? 'MANWIDTH='.winwidth(0) : ''), 'man', a:path])
+endfunction
+
+function! s:put_page(page) abort
+  setlocal modifiable
+  setlocal noreadonly
+  silent keepjumps %delete _
+  silent put =a:page
   " Remove all backspaced characters.
   execute 'silent keeppatterns keepjumps %substitute,.\b,,e'.(&gdefault?'':'g')
   while getline(1) =~# '^\s*$'
@@ -112,18 +155,14 @@ endfunction
 
 function! s:get_path(sect, name) abort
   if empty(a:sect)
-    let path = system(s:man_cmd.' '.s:man_find_arg.' '.shellescape(a:name))
-    if path !~# '^\/'
-      throw 'no manual entry for '.a:name
-    endif
-    return path
+    return s:system(['man', s:man_find_arg, a:name])
   endif
   " '-s' flag handles:
   "   - tokens like 'printf(echo)'
   "   - sections starting with '-'
   "   - 3pcap section (found on macOS)
   "   - commas between sections (for section priority)
-  return system(s:man_cmd.' '.s:man_find_arg.' -s '.shellescape(a:sect).' '.shellescape(a:name))
+  return s:system(['man', s:man_find_arg, '-s', a:sect, a:name])
 endfunction
 
 function! s:verify_exists(sect, name) abort
@@ -197,8 +236,6 @@ function! s:error(msg) abort
   echohl None
 endfunction
 
-let s:mandirs = join(split(system(s:man_cmd.' '.s:man_find_arg), ':\|\n'), ',')
-
 " see man#extract_sect_and_name_ref on why tolower(sect)
 function! man#complete(arg_lead, cmd_line, cursor_pos) abort
   let args = split(a:cmd_line)
@@ -247,9 +284,15 @@ function! man#complete(arg_lead, cmd_line, cursor_pos) abort
 endfunction
 
 function! s:complete(sect, psect, name) abort
+  try
+    let mandirs = join(split(s:system(['man', s:man_find_arg]), ':\|\n'), ',')
+  catch
+    call s:error(v:exception)
+    return
+  endtry
   let old_fic = &fileignorecase
   let &fileignorecase = &wildignorecase
-  let pages = globpath(s:mandirs,'man?/'.a:name.'*.'.a:sect.'*', 0, 1)
+  let pages = globpath(mandirs,'man?/'.a:name.'*.'.a:sect.'*', 0, 1)
   let &fileignorecase = old_fic
   " We remove duplicates in case the same manpage in different languages was found.
   return uniq(sort(map(pages, 's:format_candidate(v:val, a:psect)'), 'i'))