Support multiple git config values per option

Solves #717

Author: ajdavis

AUTHORS

@@ -28,5 +28,6 @@ Contributors are:
 -Yaroslav Halchenko <debian _at_ onerussian.com>
 -Tim Swast <swast _at_ google.com>
 -William Luc Ritchie
+-A. Jesse Jiryu Davis <jesse _at_ emptysquare.net>
 
 Portions derived from other open source works and are clearly marked.

git/config.py

@@ -146,6 +146,43 @@ def __exit__(self, exception_type, exception_value, traceback):
         self._config.__exit__(exception_type, exception_value, traceback)
 
 
+class _OMD(OrderedDict):
+    """Ordered multi-dict."""
+
+    def __setitem__(self, key, value):
+        super(_OMD, self).__setitem__(key, [value])
+
+    def add(self, key, value):
+        if key not in self:
+            super(_OMD, self).__setitem__(key, [value])
+            return
+
+        super(_OMD, self).__getitem__(key).append(value)
+
+    def setall(self, key, values):
+        super(_OMD, self).__setitem__(key, values)
+
+    def __getitem__(self, key):
+        return super(_OMD, self).__getitem__(key)[-1]
+
+    def getlast(self, key):
+        return super(_OMD, self).__getitem__(key)[-1]
+
+    def setlast(self, key, value):
+        if key not in self:
+            super(_OMD, self).__setitem__(key, [value])
+            return
+
+        prior = super(_OMD, self).__getitem__(key)
+        prior[-1] = value
+
+    def getall(self, key):
+        return super(_OMD, self).__getitem__(key)
+
+    def items_all(self):
+        return [(k, self.get(k)) for k in self]
+
+
 class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, object)):
 
     """Implements specifics required to read git style configuration files.
@@ -200,7 +237,7 @@ def __init__(self, file_or_files, read_only=True, merge_includes=True):
             contents into ours. This makes it impossible to write back an individual configuration file.
             Thus, if you want to modify a single configuration file, turn this off to leave the original
             dataset unaltered when reading it."""
-        cp.RawConfigParser.__init__(self, dict_type=OrderedDict)
+        cp.RawConfigParser.__init__(self, dict_type=_OMD)
 
         # Used in python 3, needs to stay in sync with sections for underlying implementation to work
         if not hasattr(self, '_proxies'):
@@ -348,7 +385,8 @@ def string_decode(v):
                         is_multi_line = True
                         optval = string_decode(optval[1:])
                     # end handle multi-line
-                    cursect[optname] = optval
+                    # preserves multiple values for duplicate optnames
+                    cursect.add(optname, optval)
                 else:
                     # check if it's an option with no value - it's just ignored by git
                     if not self.OPTVALUEONLY.match(line):
@@ -362,7 +400,8 @@ def string_decode(v):
                     is_multi_line = False
                     line = line[:-1]
                 # end handle quotations
-                cursect[optname] += string_decode(line)
+                optval = cursect.getlast(optname)
+                cursect.setlast(optname, optval + string_decode(line))
             # END parse section or option
         # END while reading
 
@@ -442,9 +481,17 @@ def _write(self, fp):
         git compatible format"""
         def write_section(name, section_dict):
             fp.write(("[%s]\n" % name).encode(defenc))
-            for (key, value) in section_dict.items():
-                if key != "__name__":
-                    fp.write(("\t%s = %s\n" % (key, self._value_to_string(value).replace('\n', '\n\t'))).encode(defenc))
+            for (key, value) in section_dict.items_all():
+                if key == "__name__":
+                    continue
+                elif isinstance(value, list):
+                    values = value
+                else:
+                    # self._defaults isn't a multidict
+                    values = [value]
+
+                for v in values:
+                    fp.write(("\t%s = %s\n" % (key, self._value_to_string(v).replace('\n', '\n\t'))).encode(defenc))
                 # END if key is not __name__
         # END section writing
 
@@ -457,6 +504,27 @@ def items(self, section_name):
         """:return: list((option, value), ...) pairs of all items in the given section"""
         return [(k, v) for k, v in super(GitConfigParser, self).items(section_name) if k != '__name__']
 
+    def items_all(self, section_name):
+        """:return: list((option, [values...]), ...) pairs of all items in the given section"""
+        rv = OrderedDict()
+        for k, v in self._defaults:
+            rv[k] = [v]
+
+        for k, v in self._sections[section_name].items_all():
+            if k == '__name__':
+                continue
+
+            if k not in rv:
+                rv[k] = v
+                continue
+
+            if rv[k] == v:
+                continue
+
+            rv[k].extend(v)
+
+        return rv.items()
+
     @needs_values
     def write(self):
         """Write changes to our file, if there are changes at all
@@ -508,7 +576,11 @@ def read_only(self):
         return self._read_only
 
     def get_value(self, section, option, default=None):
-        """
+        """Get an option's value.
+
+        If multiple values are specified for this option in the section, the
+        last one specified is returned.
+
         :param default:
             If not None, the given default value will be returned in case
             the option did not exist
@@ -523,6 +595,31 @@ def get_value(self, section, option, default=None):
                 return default
             raise
 
+        return self._string_to_value(valuestr)
+
+    def get_values(self, section, option, default=None):
+        """Get an option's values.
+
+        If multiple values are specified for this option in the section, all are
+        returned.
+
+        :param default:
+            If not None, a list containing the given default value will be
+            returned in case the option did not exist
+        :return: a list of properly typed values, either int, float or string
+
+        :raise TypeError: in case the value could not be understood
+            Otherwise the exceptions known to the ConfigParser will be raised."""
+        try:
+            lst = self._sections[section].getall(option)
+        except Exception:
+            if default is not None:
+                return [default]
+            raise
+
+        return [self._string_to_value(valuestr) for valuestr in lst]
+
+    def _string_to_value(self, valuestr):
         types = (int, float)
         for numtype in types:
             try:
@@ -545,7 +642,9 @@ def get_value(self, section, option, default=None):
             return True
 
         if not isinstance(valuestr, string_types):
-            raise TypeError("Invalid value type: only int, long, float and str are allowed", valuestr)
+            raise TypeError(
+                "Invalid value type: only int, long, float and str are allowed",
+                valuestr)
 
         return valuestr
 
@@ -572,6 +671,25 @@ def set_value(self, section, option, value):
         self.set(section, option, self._value_to_string(value))
         return self
 
+    @needs_values
+    @set_dirty_and_flush_changes
+    def add_value(self, section, option, value):
+        """Adds a value for the given option in section.
+        It will create the section if required, and will not throw as opposed to the default
+        ConfigParser 'set' method. The value becomes the new value of the option as returned
+        by 'get_value', and appends to the list of values returned by 'get_values`'.
+
+        :param section: Name of the section in which the option resides or should reside
+        :param option: Name of the option
+
+        :param value: Value to add to option. It must be a string or convertible
+            to a string
+        :return: this instance"""
+        if not self.has_section(section):
+            self.add_section(section)
+        self._sections[section].add(option, self._value_to_string(value))
+        return self
+
     def rename_section(self, section, new_name):
         """rename the given section to new_name
         :raise ValueError: if section doesn't exit
@@ -584,8 +702,9 @@ def rename_section(self, section, new_name):
             raise ValueError("Destination section '%s' already exists" % new_name)
 
         super(GitConfigParser, self).add_section(new_name)
-        for k, v in self.items(section):
-            self.set(new_name, k, self._value_to_string(v))
+        new_section = self._sections[new_name]
+        for k, vs in self.items_all(section):
+            new_section.setall(k, vs)
         # end for each value to copy
 
         # This call writes back the changes, which is why we don't have the respective decorator

git/test/fixtures/git_config_multiple

@@ -0,0 +1,7 @@
+[section0]
+    option0 = value0
+
+[section1]
+    option1 = value1a
+    option1 = value1b
+    other_option1 = other_value1

git/test/test_config.py

@@ -265,3 +265,100 @@ def test_empty_config_value(self):
 
         with self.assertRaises(cp.NoOptionError):
             cr.get_value('color', 'ui')
+
+    def test_multiple_values(self):
+        file_obj = self._to_memcache(fixture_path('git_config_multiple'))
+        with GitConfigParser(file_obj, read_only=False) as cw:
+            self.assertEqual(cw.get('section0', 'option0'), 'value0')
+            self.assertEqual(cw.get_values('section0', 'option0'), ['value0'])
+            self.assertEqual(cw.items('section0'), [('option0', 'value0')])
+
+            # Where there are multiple values, "get" returns the last.
+            self.assertEqual(cw.get('section1', 'option1'), 'value1b')
+            self.assertEqual(cw.get_values('section1', 'option1'),
+                             ['value1a', 'value1b'])
+            self.assertEqual(cw.items('section1'),
+                             [('option1', 'value1b'),
+                              ('other_option1', 'other_value1')])
+            self.assertEqual(cw.items_all('section1'),
+                             [('option1', ['value1a', 'value1b']),
+                              ('other_option1', ['other_value1'])])
+            with self.assertRaises(KeyError):
+                cw.get_values('section1', 'missing')
+
+            self.assertEqual(cw.get_values('section1', 'missing', 1), [1])
+            self.assertEqual(cw.get_values('section1', 'missing', 's'), ['s'])
+
+    def test_multiple_values_rename(self):
+        file_obj = self._to_memcache(fixture_path('git_config_multiple'))
+        with GitConfigParser(file_obj, read_only=False) as cw:
+            cw.rename_section('section1', 'section2')
+            cw.write()
+            file_obj.seek(0)
+            cr = GitConfigParser(file_obj, read_only=True)
+            self.assertEqual(cr.get_value('section2', 'option1'), 'value1b')
+            self.assertEqual(cr.get_values('section2', 'option1'),
+                             ['value1a', 'value1b'])
+            self.assertEqual(cr.items('section2'),
+                             [('option1', 'value1b'),
+                              ('other_option1', 'other_value1')])
+            self.assertEqual(cr.items_all('section2'),
+                             [('option1', ['value1a', 'value1b']),
+                              ('other_option1', ['other_value1'])])
+
+    def test_multiple_to_single(self):
+        file_obj = self._to_memcache(fixture_path('git_config_multiple'))
+        with GitConfigParser(file_obj, read_only=False) as cw:
+            cw.set_value('section1', 'option1', 'value1c')
+
+            cw.write()
+            file_obj.seek(0)
+            cr = GitConfigParser(file_obj, read_only=True)
+            self.assertEqual(cr.get_value('section1', 'option1'), 'value1c')
+            self.assertEqual(cr.get_values('section1', 'option1'), ['value1c'])
+            self.assertEqual(cr.items('section1'),
+                             [('option1', 'value1c'),
+                              ('other_option1', 'other_value1')])
+            self.assertEqual(cr.items_all('section1'),
+                             [('option1', ['value1c']),
+                              ('other_option1', ['other_value1'])])
+
+    def test_single_to_multiple(self):
+        file_obj = self._to_memcache(fixture_path('git_config_multiple'))
+        with GitConfigParser(file_obj, read_only=False) as cw:
+            cw.add_value('section1', 'other_option1', 'other_value1a')
+
+            cw.write()
+            file_obj.seek(0)
+            cr = GitConfigParser(file_obj, read_only=True)
+            self.assertEqual(cr.get_value('section1', 'option1'), 'value1b')
+            self.assertEqual(cr.get_values('section1', 'option1'),
+                             ['value1a', 'value1b'])
+            self.assertEqual(cr.get_value('section1', 'other_option1'),
+                             'other_value1a')
+            self.assertEqual(cr.get_values('section1', 'other_option1'),
+                             ['other_value1', 'other_value1a'])
+            self.assertEqual(cr.items('section1'),
+                             [('option1', 'value1b'),
+                              ('other_option1', 'other_value1a')])
+            self.assertEqual(
+                cr.items_all('section1'),
+                [('option1', ['value1a', 'value1b']),
+                 ('other_option1', ['other_value1', 'other_value1a'])])
+
+    def test_add_to_multiple(self):
+        file_obj = self._to_memcache(fixture_path('git_config_multiple'))
+        with GitConfigParser(file_obj, read_only=False) as cw:
+            cw.add_value('section1', 'option1', 'value1c')
+            cw.write()
+            file_obj.seek(0)
+            cr = GitConfigParser(file_obj, read_only=True)
+            self.assertEqual(cr.get_value('section1', 'option1'), 'value1c')
+            self.assertEqual(cr.get_values('section1', 'option1'),
+                             ['value1a', 'value1b', 'value1c'])
+            self.assertEqual(cr.items('section1'),
+                             [('option1', 'value1c'),
+                              ('other_option1', 'other_value1')])
+            self.assertEqual(cr.items_all('section1'),
+                             [('option1', ['value1a', 'value1b', 'value1c']),
+                              ('other_option1', ['other_value1'])])