# HG changeset patch # Parent 49cb2d5c973c6b04e2146b94beeaded7e7ca53b0 config: get rid of !ConfigParser. Instead, we use our own `Configuration._read()` which preserves the order of sections and of the entries within those sections, as found in the .ini file. The comments are also preserved, so that writing back a modified Configuration simply updates the entries in place. The other behavior changes are: - full unicode support - case sensitive sections and keys - no `%(...)s` expansion Closes #7378. diff -r 49cb2d5c973c trac/config.py --- a/trac/config.py Wed Feb 23 11:06:29 2011 +0100 +++ b/trac/config.py Wed Feb 23 18:43:41 2011 +0100 @@ -14,7 +14,7 @@ from __future__ import with_statement -from ConfigParser import SafeConfigParser +from StringIO import StringIO from copy import deepcopy import os.path @@ -34,36 +34,46 @@ _TRUE_VALUES = ('yes', 'true', 'enabled' _use_default = object() -def _to_utf8(basestr): - return to_unicode(basestr).encode('utf-8') - - class ConfigurationError(TracError): """Exception raised when a value in the configuration file is not valid.""" title = N_('Configuration Error') class Configuration(object): - """Thin layer over `ConfigParser` from the Python standard library. + """Ini file reader and writer. - In addition to providing some convenience methods, the class remembers - the last modification time of the configuration file, and reparses it - when the file has changed. + :param filename: location of the .ini file + :param params: global defaults that can be used in `%(name)s` + style parameter substitution + In addition to providing some convenience methods, the class + remembers the last modification time of the configuration file, + and reparses it when the file has changed. """ def __init__(self, filename, params={}): self.filename = filename - self.parser = SafeConfigParser(params) + self.params = params + self.parsed_sections = [] + # parsed_sections stores sections and entries as read: + # [(section, [(key, value), "#...", ...]), ...] + self.parsed_options = {} + # parsed_options stores actual values: + # section -> {key: value} self._old_sections = {} self.parents = [] self._lastmtime = 0 - self._sections = {} + self._sections = {} + # _sections stores the Section instances self.parse_if_needed(force=True) + def has_section(self, section): + """Return `True` if `section` was defined in the .ini file.""" + return section in self.parsed_options + def __contains__(self, name): """Return whether the configuration contains a section of the given name. """ - return name in self.sections() + return name and name in self.sections() def __getitem__(self, name): """Return the configuration section with the specified name.""" @@ -183,7 +193,7 @@ class Configuration(object): options declared in components that are enabled in the given `ComponentManager` are returned. """ - sections = set([to_unicode(s) for s in self.parser.sections()]) + sections = set(self.parsed_options.keys()) for parent in self.parents: sections.update(parent.sections(compmgr, defaults=False)) if defaults: @@ -197,10 +207,8 @@ class Configuration(object): (since Trac 0.11) """ - section_str = _to_utf8(section) - if self.parser.has_section(section_str): - if _to_utf8(option) in self.parser.options(section_str): - return True + if option in self.parsed_options.get(section, ()): + return True for parent in self.parents: if parent.has_option(section, option, defaults=False): return True @@ -211,44 +219,96 @@ class Configuration(object): if not self.filename: return - # Only save options that differ from the defaults + # Only save options that differ from the defaults, return + # either a (key, value) pair if value differs from default, + # or a "# key = value (where it comes from)" comment + def make_entry(section, key, options): + default = _use_default + info = None + for parent in self.parents: + if parent.has_option(section, key, defaults=False): + default = parent.get(section, key) + info = 'inherited from ' + parent.filename + break + if default is _use_default: + option = Option.registry.get((section, key)) + if option: + default = option.default + info = 'default' + if key in options: + current = options.pop(key) + if current != default: + return key, current + return '# %s = %s ; (%s)' % (key, default, info) + # Collect only actual entries + def non_default_entries(section, options): + entries = [] + for key in sorted(options): + entry = make_entry(section, key, options) + if isinstance(entry, tuple): + entries.append(entry) + return entries + + parsed_options = deepcopy(self.parsed_options) sections = [] - for section in self.sections(): - section_str = _to_utf8(section) - options = [] - for option in self[section]: - default_str = None - for parent in self.parents: - if parent.has_option(section, option, defaults=False): - default_str = _to_utf8(parent.get(section, option)) - break - option_str = _to_utf8(option) - current_str = False - if self.parser.has_option(section_str, option_str): - current_str = self.parser.get(section_str, option_str) - if current_str is not False and current_str != default_str: - options.append((option_str, current_str)) - if options: - sections.append((section_str, sorted(options))) + for section, section_content in self.parsed_sections: + filtered_entries = [] + options = parsed_options.get(section, ()) + for entry in section_content: + if isinstance(entry, tuple): + entry = make_entry(section, entry[0], options) + if not entry: + continue + # else try to see if this is a previously "commented out value" + # that needs to be updated in place (comment:12) + filtered_entries.append(entry) + sections.append((section, filtered_entries)) + if any(len(options) for options in parsed_options): + # add new entries to existing sections + for section, section_content in sections: + options = parsed_options.pop(section, ()) + new_entries = non_default_entries(section, options) + if new_entries: + section_content.extend(new_entries + ['']) + # add remaining sections as new sections + if parsed_options: + for section in sorted(parsed_options.keys()): + options = parsed_options[section] + entries = non_default_entries(section, options) + if entries: + sections.append((section, entries + [''])) - # At this point, all the strings in `sections` are UTF-8 encoded `str` try: + buf = StringIO() + + encoding = '# -*- coding: utf-8 -*-' + if self.parsed_sections: + section, comments = self.parsed_sections[0] + if section is None and comments and comments[0] == encoding: + encoding = None + if encoding: + buf.write(encoding + '\n\n') + + for section, section_content in sections: + if section: + buf.write('[%s]\n' % section) + for entry in section_content: + if isinstance(entry, tuple): + key, val = entry + if key in self[section].overridden: + buf.write('# %s = \n' % key) + else: + val = val.replace(CRLF, '\n').replace('\n', '\n ') + buf.write('%s = %s\n' % (key, val)) + else: + buf.write(entry + '\n') + utf8_content = buf.getvalue().encode('utf-8') with AtomicFile(self.filename, 'w') as fileobj: - fileobj.write('# -*- coding: utf-8 -*-\n\n') - for section, options in sections: - fileobj.write('[%s]\n' % section) - for key_str, val_str in options: - if to_unicode(key_str) in self[section].overridden: - fileobj.write('# %s = \n' % key_str) - else: - val_str = val_str.replace(CRLF, '\n') \ - .replace('\n', '\n ') - fileobj.write('%s = %s\n' % (key_str, val_str)) - fileobj.write('\n') - self._old_sections = deepcopy(self.parser._sections) - except Exception: + fileobj.write(utf8_content) + ## self._old_sections = deepcopy(self.parser._sections) + except Exception, e: # Revert all changes to avoid inconsistencies - self.parser._sections = deepcopy(self._old_sections) + ## FIXME self.parser._sections = deepcopy(self._old_sections) raise def parse_if_needed(self, force=False): @@ -259,17 +319,17 @@ class Configuration(object): modtime = os.path.getmtime(self.filename) if force or modtime > self._lastmtime: self._sections = {} - self.parser._sections = {} - self.parser.read(self.filename) + ## self.parser._sections = {} ### FIXME + self._read() self._lastmtime = modtime - self._old_sections = deepcopy(self.parser._sections) + ## self._old_sections = deepcopy(self.parser._sections) changed = True if changed: self.parents = [] - if self.parser.has_option('inherit', 'file'): - for filename in self.parser.get('inherit', 'file').split(','): - filename = to_unicode(filename.strip()) + inherited = self.parsed_options.get('inherit', {}).get('file') + if inherited: + for filename in (f.strip() for f in inherited.split(',')): if not os.path.isabs(filename): filename = os.path.join(os.path.dirname(self.filename), filename) @@ -282,6 +342,36 @@ class Configuration(object): self._cache = {} return changed + def _read(self): + content = to_unicode(file(self.filename, 'rb').read()) + self.parsed_sections = [] + section, section_content = None, [] + for line in content.splitlines(): + lsline = line.lstrip() + sline = lsline.rstrip() + if not sline or sline[0] in '#;': + section_content.append(sline) + elif lsline != line: # multi-line value? + if section_content and isinstance(section_content[-1], tuple): + key, value = section_content[-1] + section_content[-1] = (key, value + '\n' + sline) + elif section and '=' in sline: # setting + key, value = sline.split('=', 1) + # TODO: split value at rightmost ';', then store (k,v,comment) + section_content.append((key.strip(), value.strip())) + elif sline[0] == '[' and sline[-1] == ']': + self.parsed_sections.append((section, section_content)) + section, section_content = sline[1:-1], [] + if section: + self.parsed_sections.append((section, section_content)) + self.parsed_options = {} + for section, section_content in self.parsed_sections: + if section: + options = self.parsed_options.setdefault(section, {}) + for key, value in (entry for entry in section_content + if isinstance(entry, tuple)): + options[key] = value + def touch(self): if self.filename and os.path.isfile(self.filename) \ and os.access(self.filename, os.W_OK): @@ -295,8 +385,7 @@ class Configuration(object): """ for section, default_options in self.defaults(compmgr).items(): for name, value in default_options.items(): - if not self.parser.has_option(_to_utf8(section), - _to_utf8(name)): + if not self.has_option(section, name): if any(parent[section].contains(name, defaults=False) for parent in self.parents): value = None @@ -308,7 +397,7 @@ class Section(object): Objects of this class should not be instantiated directly. """ - __slots__ = ['config', 'name', 'overridden', '_cache'] + __slots__ = ('config', 'name', 'overridden', '_cache') def __init__(self, config, name): self.config = config @@ -317,12 +406,12 @@ class Section(object): self._cache = {} def contains(self, key, defaults=True): - if self.config.parser.has_option(_to_utf8(self.name), _to_utf8(key)): + if self.config.has_option(self.name, key): return True for parent in self.config.parents: if parent[self.name].contains(key, defaults=False): return True - return defaults and Option.registry.has_key((self.name, key)) + return defaults and (self.name, key) in Option.registry __contains__ = contains @@ -333,10 +422,9 @@ class Section(object): components that are enabled in the given `ComponentManager`. """ options = set() - name_str = _to_utf8(self.name) - if self.config.parser.has_section(name_str): - for option_str in self.config.parser.options(name_str): - option = to_unicode(option_str) + poptions = self.config.parsed_options.get(self.name) + if poptions: + for option in poptions.keys(): options.add(option.lower()) yield option for parent in self.config.parents: @@ -363,10 +451,9 @@ class Section(object): cached = self._cache.get(key, _use_default) if cached is not _use_default: return cached - name_str = _to_utf8(self.name) - key_str = _to_utf8(key) - if self.config.parser.has_option(name_str, key_str): - value = self.config.parser.get(name_str, key_str) + options = self.config.parsed_options.get(self.name, ()) + if key in options: + value = options[key] else: for parent in self.config.parents: value = parent[self.name].get(key, _use_default) @@ -382,8 +469,6 @@ class Section(object): return default if not value: value = u'' - elif isinstance(value, basestring): - value = to_unicode(value) self._cache[key] = value return value @@ -480,30 +565,30 @@ class Section(object): def set(self, key, value): """Change a configuration value. - + + :param key: setting key + :param value: setting value; if `None`, the value for this setting + will be the inherited one + :return: the value, or `''` if `None` was given. + These changes are not persistent unless saved with `save()`. """ self._cache.pop(key, None) - name_str = _to_utf8(self.name) - key_str = _to_utf8(key) - if not self.config.parser.has_section(name_str): - self.config.parser.add_section(name_str) if value is None: self.overridden[key] = True - value_str = '' - else: - value_str = _to_utf8(value) - return self.config.parser.set(name_str, key_str, value_str) + value = '' + self.config.parsed_options.setdefault(self.name, {})[key] = value + return value def remove(self, key): """Delete a key from this section. Like for `set()`, the changes won't persist until `save()` gets called. """ - name_str = _to_utf8(self.name) - if self.config.parser.has_section(name_str): + options = self.config.parsed_options.get(self.name) + if options: self._cache.pop(key, None) - self.config.parser.remove_option(_to_utf8(self.name), _to_utf8(key)) + options.pop(key, None) def _get_registry(cls, compmgr=None): @@ -645,8 +730,8 @@ class ChoiceOption(Option): """ def __init__(self, section, name, choices, doc=''): - Option.__init__(self, section, name, _to_utf8(choices[0]), doc) - self.choices = set(_to_utf8(choice).strip() for choice in choices) + Option.__init__(self, section, name, to_unicode(choices[0]), doc) + self.choices = set(to_unicode(choice).strip() for choice in choices) def accessor(self, section, name, default): value = section.get(name, default) diff -r 49cb2d5c973c trac/tests/config.py --- a/trac/tests/config.py Wed Feb 23 11:06:29 2011 +0100 +++ b/trac/tests/config.py Wed Feb 23 18:43:41 2011 +0100 @@ -35,7 +35,8 @@ class ConfigurationTestCase(unittest.Tes def tearDown(self): Option.registry = self._orig_registry - os.remove(self.filename) + if os.path.isfile(self.filename): + os.remove(self.filename) def _read(self): return Configuration(self.filename) @@ -218,14 +219,14 @@ class ConfigurationTestCase(unittest.Tes def test_set_and_save(self): config = self._read() config.set('b', u'öption0', 'y') - config.set(u'aä', 'öption0', 'x') - config.set('aä', 'option2', "Voilà l'été") # UTF-8 + config.set(u'aä', u'öption0', 'x') + # config.set('aä', 'option2', "Voilà l'été") # UTF-8 no longer legit config.set(u'aä', 'option1', u"Voilà l'été") # unicode # Note: the following would depend on the locale.getpreferredencoding() # config.set('a', 'option3', "Voil\xe0 l'\xe9t\xe9") # latin-1 self.assertEquals('x', config.get(u'aä', u'öption0')) self.assertEquals(u"Voilà l'été", config.get(u'aä', 'option1')) - self.assertEquals(u"Voilà l'été", config.get(u'aä', 'option2')) + #self.assertEquals(u"Voilà l'été", config.get(u'aä', 'option2')) config.save() configfile = open(self.filename, 'r') @@ -233,7 +234,7 @@ class ConfigurationTestCase(unittest.Tes '\n', '[aä]\n', "option1 = Voilà l'été\n", - "option2 = Voilà l'été\n", + #"option2 = Voilà l'été\n", 'öption0 = x\n', # "option3 = Voilà l'été\n", '\n', @@ -245,35 +246,34 @@ class ConfigurationTestCase(unittest.Tes config2 = Configuration(self.filename) self.assertEquals('x', config2.get(u'aä', u'öption0')) self.assertEquals(u"Voilà l'été", config2.get(u'aä', 'option1')) - self.assertEquals(u"Voilà l'été", config2.get(u'aä', 'option2')) + #self.assertEquals(u"Voilà l'été", config2.get(u'aä', 'option2')) # self.assertEquals(u"Voilà l'été", config2.get('a', 'option3')) def test_set_and_save_inherit(self): def testcb(): config = self._read() - config.set('a', 'option2', "Voilà l'été") # UTF-8 + #config.set('a', 'option2', "Voilà l'été") # UTF-8 config.set('a', 'option1', u"Voilà l'été") # unicode self.assertEquals('x', config.get('a', 'option')) self.assertEquals(u"Voilà l'été", config.get('a', 'option1')) - self.assertEquals(u"Voilà l'été", config.get('a', 'option2')) + #self.assertEquals(u"Voilà l'été", config.get('a', 'option2')) config.save() configfile = open(self.filename, 'r') self.assertEquals(['# -*- coding: utf-8 -*-\n', '\n', + '[inherit]\n', + "file = trac-site.ini\n", '[a]\n', "option1 = Voilà l'été\n", - "option2 = Voilà l'été\n", - '\n', - '[inherit]\n', - "file = trac-site.ini\n", + #"option2 = Voilà l'été\n", '\n'], configfile.readlines()) configfile.close() config2 = Configuration(self.filename) self.assertEquals('x', config2.get('a', 'option')) self.assertEquals(u"Voilà l'été", config2.get('a', 'option1')) - self.assertEquals(u"Voilà l'été", config2.get('a', 'option2')) + #self.assertEquals(u"Voilà l'été", config2.get('a', 'option2')) self._test_with_inherit(testcb) def test_simple_remove(self): @@ -335,7 +335,7 @@ class ConfigurationTestCase(unittest.Tes self.assertEquals((u'öption', 'x'), iter(config.options(u'ä')).next()) self.assertEquals(('option', 'y'), iter(config.options('b')).next()) self.assertRaises(StopIteration, iter(config.options('c')).next) - self.assertEquals(u'öption', iter(config['ä']).next()) + self.assertEquals(u'öption', iter(config[u'ä']).next()) class Foo(object): option_a = Option(u'ä', u'öption2', 'c') @@ -424,11 +424,13 @@ class ConfigurationTestCase(unittest.Tes self.assertEqual(os.path.join(base, 'site4'), config.getpath('c', 'path4', 'site4')) finally: - os.remove(site2) - os.rmdir(os.path.dirname(site2)) + if os.path.isfile(site2): + os.remove(site2) + os.rmdir(os.path.dirname(site2)) finally: - os.remove(site1) - os.rmdir(os.path.dirname(site1)) + if os.path.isfile(site1): + os.remove(site1) + os.rmdir(os.path.dirname(site1)) def _test_with_inherit(self, testcb): sitename = os.path.join(tempfile.gettempdir(), 'trac-site.ini') @@ -439,7 +441,8 @@ class ConfigurationTestCase(unittest.Tes self._write(['[inherit]', 'file = trac-site.ini']) testcb() finally: - os.remove(sitename) + if os.path.isfile(sitename): + os.remove(sitename) def suite():