Refactor duplicate logic in installers

This commit extracts duplicate logic out of three installers.

Pros.
- Better maintainability
- Easier to add/extend the features
- Fixes a bug when 'commit' option is used, submodules are updated
  before the designated commit is checked out

Cons.
- The whole process takes slightly longer due to lost parallelism after pull
- Especially, submodule updates are not parallelized
    - However, this shouldn't matter much in practice as there are few
      plugins that heavily rely on submodules

plug.vim

@@ -58,7 +58,7 @@
 " More information: https://github.com/junegunn/vim-plug
 "
 "
-" Copyright (c) 2015 Junegunn Choi
+" Copyright (c) 2016 Junegunn Choi
 "
 " MIT License
 "
@@ -708,6 +708,28 @@ function! s:assign_name()
   silent! execute 'f' fnameescape(name)
 endfunction
 
+function! s:bang(cmd, ...)
+  try
+    " FIXME: Escaping is incomplete. We could use shellescape with eval,
+    "        but it won't work on Windows.
+    let cmd = a:0 > 0 ? s:with_cd(a:cmd, a:1) : a:cmd
+    let g:_plug_bang = '!'.escape(cmd, '#!%')
+    execute "normal! :execute g:_plug_bang\<cr>\<cr>"
+  finally
+    unlet g:_plug_bang
+  endtry
+  return v:shell_error ? 'Exit status: ' . v:shell_error : ''
+endfunction
+
+function! s:regress_bar()
+  let bar = substitute(getline(2)[1:-2], '.*\zs=', 'x', '')
+  call s:progress_bar(2, bar, len(bar))
+endfunction
+
+function! s:is_updated(file, dir)
+  return !empty(s:system_chomp('git log --pretty=format:"%h" "HEAD...HEAD@{1}" '.a:file, a:dir))
+endfunction
+
 function! s:do(pull, force, todo)
   for [name, spec] in items(a:todo)
     if !isdirectory(spec.dir)
@@ -715,24 +737,14 @@ function! s:do(pull, force, todo)
     endif
     let installed = has_key(s:update.new, name)
     let updated = installed ? 0 :
-      \ (a:pull && index(s:update.errors, name) < 0 && !empty(s:system_chomp('git log --pretty=format:"%h" "HEAD...HEAD@{1}"', spec.dir)))
+      \ (a:pull && index(s:update.errors, name) < 0 && s:is_updated('', spec.dir))
     if a:force || installed || updated
       execute 'cd' s:esc(spec.dir)
       call append(3, '- Post-update hook for '. name .' ... ')
       let error = ''
       let type = type(spec.do)
       if type == s:TYPE.string
-        try
+        let error = s:bang(spec.do)
-          " FIXME: Escaping is incomplete. We could use shellescape with eval,
-          "        but it won't work on Windows.
-          let g:_plug_do = '!'.escape(spec.do, '#!%')
-          execute "normal! :execute g:_plug_do\<cr>\<cr>"
-        finally
-          if v:shell_error
-            let error = 'Exit status: ' . v:shell_error
-          endif
-          unlet g:_plug_do
-        endtry
       elseif type == s:TYPE.funcref
         try
           let status = installed ? 'installed' : (updated ? 'updated' : 'unchanged')
@@ -745,6 +757,10 @@ function! s:do(pull, force, todo)
       endif
       call setline(4, empty(error) ? (getline(4) . 'OK')
                                  \ : ('x' . getline(4)[1:] . error))
+      if !empty(error)
+        call add(s:update.errors, name)
+        call s:regress_bar()
+      endif
       cd -
     endif
   endfor
@@ -754,32 +770,14 @@ function! s:hash_match(a, b)
   return stridx(a:a, a:b) == 0 || stridx(a:b, a:a) == 0
 endfunction
 
-function! s:checkout(plugs)
+function! s:checkout(spec)
-  for [name, spec] in items(a:plugs)
+  let sha = a:spec.commit
-    let sha = spec.commit
+  let output = s:system('git rev-parse HEAD', a:spec.dir)
-    call append(3, '- Checking out '.sha[:6].' of '.name.' ... ')
+  if !v:shell_error && !s:hash_match(sha, s:lines(output)[0])
-    redraw
+    let output = s:system(
-
+          \ 'git fetch --depth 999999 && git checkout '.s:esc(sha), a:spec.dir)
-    let error = []
+  endif
-    let output = s:lines(s:system('git rev-parse HEAD', spec.dir))
+  return output
-    if v:shell_error
-      let error = output
-    elseif !s:hash_match(sha, output[0])
-      let output = s:lines(s:system(
-            \ 'git fetch --depth 999999 && git checkout '.sha, spec.dir))
-      if v:shell_error
-        let error = output
-      endif
-    endif
-    if empty(error)
-      call setline(4, getline(4) . 'OK')
-    else
-      call setline(4, 'x'.getline(4)[1:] . 'Error')
-      for line in reverse(error)
-        call append(4, '    '.line)
-      endfor
-    endif
-  endfor
 endfunction
 
 function! s:finish(pull)
@@ -788,7 +786,7 @@ function! s:finish(pull)
     let s = new_frozen > 1 ? 's' : ''
     call append(3, printf('- Installed %d frozen plugin%s', new_frozen, s))
   endif
-  call append(3, '- Finishing ... ')
+  call append(3, '- Finishing ... ') | 4
   redraw
   call plug#helptags()
   call plug#end()
@@ -931,13 +929,52 @@ function! s:update_impl(pull, force, args) abort
   endif
 endfunction
 
+function! s:log4(name, msg)
+  call setline(4, printf('- %s (%s)', a:msg, a:name))
+  redraw
+endfunction
+
 function! s:update_finish()
   if exists('s:git_terminal_prompt')
     let $GIT_TERMINAL_PROMPT = s:git_terminal_prompt
   endif
   if s:switch_in()
-    call s:checkout(filter(copy(s:update.all), 'has_key(v:val, "commit")'))
+    call append(3, '- Updating ...') | 4
-    call s:do(s:update.pull, s:update.force, filter(copy(s:update.all), 'has_key(v:val, "do")'))
+    for [name, spec] in items(filter(copy(s:update.all), 'index(s:update.errors, v:key) < 0 && (s:update.pull || has_key(s:update.new, v:key))'))
+      let pos = s:logpos(name)
+      if !pos
+        continue
+      endif
+      if has_key(spec, 'commit')
+        call s:log4(name, 'Checking out '.spec.commit)
+        let out = s:checkout(spec)
+      elseif has_key(spec, 'tag')
+        call s:log4(name, 'Checking out '.spec.tag)
+        let out = s:system('git checkout -q '.s:esc(spec.tag).' 2>&1', spec.dir)
+      else
+        let branch = s:esc(get(spec, 'branch', 'master'))
+        call s:log4(name, 'Merging origin/'.branch)
+        let out = s:system('git checkout -q '.branch.' 2>&1'
+              \. (has_key(s:update.new, name) ? '' : ('&& git merge --ff-only origin/'.branch.' 2>&1')), spec.dir)
+      endif
+      if !v:shell_error && filereadable(spec.dir.'/.gitmodules') &&
+            \ (has_key(s:update.new, name) || s:is_updated('.gitmodules', spec.dir))
+        call s:log4(name, 'Updating submodules. This may take a while.')
+        let out .= s:bang('git submodule update --init --recursive 2>&1', spec.dir)
+      endif
+      let msg = printf('%s %s: %s', v:shell_error ? 'x': '-', name, get(s:lines(out), -1, ''))
+      if v:shell_error
+        call add(s:update.errors, name)
+        call s:regress_bar()
+        execute pos 'd _'
+        call append(4, msg) | 4
+      elseif !empty(out)
+        call setline(pos, msg)
+      endif
+      redraw
+    endfor
+    4 d _
+    call s:do(s:update.pull, s:update.force, filter(copy(s:update.all), 'index(s:update.errors, v:key) < 0 && has_key(v:val, "do")'))
     call s:finish(s:update.pull)
     call setline(1, 'Updated. Elapsed time: ' . split(reltimestr(reltime(s:update.start)))[0] . ' sec.')
     call s:switch_out('normal! gg')
@@ -1025,7 +1062,7 @@ function! s:reap(name)
   endif
   let s:update.bar .= job.error ? 'x' : '='
 
-  call s:log(job.error ? 'x' : '-', a:name, job.result)
+  call s:log(job.error ? 'x' : '-', a:name, empty(job.result) ? 'OK' : job.result)
   call s:bar()
 
   call remove(s:jobs, a:name)
@@ -1042,7 +1079,7 @@ function! s:bar()
 endfunction
 
 function! s:logpos(name)
-  for i in range(1, line('$'))
+  for i in range(4, line('$'))
     if getline(i) =~# '^[-+x*] '.a:name.':'
       return i
     endif
@@ -1093,17 +1130,12 @@ while 1 " Without TCO, Vim stack is bound to explode
   redraw
 
   let has_tag = has_key(spec, 'tag')
-  let checkout = s:shellesc(has_tag ? spec.tag : spec.branch)
-  let merge = s:shellesc(has_tag ? spec.tag : 'origin/'.spec.branch)
-
   if !new
     let error = s:git_validate(spec, 0)
     if empty(error)
       if pull
         let fetch_opt = (has_tag && !empty(globpath(spec.dir, '.git/shallow'))) ? '--depth 99999999' : ''
-        call s:spawn(name,
+        call s:spawn(name, printf('git fetch %s %s 2>&1', fetch_opt, prog), { 'dir': spec.dir })
-          \ printf('(git fetch %s %s 2>&1 && git checkout -q %s 2>&1 && git merge --ff-only %s 2>&1 && git submodule update --init --recursive 2>&1)',
-          \ fetch_opt, prog, checkout, merge), { 'dir': spec.dir })
       else
         let s:jobs[name] = { 'running': 0, 'result': 'Already installed', 'error': 0 }
       endif
@@ -1112,11 +1144,10 @@ while 1 " Without TCO, Vim stack is bound to explode
     endif
   else
     call s:spawn(name,
-          \ printf('git clone %s %s --recursive %s -b %s %s 2>&1',
+          \ printf('git clone %s %s %s %s 2>&1',
           \ has_tag ? '' : s:clone_opt,
           \ prog,
           \ s:shellesc(spec.uri),
-          \ checkout,
           \ s:shellesc(s:trim(spec.dir))), { 'new': 1 })
   endif
 
@@ -1345,10 +1376,7 @@ class Plugin(object):
     self.args = args
     self.buf_q = buf_q
     self.lock = lock
-    tag = args.get('tag', 0)
+    self.tag = args.get('tag', 0)
-    self.checkout = esc(tag if tag else args['branch'])
-    self.merge = esc(tag if tag else 'origin/' + args['branch'])
-    self.tag = tag
 
   def manage(self):
     try:
@@ -1384,9 +1412,9 @@ class Plugin(object):
 
     self.write(Action.INSTALL, self.name, ['Installing ...'])
     callback = functools.partial(self.write, Action.INSTALL, self.name)
-    cmd = 'git clone {0} {1} --recursive {2} -b {3} {4} 2>&1'.format(
+    cmd = 'git clone {0} {1} {2} {3} 2>&1'.format(
           '' if self.tag else G_CLONE_OPT, G_PROGRESS, self.args['uri'],
-          self.checkout, esc(target))
+          esc(target))
     com = Command(cmd, None, G_TIMEOUT, callback, clean(target))
     result = com.execute(G_RETRIES)
     self.write(Action.DONE, self.name, result[-1:])
@@ -1412,11 +1440,7 @@ class Plugin(object):
       self.write(Action.UPDATE, self.name, ['Updating ...'])
       callback = functools.partial(self.write, Action.UPDATE, self.name)
       fetch_opt = '--depth 99999999' if self.tag and os.path.isfile(os.path.join(self.args['dir'], '.git/shallow')) else ''
-      cmds = ['git fetch {0} {1}'.format(fetch_opt, G_PROGRESS),
+      cmd = 'git fetch {0} {1} 2>&1'.format(fetch_opt, G_PROGRESS)
-              'git checkout -q {0}'.format(self.checkout),
-              'git merge --ff-only {0}'.format(self.merge),
-              'git submodule update --init --recursive']
-      cmd = ' 2>&1 && '.join(cmds)
       com = Command(cmd, self.args['dir'], G_TIMEOUT, callback)
       result = com.execute(G_RETRIES)
       self.write(Action.DONE, self.name, result[-1:])
@@ -1508,7 +1532,7 @@ def main():
   while not buf_q.empty() or thr.active_count() != start_cnt:
     try:
       action, name, msg = buf_q.get(True, 0.25)
-      buf.write(action, name, msg)
+      buf.write(action, name, ['OK'] if not msg else msg)
       buf_q.task_done()
     except queue.Empty:
       pass
@@ -1602,7 +1626,7 @@ function! s:update_ruby()
           end
       result =
         if type || type.nil?
-          ["#{b} #{name}: #{result.lines.to_a.last}"]
+          ["#{b} #{name}: #{result.lines.to_a.last || 'OK'}"]
         elsif result =~ /^Interrupted|^Timeout/
           ["#{b} #{name}: #{result}"]
         else
@@ -1692,10 +1716,7 @@ function! s:update_ruby()
       threads << Thread.new {
         while pair = take1.call
           name = pair.first
-          dir, uri, branch, tag = pair.last.values_at *%w[dir uri branch tag]
+          dir, uri, tag = pair.last.values_at *%w[dir uri tag]
-          checkout = esc(tag ? tag : branch)
-          merge = esc(tag ? tag : "origin/#{branch}")
-          subm = "git submodule update --init --recursive 2>&1"
           exists = File.directory? dir
           ok, result =
             if exists
@@ -1716,7 +1737,7 @@ function! s:update_ruby()
                 if pull
                   log.call name, 'Updating ...', :update
                   fetch_opt = (tag && File.exist?(File.join(dir, '.git/shallow'))) ? '--depth 99999999' : ''
-                  bt.call "#{chdir} && git fetch #{fetch_opt} #{progress} 2>&1 && git checkout -q #{checkout} 2>&1 && git merge --ff-only #{merge} 2>&1 && #{subm}", name, :update, nil
+                  bt.call "#{chdir} && git fetch #{fetch_opt} #{progress} 2>&1", name, :update, nil
                 else
                   [true, skip]
                 end
@@ -1724,7 +1745,7 @@ function! s:update_ruby()
             else
               d = esc dir.sub(%r{[\\/]+$}, '')
               log.call name, 'Installing ...', :install
-              bt.call "git clone #{clone_opt unless tag} #{progress} --recursive #{uri} -b #{checkout} #{d} 2>&1", name, :install, proc {
+              bt.call "git clone #{clone_opt unless tag} #{progress} #{uri} #{d} 2>&1", name, :install, proc {
                 FileUtils.rm_rf dir
               }
             end

test/workflow.vader

@@ -1296,15 +1296,20 @@ Execute (#221 Shallow-clone disabled by tag):
   cd -
 
 Execute (Commit hash support):
+  " Get goyo back to master
+  call plug#begin(g:temp_plugged)
+  Plug 'junegunn/goyo.vim'
+  call plug#end()
+  PlugUpdate
+
   call plug#begin(g:temp_plugged)
   Plug 'junegunn/goyo.vim',  { 'commit': 'ffffffff' }
   Plug 'junegunn/vim-emoji', { 'commit': '9db7fcfee0d90dafdbcb7a32090c0a9085eb054a' }
   call plug#end()
   PlugUpdate
   Log getline(1, '$')
-  AssertEqual ['x Checking out fffffff of goyo.vim ... Error',
+  AssertEqual 'x goyo.vim: error: pathspec ''ffffffff'' did not match any file(s) known to git.', getline(5)
-              \'    error: pathspec ''ffffffff'' did not match any file(s) known to git.',
+  AssertEqual 0, stridx(getline(6), '- vim-emoji: HEAD is now at 9db7fcf...')
-              \'- Checking out 9db7fcf of vim-emoji ... OK'], getline(5, 7)
 
   let hash = system(printf('cd %s && git rev-parse HEAD', g:plugs['vim-emoji'].dir))[:-2]
   AssertEqual '9db7fcfee0d90dafdbcb7a32090c0a9085eb054a', hash