fix(config): selective cfg write;fix cfg parser

* config parser now handles quoted values correctly. This doesn't hamper
  multi-line support.
* added regression test to travis to assure we will be warned if we
  rewrite and break the user's .gitconfig file
* only rewrite configuration files if we actually called a mutating
  method on the writer. Previously it would always rewrite it.

Fixes #285

4 files changed, 20 insertions, 7 deletions

diff --git a/.travis.yml b/.travis.yml
index b53228ca..389780c7 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,6 +25,9 @@ install:
   # as commits are performed with the default user, it needs to be set for travis too
   - git config --global user.email "travis@ci.com"
   - git config --global user.name "Travis Runner"
+  # If we rewrite the user's config by accident, we will mess it up
+  # and cause subsequent tests to fail
+  - cp git/test/fixtures/.gitconfig ~/
 script: 
   # Make sure we limit open handles to see if we are leaking them
   - ulimit -n 96
diff --git a/git/config.py b/git/config.py
index 38dd1b44..a6a25c7b 100644
--- a/git/config.py
+++ b/git/config.py
@@ -81,6 +81,7 @@ def set_dirty_and_flush_changes(non_const_func):
 
     def flush_changes(self, *args, **kwargs):
         rval = non_const_func(self, *args, **kwargs)
+        self._dirty = True
         self.write()
         return rval
     # END wrapper method
@@ -190,6 +191,7 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje
 
         self._file_or_files = file_or_files
         self._read_only = read_only
+        self._dirty = False
         self._is_initialized = False
         self._merge_includes = merge_includes
         self._lock = None
@@ -304,7 +306,7 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje
                 if mo:
                     # We might just have handled the last line, which could contain a quotation we want to remove
                     optname, vi, optval = mo.group('option', 'vi', 'value')
-                    if vi in ('=', ':') and ';' in optval:
+                    if vi in ('=', ':') and ';' in optval and not optval.strip().startswith('"'):
                         pos = optval.find(';')
                         if pos != -1 and optval[pos - 1].isspace():
                             optval = optval[:pos]
@@ -433,6 +435,8 @@ class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, obje
         :raise IOError: if this is a read-only writer instance or if we could not obtain
             a file lock"""
         self._assure_writable("write")
+        if not self._dirty:
+            return
 
         if isinstance(self._file_or_files, (list, tuple)):
             raise AssertionError("Cannot write back if there is not exactly a single file to write to, have %i files"
diff --git a/git/test/fixtures/.gitconfig b/git/test/fixtures/.gitconfig
new file mode 100644
index 00000000..6a0459f6
--- /dev/null
+++ b/git/test/fixtures/.gitconfig
@@ -0,0 +1,3 @@
+[alias]
+    rbi = "!g() { git rebase -i origin/${1:-master} ; } ; g"
+    expush = "!f() { git branch -f tmp ; { git rbi $1 && git push ; } ; git reset --hard tmp ; git rebase origin/${1:-master}; } ; f"
\ No newline at end of file
diff --git a/git/test/test_config.py b/git/test/test_config.py
index fc2b87b6..7758a094 100644
--- a/git/test/test_config.py
+++ b/git/test/test_config.py
@@ -18,7 +18,6 @@ from git.compat import (
 )
 import io
 import os
-from copy import copy
 from git.config import cp
 
 
@@ -30,21 +29,18 @@ class TestBase(TestCase):
         sio.name = file_path
         return sio
 
-    def _parsers_equal_or_raise(self, lhs, rhs):
-        pass
-
     def test_read_write(self):
         # writer must create the exact same file as the one read before
         for filename in ("git_config", "git_config_global"):
             file_obj = self._to_memcache(fixture_path(filename))
-            file_obj_orig = copy(file_obj)
             w_config = GitConfigParser(file_obj, read_only=False)
             w_config.read()                 # enforce reading
             assert w_config._sections
             w_config.write()                # enforce writing
 
             # we stripped lines when reading, so the results differ
-            assert file_obj.getvalue() and file_obj.getvalue() != file_obj_orig.getvalue()
+            assert file_obj.getvalue()
+            self.assertEqual(file_obj.getvalue(), self._to_memcache(fixture_path(filename)).getvalue())
 
             # creating an additional config writer must fail due to exclusive access
             self.failUnlessRaises(IOError, GitConfigParser, file_obj, read_only=False)
@@ -207,3 +203,10 @@ class TestBase(TestCase):
         assert not cw.has_section('core')
         assert len(cw.items(nn)) == 4
         cw.release()
+
+    def test_complex_aliases(self):
+        file_obj = self._to_memcache(fixture_path('.gitconfig'))
+        w_config = GitConfigParser(file_obj, read_only=False)
+        self.assertEqual(w_config.get('alias', 'rbi'), '"!g() { git rebase -i origin/${1:-master} ; } ; g"')
+        w_config.release()
+        self.assertEqual(file_obj.getvalue(), self._to_memcache(fixture_path('.gitconfig')).getvalue())