git plugin: addressed review + misc improvments

* fixed annoying recursion in exception handlers (fake_env and sand_env)
* "pubs git" always not quiet
* color option for git ouput through "pubs git"
* "pubs git" output without any "info:" prefix or extraneous new line.
* is_loaded() method for plugins

pubs/config/spec.py

@@ -96,11 +96,24 @@ active = force_list(default=list('alias'))
 # description = lists number of pubs in repo
 
 [[git]]
-# the plugin allows to use `pubs git` and commit changes automatically
+# The git plugin will commit changes to the repository in a git repository
-# if False, will display git output when invoked
+# created at the root of the pubs directory. All detected changes will be
+# commited every time a change is made by a pubs command.
+# The plugin also propose the `pubs git` subcommand, to directory send git
+# command to the pubs repository. Therefore, `pubs git status` is equivalent
+# to `git -C <pubsdir> status`, with the `-C` flag instructing
+# to invoke git as if the current directory was <pubsdir>. Note that a
+# limitation of the subcommand is that you cannot use git commands with the
+#  `-c` option (pubs will interpret it first.)
+
+# if False, will display git output when automatic commit are made.
+# Invocation of `pubs git` will always have output displayed.
 quiet = boolean(default=True)
 # if True, git will not automatically commit changes
 manual = boolean(default=False)
+# if True, color will be conserved from git output (this add `-c color:always`
+# to the git invocation).
+force_color = boolean(default=True)
 
 
 [internal]

pubs/events.py

@@ -48,27 +48,27 @@ class PaperChangeEvent(Event):
 
 # Used by repo.push_paper()
 class AddEvent(PaperChangeEvent):
-    _format = "Adds paper {citekey}."
+    _format = "Added paper {citekey}."
 
 # Used by repo.push_doc()
 class DocAddEvent(PaperChangeEvent):
-    _format = "Adds document for {citekey}."
+    _format = "Added document for {citekey}."
 
 # Used by repo.remove_paper()
 class RemoveEvent(PaperChangeEvent):
-    _format = "Removes paper for {citekey}."
+    _format = "Removed paper for {citekey}."
 
 # Used by repo.remove_doc()
 class DocRemoveEvent(PaperChangeEvent):
-    _format = "Removes document for {citekey}."
+    _format = "Removed document for {citekey}."
 
 # Used by commands.tag_cmd.command()
 class TagEvent(PaperChangeEvent):
-    _format = "Updates tags for {citekey}."
+    _format = "Updated tags for {citekey}."
 
 # Used by commands.edit_cmd.command()
 class ModifyEvent(PaperChangeEvent):
-    _format = "Modifies {file_type} file of {citekey}."
+    _format = "Modified {file_type} file of {citekey}."
 
     def __init__(self, citekey, file_type):
         super(ModifyEvent, self).__init__(citekey)
@@ -80,7 +80,7 @@ class ModifyEvent(PaperChangeEvent):
 
 # Used by repo.rename_paper()
 class RenameEvent(PaperChangeEvent):
-    _format = "Renames paper {old_citekey} to {citekey}."
+    _format = "Renamed paper {old_citekey} to {citekey}."
 
     def __init__(self, paper, old_citekey):
         super(RenameEvent, self).__init__(paper.citekey)
@@ -93,4 +93,4 @@ class RenameEvent(PaperChangeEvent):
 
 # Used by commands.note_cmd.command()
 class NoteEvent(PaperChangeEvent):
-    _format = "Modifies note {citekey}."
+    _format = "Modified note of {citekey}."

pubs/plugins.py

@@ -27,6 +27,10 @@ class PapersPlugin(object):
         else:
             raise RuntimeError("{} instance not created".format(cls.__name__))
 
+    @classmethod
+    def is_loaded(cls):
+        return cls in _instances
+
 
 def load_plugins(conf, ui):
     """Imports the modules for a sequence of plugin names. Each name

pubs/plugs/git/git.py

@@ -1,14 +1,16 @@
 import os
 import sys
 import argparse
-from subprocess import Popen, PIPE
+from subprocess import Popen, PIPE, STDOUT
 from pipes import quote as shell_quote
 
+from ... import uis
 from ...plugins import PapersPlugin
 from ...events import PaperChangeEvent, PostCommandEvent
 
 
 GITIGNORE = """# files or directories for the git plugin to ignore
+.gitignore
 .cache/
 """
 
@@ -26,8 +28,9 @@ class GitPlugin(PapersPlugin):
 
     def __init__(self, conf, ui):
         self.ui = ui
-        self.pubsdir = conf['main']['pubsdir']
+        self.pubsdir     = os.path.expanduser(conf['main']['pubsdir'])
         self.manual      = conf['plugins'].get('git', {}).get('manual', False)
+        self.force_color = conf['plugins'].get('git', {}).get('force_color', True)
         self.quiet   = conf['plugins'].get('git', {}).get('quiet', True)
         self.list_of_changes = []
         self._gitinit()
@@ -35,13 +38,16 @@ class GitPlugin(PapersPlugin):
     def _gitinit(self):
         """Initialize the git repository if necessary."""
         # check that a `.git` directory is present in the pubs dir
-        git_path = os.path.expanduser(os.path.join(self.pubsdir, '.git'))
+        git_path = os.path.join(self.pubsdir, '.git')
         if not os.path.isdir(git_path):
+            try:
                 self.shell('init')
+            except RuntimeError as exc:
+                self.ui.error(exc.args[0])
+                sys.exit(1)
         # check that a `.gitignore` file is present
-        gitignore_path = os.path.expanduser(os.path.join(self.pubsdir, '.gitignore'))
+        gitignore_path = os.path.join(self.pubsdir, '.gitignore')
         if not os.path.isfile(gitignore_path):
-            print('bla')
             with open(gitignore_path, 'w') as fd:
                 fd.write(GITIGNORE)
 
@@ -55,25 +61,30 @@ class GitPlugin(PapersPlugin):
 
     def command(self, conf, args):
         """Execute a git command in the pubs directory"""
-        self.shell(' '.join([shell_quote(a) for a in args.arguments]))
+        self.shell(' '.join([shell_quote(a) for a in args.arguments]), command=True)
 
-    def shell(self, cmd, input_stdin=None):
+    def shell(self, cmd, input_stdin=None, command=False):
         """Runs the git program in a shell
 
         :param cmd: the git command, and all arguments, as a single string (e.g. 'add .')
         :param input_stdin:  if Python 3, must be bytes (i.e., from str, s.encode('utf-8'))
+        :param command: if True, we're dealing with an explicit `pubs git` invocation.
         """
-        git_cmd = 'git -C {} {}'.format(self.pubsdir, cmd)
+        colorize = ' -c color.ui=always' if self.force_color else ''
-        p = Popen(git_cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE, shell=True)
+        git_cmd = 'git -C {}{} {}'.format(self.pubsdir, colorize, cmd)
+        #print(git_cmd)
+        p = Popen(git_cmd, stdin=PIPE, stdout=PIPE, stderr=STDOUT, shell=True)
         output, err = p.communicate(input_stdin)
         p.wait()
+
         if p.returncode != 0:
-            msg = ('The git plugin encountered an error when running the git command:\n' +
+            raise RuntimeError('The git plugin encountered an error when running the git command:\n' +
-                   '{}\n{}\n'.format(git_cmd, err.decode('utf-8')) +
+                '{}\n\nReturned output:\n{}\n'.format(git_cmd, output.decode('utf-8')) +
-                   'You may fix the state of the git repository {} manually.\n'.format(self.pubsdir) +
+                'If needed, you may fix the state of the {} git repository '.format(self.pubsdir) +
-                   'If relevant, you may submit a bug report at ' +
+                'manually.\nIf relevant, you may submit a bug report at ' +
                 'https://github.com/pubs/pubs/issues')
-            self.ui.warning(msg)
+        elif command:
+            self.ui.message(output.decode('utf-8'), end='')
         elif not self.quiet:
             self.ui.info(output.decode('utf-8'))
         return output, err, p.returncode
@@ -82,18 +93,17 @@ class GitPlugin(PapersPlugin):
 @PaperChangeEvent.listen()
 def paper_change_event(event):
     """When a paper is changed, commit the changes to the directory."""
-    try:
+    if GitPlugin.is_loaded():
         git = GitPlugin.get_instance()
         if not git.manual:
             event_desc = event.description
             for a, b in [('\\','\\\\'), ('"','\\"'), ('$','\\$'), ('`','\\`')]:
                 event_desc = event_desc.replace(a, b)
             git.list_of_changes.append(event_desc)
-    except RuntimeError:
-        pass
 
 @PostCommandEvent.listen()
 def git_commit(event):
+    if GitPlugin.is_loaded():
         try:
             git = GitPlugin.get_instance()
             if len(git.list_of_changes) > 0:
@@ -103,5 +113,5 @@ def git_commit(event):
 
                     git.shell('add .')
                     git.shell('commit -F-', message.encode('utf-8'))
-    except RuntimeError:
+        except RuntimeError as exc:
-        pass
+            uis.get_ui().warning(exc.args[0])

pubs/uis.py

@@ -105,6 +105,19 @@ class PrintUI(object):
             self.exit()
         return True # never happens
 
+    def test_handle_exception(self, exc):
+        """Attempts to handle exception.
+
+        :returns: True if exception has been handled (currently never happens)
+        """
+        self.error(ustr(exc))
+        if DEBUG or self.debug:
+            raise
+        else:
+            self.exit()
+        return True # never happens
+
+
 
 class InputUI(PrintUI):
     """UI class. Stores configuration parameters and system information.

tests/fake_env.py

@@ -9,7 +9,7 @@ import dotdot
 from pyfakefs import fake_filesystem, fake_filesystem_unittest
 
 from pubs.p3 import input, _fake_stdio, _get_fake_stdio_ucontent
-from pubs import content, filebroker
+from pubs import content, filebroker, uis
 
 # code for fake fs
 
@@ -20,6 +20,8 @@ real_shutil  = shutil
 real_glob    = glob
 real_io      = io
 
+original_exception_handler = uis.InputUI.handle_exception
+
 
 # capture output
 
@@ -70,6 +72,7 @@ class FakeInput():
         self.inputs = list(inputs) or []
         self.module_list = module_list
         self._cursor = 0
+        self._original_handler = None
 
     def as_global(self):
         for md in self.module_list:
@@ -78,13 +81,11 @@ class FakeInput():
                 md.InputUI.editor_input = self
                 md.InputUI.edit_file = self.input_to_file
                 # Do not catch UnexpectedInput
-                original_handler = md.InputUI.handle_exception
-
                 def handler(ui, exc):
                     if isinstance(exc, self.UnexpectedInput):
                         raise
                     else:
-                        original_handler(ui, exc)
+                        original_exception_handler(ui, exc)
 
                 md.InputUI.handle_exception = handler
 

tests/sand_env.py

@@ -8,15 +8,17 @@ import unittest
 
 import six
 
-from pubs import pubs_cmd, color, content, uis, p3
+from pubs import pubs_cmd, color, content, uis, p3, events
 from pubs.config import conf
 from pubs.p3 import _fake_stdio, _get_fake_stdio_ucontent
 
 
 # makes the tests very noisy
-PRINT_OUTPUT   = True
+PRINT_OUTPUT   = False
 CAPTURE_OUTPUT = True
 
+original_exception_handler = uis.InputUI.handle_exception
+
 
 class FakeSystemExit(Exception):
     """\
@@ -71,7 +73,6 @@ class FakeInput():
         input() returns 'no'
         input() raises IndexError
      """
-
     class UnexpectedInput(Exception):
         pass
 
@@ -87,14 +88,11 @@ class FakeInput():
                 md.InputUI.editor_input = self
                 md.InputUI.edit_file = self.input_to_file
 
-                # Do not catch UnexpectedInput
-                original_handler = md.InputUI.handle_exception
-
                 def handler(ui, exc):
                     if isinstance(exc, self.UnexpectedInput):
                         raise
                     else:
-                        original_handler(ui, exc)
+                        original_exception_handler(ui, exc)
 
                 md.InputUI.handle_exception = handler
 
@@ -141,7 +139,6 @@ class SandboxedCommandTestCase(unittest.TestCase):
     def _preprocess_cmd(self, cmd):
         """Sandbox the pubs command into a temporary directory"""
         cmd_chunks = cmd.split(' ')
-        print(cmd, cmd_chunks[0], 'pubs')
         assert cmd_chunks[0] == 'pubs'
         prefix = ['pubs', '-c', self.default_conf_path]
         if cmd_chunks[1] == 'init':

tests/test_git.py

@@ -59,19 +59,31 @@ class TestGitPlugin(sand_env.SandboxedCommandTestCase):
         self.assertEqual(hash_g, hash_h)
         self.assertNotEqual(hash_h, hash_i)
 
+        # # basically can't test that because each command is not completely independent in
+        # # SandoboxedCommands.
+        # # will work if we use subprocess.
+        # conf = config.load_conf(path=self.default_conf_path)
+        # conf['plugins']['active'] = []
+        # config.save_conf(conf, path=self.default_conf_path)
+        #
+        # self.execute_cmds([('pubs add data/pagerank.bib',)])
+        # hash_j = git_hash(self.default_pubs_dir)
+        #
+        # self.assertEqual(hash_i, hash_j)
+
+    def test_manual(self):
         conf = config.load_conf(path=self.default_conf_path)
-        conf['plugins']['active'] = []
+        conf['plugins']['active'] = ['git']
+        conf['plugins']['git']['manual'] = True
         config.save_conf(conf, path=self.default_conf_path)
 
+        # this three lines just to initialize the git HEAD
         self.execute_cmds([('pubs add data/pagerank.bib',)])
-        hash_j = git_hash(self.default_pubs_dir)
+        self.execute_cmds([('pubs git add .',)])
-
+        self.execute_cmds([('pubs git commit -m "initial_commit"',)])
-        self.assertEqual(hash_i, hash_j)
 
-        conf = config.load_conf(path=self.default_conf_path)
+        self.execute_cmds([('pubs add data/pagerank.bib',)])
-        conf['plugins']['active'] = ['git']
+        hash_j = git_hash(self.default_pubs_dir)
-        conf['plugins']['git']['manual'] = True
-        config.save_conf(conf, path=self.default_conf_path)
 
         self.execute_cmds([('pubs add data/pagerank.bib',)])
         hash_k = git_hash(self.default_pubs_dir)