Merge pull request #283 from starcraftman/refactor_command

Refactor Python Command & .travis.yml

.travis.yml

@@ -1,35 +1,49 @@
 language: ruby
 rvm:
   - 1.8.7
-  - 1.9.2 # Test with vim-nox package on ubuntu
-  - 1.9.3 # Test against python installer
   - 2.0.0
-  - 2.1.0 # Test against python3 installer
-
-before_script: |
-  sudo apt-get update -y
-  if [ $(ruby -e 'puts RUBY_VERSION') = 1.9.2 ]; then
-    sudo apt-get install -y vim-nox
-    sudo ln -s /usr/bin/vim /usr/local/bin/vim
-  else
-    git clone --depth 1 https://github.com/vim/vim
-    cd vim
-    if [ $(ruby -e 'puts RUBY_VERSION') = 1.9.3 ]; then
-      sudo apt-get install -y python2.7-dev
-      ./configure --disable-gui --with-features=huge --enable-pythoninterp
-    elif [ $(ruby -e 'puts RUBY_VERSION') = 2.1.0 ]; then
-      sudo apt-get install -y python3-dev
-      ./configure --disable-gui --with-features=huge --enable-python3interp
-    else
-      ./configure --disable-gui --with-features=huge --enable-rubyinterp
-    fi
-    make
-    sudo make install
-    cd -
-  fi
-
+env:
+  - ENV=nox
+  - ENV=python
+  - ENV=python3
+  - ENV=ruby
+matrix:
+  exclude:
+    - rvm: 2.0.0
+  include:
+    - rvm: 2.0.0
+      env: ENV=ruby
+install: |
   git config --global user.email "you@example.com"
   git config --global user.name "Your Name"
+  sudo apt-get update -y
 
-script: |
-  test/run !
+  if [ "$ENV" == "nox" ]; then
+    sudo apt-get install -y vim-nox
+    sudo ln -s /usr/bin/vim /usr/local/bin/vim
+    return
+  fi
+
+  C_OPTS="--with-features=huge --disable-gui "
+  case "$ENV" in
+    python)
+      PACKS=python2.7-dev
+      C_OPtS+=--enable-pythoninterp
+      ;;
+    python3)
+      PACKS=python3-dev
+      C_OPtS+=--enable-python3interp
+      ;;
+    ruby)
+      C_OPTS+=--enable-rubyinterp
+      ;;
+  esac
+
+  sudo apt-get install -y $PACKS
+  git clone --depth 1 https://github.com/vim/vim
+  cd vim
+  ./configure $C_OPTS
+  make
+  sudo make install
+  cd -
+script: test/run !

plug.vim

@@ -1050,17 +1050,17 @@ G_LOG_PROB = 1.0 / int(vim.eval('s:update.threads'))
 G_STOP = thr.Event()
 G_THREADS = {}
 
-class BaseExc(Exception):
+class PlugError(Exception):
   def __init__(self, msg):
     self._msg = msg
   @property
   def msg(self):
     return self._msg
-class CmdTimedOut(BaseExc):
+class CmdTimedOut(PlugError):
   pass
-class CmdFailed(BaseExc):
+class CmdFailed(PlugError):
   pass
-class InvalidURI(BaseExc):
+class InvalidURI(PlugError):
   pass
 class Action(object):
   INSTALL, UPDATE, ERROR, DONE = ['+', '*', 'x', '-']
@@ -1074,7 +1074,7 @@ class Buffer(object):
     self.maxy = int(vim.eval('winheight(".")'))
     self.num_plugs = num_plugs
 
-  def _where(self, name):
+  def __where(self, name):
     """ Find first line with name in current buffer. Return line num. """
     found, lnum = False, 0
     matcher = re.compile('^[-+x*] {0}:'.format(name))
@@ -1103,8 +1103,7 @@ class Buffer(object):
   def write(self, action, name, lines):
     first, rest = lines[0], lines[1:]
     msg = ['{0} {1}{2}{3}'.format(action, name, ': ' if first else '', first)]
-    padded_rest = ['    ' + line for line in rest]
-    msg.extend(padded_rest)
+    msg.extend(['    ' + line for line in rest])
 
     try:
       if action == Action.ERROR:
@@ -1114,7 +1113,7 @@ class Buffer(object):
         self.bar += '='
 
       curbuf = vim.current.buffer
-      lnum = self._where(name)
+      lnum = self.__where(name)
       if lnum != -1: # Found matching line num
         del curbuf[lnum]
         if lnum > self.maxy and action in set([Action.INSTALL, Action.UPDATE]):
@@ -1128,56 +1127,62 @@ class Buffer(object):
       pass
 
 class Command(object):
-  def __init__(self, cmd, cmd_dir=None, timeout=60, ntries=3, cb=None, clean=None):
+  def __init__(self, cmd, cmd_dir=None, timeout=60, cb=None, clean=None):
     self.cmd = cmd
     self.cmd_dir = cmd_dir
     self.timeout = timeout
-    self.ntries = ntries
     self.callback = cb if cb else (lambda msg: None)
-    self.clean = clean
+    self.clean = clean if clean else (lambda: None)
+    self.proc = None
 
-  def attempt_cmd(self):
-    """ Tries to run the command, returns result if no exceptions. """
-    attempt = 0
-    finished = False
-    limit = self.timeout
+  @property
+  def alive(self):
+    """ Returns true only if command still running. """
+    return self.proc and self.proc.poll() is None
+
+  def execute(self, ntries=3):
+    """ Execute the command with ntries if CmdTimedOut.
+        Returns the output of the command if no Exception.
+    """
+    attempt, finished, limit = 0, False, self.timeout
 
     while not finished:
       try:
         attempt += 1
-        result = self.timeout_cmd()
+        result = self.try_command()
         finished = True
+        return result
       except CmdTimedOut:
-        if attempt != self.ntries:
-          for count in range(3, 0, -1):
-            if G_STOP.is_set():
-              raise KeyboardInterrupt
-            msg = 'Timeout. Will retry in {0} second{1} ...'.format(
-                count, 's' if count != 1 else '')
-            self.callback([msg])
-            time.sleep(1)
+        if attempt != ntries:
+          self.notify_retry()
           self.timeout += limit
-          self.callback(['Retrying ...'])
         else:
           raise
 
-    return result
+  def notify_retry(self):
+    """ Retry required for command, notify user. """
+    for count in range(3, 0, -1):
+      if G_STOP.is_set():
+        raise KeyboardInterrupt
+      msg = 'Timeout. Will retry in {0} second{1} ...'.format(
+            count, 's' if count != 1 else '')
+      self.callback([msg])
+      time.sleep(1)
+    self.callback(['Retrying ...'])
 
-  def timeout_cmd(self):
+  def try_command(self):
     """ Execute a cmd & poll for callback. Returns list of output.
-    Raises CmdFailed   -> return code for Popen isn't 0
-    Raises CmdTimedOut -> command exceeded timeout without new output
+        Raises CmdFailed   -> return code for Popen isn't 0
+        Raises CmdTimedOut -> command exceeded timeout without new output
     """
-    proc = None
     first_line = True
-    try:
-      tfile = tempfile.NamedTemporaryFile(mode='w+b', delete=False)
-      proc = subprocess.Popen(self.cmd, cwd=self.cmd_dir, stdout=tfile,
-          stderr=subprocess.STDOUT, shell=True, preexec_fn=os.setsid)
-      while proc.poll() is None:
-        # Yield this thread
-        time.sleep(0.2)
 
+    try:
+      tfile = tempfile.NamedTemporaryFile(mode='w+b')
+      self.proc = subprocess.Popen(self.cmd, cwd=self.cmd_dir, stdout=tfile,
+                                   stderr=subprocess.STDOUT, shell=True,
+                                   preexec_fn=os.setsid)
+      while self.alive:
         if G_STOP.is_set():
           raise KeyboardInterrupt
 
@@ -1191,23 +1196,24 @@ class Command(object):
         if time_diff > self.timeout:
           raise CmdTimedOut(['Timeout!'])
 
+        time.sleep(0.33)
+
       tfile.seek(0)
       result = [line.decode('utf-8', 'replace').rstrip() for line in tfile]
 
-      if proc.returncode != 0:
-        msg = ['']
-        msg.extend(result)
-        raise CmdFailed(msg)
-    except:
-      if proc and proc.poll() is None:
-        os.killpg(proc.pid, signal.SIGTERM)
-      if self.clean:
-        self.clean()
-      raise
-    finally:
-      os.remove(tfile.name)
+      if self.proc.returncode != 0:
+        raise CmdFailed([''] + result)
 
-    return result
+      return result
+    except:
+      self.terminate()
+      raise
+
+  def terminate(self):
+    """ Terminate process and cleanup. """
+    if self.alive:
+      os.killpg(self.proc.pid, signal.SIGTERM)
+    self.clean()
 
 class Plugin(object):
   def __init__(self, name, args, buf_q, lock):
@@ -1228,7 +1234,7 @@ class Plugin(object):
         self.install()
         with self.lock:
           thread_vim_command("let s:update.new['{0}'] = 1".format(self.name))
-    except (CmdTimedOut, CmdFailed, InvalidURI) as exc:
+    except PlugError as exc:
       self.write(Action.ERROR, self.name, exc.msg)
     except KeyboardInterrupt:
       G_STOP.set()
@@ -1253,11 +1259,18 @@ 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(
-        '' if self.tag else G_CLONE_OPT, G_PROGRESS, self.args['uri'], self.checkout, esc(target))
-    com = Command(cmd, None, G_TIMEOUT, G_RETRIES, callback, clean(target))
-    result = com.attempt_cmd()
+          '' if self.tag else G_CLONE_OPT, G_PROGRESS, self.args['uri'],
+          self.checkout, esc(target))
+    com = Command(cmd, None, G_TIMEOUT, callback, clean(target))
+    result = com.execute(G_RETRIES)
     self.write(Action.DONE, self.name, result[-1:])
 
+  def repo_uri(self):
+    cmd = 'git rev-parse --abbrev-ref HEAD 2>&1 && git config remote.origin.url'
+    command = Command(cmd, self.args['dir'], G_TIMEOUT,)
+    result = command.execute(G_RETRIES)
+    return result[-1]
+
   def update(self):
     match = re.compile(r'git::?@')
     actual_uri = re.sub(match, '', self.repo_uri())
@@ -1278,18 +1291,12 @@ class Plugin(object):
               '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, G_RETRIES, callback)
-      result = com.attempt_cmd()
+      com = Command(cmd, self.args['dir'], G_TIMEOUT, callback)
+      result = com.execute(G_RETRIES)
       self.write(Action.DONE, self.name, result[-1:])
     else:
       self.write(Action.DONE, self.name, ['Already installed'])
 
-  def repo_uri(self):
-    cmd = 'git rev-parse --abbrev-ref HEAD 2>&1 && git config remote.origin.url'
-    command = Command(cmd, self.args['dir'], G_TIMEOUT, G_RETRIES)
-    result = command.attempt_cmd()
-    return result[-1]
-
   def write(self, action, name, msg):
     self.buf_q.put((action, name, msg))
 
@@ -1326,7 +1333,7 @@ class RefreshThread(thr.Thread):
     while self.running:
       with self.lock:
         thread_vim_command('noautocmd normal! a')
-      time.sleep(0.2)
+      time.sleep(0.33)
 
   def stop(self):
     self.running = False