From f7bbe6b01c82d9bcb3333b07bae0c9755eecdbbf Mon Sep 17 00:00:00 2001 From: "A. Jesse Jiryu Davis" Date: Sun, 20 Jan 2019 22:19:00 -0500 Subject: [PATCH 1/3] Support multiple git config values per option Solves #717 --- AUTHORS | 1 + git/config.py | 139 ++++++++++++++++++++++++-- git/test/fixtures/git_config_multiple | 7 ++ git/test/test_config.py | 97 ++++++++++++++++++ 4 files changed, 234 insertions(+), 10 deletions(-) create mode 100644 git/test/fixtures/git_config_multiple diff --git a/AUTHORS b/AUTHORS index 2e006ba23..87f2aa63e 100644 --- a/AUTHORS +++ b/AUTHORS @@ -28,5 +28,6 @@ Contributors are: -Yaroslav Halchenko -Tim Swast -William Luc Ritchie +-A. Jesse Jiryu Davis Portions derived from other open source works and are clearly marked. diff --git a/git/config.py b/git/config.py index 68d65ae91..837180924 100644 --- a/git/config.py +++ b/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 diff --git a/git/test/fixtures/git_config_multiple b/git/test/fixtures/git_config_multiple new file mode 100644 index 000000000..03a975680 --- /dev/null +++ b/git/test/fixtures/git_config_multiple @@ -0,0 +1,7 @@ +[section0] + option0 = value0 + +[section1] + option1 = value1a + option1 = value1b + other_option1 = other_value1 diff --git a/git/test/test_config.py b/git/test/test_config.py index 4d6c82363..67e4810d0 100644 --- a/git/test/test_config.py +++ b/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'])]) From a26349d8df88107bd59fd69c06114d3b213d0b27 Mon Sep 17 00:00:00 2001 From: "A. Jesse Jiryu Davis" Date: Sun, 20 Jan 2019 23:00:15 -0500 Subject: [PATCH 2/3] Python 3 compatibility #717 --- git/config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git/config.py b/git/config.py index 837180924..e2c055fd5 100644 --- a/git/config.py +++ b/git/config.py @@ -523,7 +523,8 @@ def items_all(self, section_name): rv[k].extend(v) - return rv.items() + # For consistency with items(), return a list, even in Python 3 + return list(rv.items()) @needs_values def write(self): From 4106f183ad0875734ba2c697570f9fd272970804 Mon Sep 17 00:00:00 2001 From: "A. Jesse Jiryu Davis" Date: Mon, 21 Jan 2019 09:38:51 -0500 Subject: [PATCH 3/3] Use items and items_all correctly #717 --- git/config.py | 35 ++++++++++++++++------------------- git/test/test_config.py | 12 +++++++++++- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/git/config.py b/git/config.py index e2c055fd5..b03d9d42e 100644 --- a/git/config.py +++ b/git/config.py @@ -176,11 +176,19 @@ def setlast(self, key, value): prior = super(_OMD, self).__getitem__(key) prior[-1] = value + def get(self, key, default=None): + return super(_OMD, self).get(key, [default])[-1] + def getall(self, key): return super(_OMD, self).__getitem__(key) + def items(self): + """List of (key, last value for key).""" + return [(k, self[k]) for k in self] + def items_all(self): - return [(k, self.get(k)) for k in self] + """List of (key, list of values for key).""" + return [(k, self.getall(k)) for k in self] class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, object)): @@ -481,14 +489,9 @@ 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_all(): + for (key, values) 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)) @@ -506,25 +509,19 @@ def items(self, section_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] + rv = _OMD(self._defaults) - for k, v in self._sections[section_name].items_all(): + for k, vs in self._sections[section_name].items_all(): if k == '__name__': continue - if k not in rv: - rv[k] = v - continue - - if rv[k] == v: + if k in rv and rv.getall(k) == vs: continue - rv[k].extend(v) + for v in vs: + rv.add(k, v) - # For consistency with items(), return a list, even in Python 3 - return list(rv.items()) + return rv.items_all() @needs_values def write(self): diff --git a/git/test/test_config.py b/git/test/test_config.py index 67e4810d0..93f94748a 100644 --- a/git/test/test_config.py +++ b/git/test/test_config.py @@ -11,7 +11,7 @@ GitConfigParser ) from git.compat import string_types -from git.config import cp +from git.config import _OMD, cp from git.test.lib import ( TestCase, fixture_path, @@ -362,3 +362,13 @@ def test_add_to_multiple(self): self.assertEqual(cr.items_all('section1'), [('option1', ['value1a', 'value1b', 'value1c']), ('other_option1', ['other_value1'])]) + + def test_setlast(self): + # Test directly, not covered by higher-level tests. + omd = _OMD() + omd.setlast('key', 'value1') + self.assertEqual(omd['key'], 'value1') + self.assertEqual(omd.getall('key'), ['value1']) + omd.setlast('key', 'value2') + self.assertEqual(omd['key'], 'value2') + self.assertEqual(omd.getall('key'), ['value2'])