Rev 6130: (vila) Simplify option expansion. (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Tue Sep 6 11:53:51 UTC 2011
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 6130 [merge]
revision-id: pqm at pqm.ubuntu.com-20110906115348-itl2uhj34oitw3as
parent: pqm at pqm.ubuntu.com-20110906110130-8e1m8kc6h1hlt6t5
parent: v.ladeuil+lp at free.fr-20110906105055-lcomuvwi01hsnyru
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2011-09-06 11:53:48 +0000
message:
(vila) Simplify option expansion. (Vincent Ladeuil)
modified:
bzrlib/config.py config.py-20051011043216-070c74f4e9e338e8
bzrlib/tests/test_config.py testconfig.py-20051011041908-742d0c15d8d8c8eb
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2011-09-02 08:34:51 +0000
+++ b/bzrlib/config.py 2011-09-06 11:53:48 +0000
@@ -2426,19 +2426,33 @@
return int(unicode_str)
+# Use a an empty dict to initialize an empty configobj avoiding all
+# parsing and encoding checks
+_list_converter_config = configobj.ConfigObj(
+ {}, encoding='utf-8', list_values=True, interpolation=False)
+
+
def list_from_store(unicode_str):
+ if not isinstance(unicode_str, basestring):
+ raise TypeError
+ # Now inject our string directly as unicode. All callers got their value
+ # from configobj, so values that need to be quoted are already properly
+ # quoted.
+ _list_converter_config.reset()
+ _list_converter_config._parse([u"list=%s" % (unicode_str,)])
+ maybe_list = _list_converter_config['list']
# ConfigObj return '' instead of u''. Use 'str' below to catch all cases.
- if isinstance(unicode_str, (str, unicode)):
- if unicode_str:
+ if isinstance(maybe_list, basestring):
+ if maybe_list:
# A single value, most probably the user forgot (or didn't care to
# add) the final ','
- l = [unicode_str]
+ l = [maybe_list]
else:
# The empty string, convert to empty list
l = []
else:
# We rely on ConfigObj providing us with a list already
- l = unicode_str
+ l = maybe_list
return l
@@ -2715,7 +2729,8 @@
co_input = StringIO(bytes)
try:
# The config files are always stored utf8-encoded
- self._config_obj = ConfigObj(co_input, encoding='utf-8')
+ self._config_obj = ConfigObj(co_input, encoding='utf-8',
+ list_values=False)
except configobj.ConfigObjError, e:
self._config_obj = None
raise errors.ParseConfigError(e.errors, self.external_url())
@@ -3047,35 +3062,29 @@
except KeyError:
# Not registered
opt = None
+ def expand_and_convert(val):
+ # This may need to be called twice if the value is None or ends up
+ # being None during expansion or conversion.
+ if val is not None:
+ if expand:
+ if isinstance(val, basestring):
+ val = self._expand_options_in_string(val)
+ else:
+ trace.warning('Cannot expand "%s":'
+ ' %s does not support option expansion'
+ % (name, type(val)))
+ if opt is not None:
+ val = opt.convert_from_unicode(val)
+ return val
+ value = expand_and_convert(value)
if opt is not None and value is None:
# If the option is registered, it may provide a default value
value = opt.get_default()
- if expand:
- value = self._expand_option_value(value)
- if opt is not None and value is not None:
- value = opt.convert_from_unicode(value)
- if value is None:
- # The conversion failed, fallback to the default value
- value = opt.get_default()
- if expand:
- value = self._expand_option_value(value)
- value = opt.convert_from_unicode(value)
+ value = expand_and_convert(value)
for hook in ConfigHooks['get']:
hook(self, name, value)
return value
- def _expand_option_value(self, value):
- """Expand the option value depending on its type."""
- if isinstance(value, list):
- value = self._expand_options_in_list(value)
- elif isinstance(value, dict):
- trace.warning('Cannot expand "%s":'
- ' Dicts do not support option expansion'
- % (name,))
- elif isinstance(value, (str, unicode)):
- value = self._expand_options_in_string(value)
- return value
-
def expand_options(self, string, env=None):
"""Expand option references in the string in the configuration context.
@@ -3088,29 +3097,6 @@
"""
return self._expand_options_in_string(string, env)
- def _expand_options_in_list(self, slist, env=None, _refs=None):
- """Expand options in a list of strings in the configuration context.
-
- :param slist: A list of strings.
-
- :param env: An option dict defining additional configuration options or
- overriding existing ones.
-
- :param _refs: Private list (FIFO) containing the options being
- expanded to detect loops.
-
- :returns: The flatten list of expanded strings.
- """
- # expand options in each value separately flattening lists
- result = []
- for s in slist:
- value = self._expand_options_in_string(s, env, _refs)
- if isinstance(value, list):
- result.extend(value)
- else:
- result.append(value)
- return result
-
def _expand_options_in_string(self, string, env=None, _refs=None):
"""Expand options in the string in the configuration context.
@@ -3139,15 +3125,11 @@
# Shorcut the trivial case: no refs
return result
chunks = []
- list_value = False
# Split will isolate refs so that every other chunk is a ref
chunk_is_ref = False
for chunk in raw_chunks:
if not chunk_is_ref:
- if chunk:
- # Keep only non-empty strings (or we get bogus empty
- # slots when a list value is involved).
- chunks.append(chunk)
+ chunks.append(chunk)
chunk_is_ref = True
else:
name = chunk[1:-1]
@@ -3157,22 +3139,10 @@
value = self._expand_option(name, env, _refs)
if value is None:
raise errors.ExpandingUnknownOption(name, string)
- if isinstance(value, list):
- list_value = True
- chunks.extend(value)
- else:
- chunks.append(value)
+ chunks.append(value)
_refs.pop()
chunk_is_ref = False
- if list_value:
- # Once a list appears as the result of an expansion, all
- # callers will get a list result. This allows a consistent
- # behavior even when some options in the expansion chain
- # defined as strings (no comma in their value) but their
- # expanded value is a list.
- return self._expand_options_in_list(chunks, env, _refs)
- else:
- result = ''.join(chunks)
+ result = ''.join(chunks)
return result
def _expand_option(self, name, env, _refs):
@@ -3187,10 +3157,7 @@
# configs, getting the option value should restart from the top
# config, not the current one) -- vila 20101222
value = self.get(name, expand=False)
- if isinstance(value, list):
- value = self._expand_options_in_list(value, env, _refs)
- else:
- value = self._expand_options_in_string(value, env, _refs)
+ value = self._expand_options_in_string(value, env, _refs)
return value
def _get_mutable_section(self):
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2011-09-02 08:34:51 +0000
+++ b/bzrlib/tests/test_config.py 2011-09-06 11:53:48 +0000
@@ -2462,8 +2462,11 @@
from_unicode=config.list_from_store)
def test_convert_invalid(self):
+ opt = self.get_option()
+ # We don't even try to convert a list into a list, we only expect
+ # strings
+ self.assertConvertInvalid(opt, [1])
# No string is invalid as all forms can be converted to a list
- pass
def test_convert_valid(self):
opt = self.get_option()
@@ -2476,10 +2479,6 @@
self.assertConverted([u'42'], opt, u'42')
# A single string
self.assertConverted([u'bar'], opt, u'bar')
- # A list remains a list (configObj will turn a string containing commas
- # into a list, but that's not what we're testing here)
- self.assertConverted([u'foo', u'1', u'True'],
- opt, [u'foo', u'1', u'True'])
class TestOptionRegistry(tests.TestCase):
@@ -2660,7 +2659,7 @@
class TestIniFileStoreContent(tests.TestCaseWithTransport):
- """Simulate loading a config store without content of various encodings.
+ """Simulate loading a config store with content of various encodings.
All files produced by bzr are in utf8 content.
@@ -2719,7 +2718,7 @@
class TestIniConfigContent(tests.TestCaseWithTransport):
- """Simulate loading a IniBasedConfig without content of various encodings.
+ """Simulate loading a IniBasedConfig with content of various encodings.
All files produced by bzr are in utf8 content.
@@ -2907,8 +2906,10 @@
sections = list(store.get_sections())
self.assertLength(4, sections)
# The default section has no name.
- # List values are provided as lists
- self.assertSectionContent((None, {'foo': 'bar', 'l': ['1', '2']}),
+ # List values are provided as strings and need to be explicitly
+ # converted by specifying from_unicode=list_from_store at option
+ # registration
+ self.assertSectionContent((None, {'foo': 'bar', 'l': u'1,2'}),
sections[0])
self.assertSectionContent(
('DEFAULT', {'foo_in_DEFAULT': 'foo_DEFAULT'}), sections[1])
@@ -3367,6 +3368,16 @@
self.conf.store._load_from_string('foo=m,o,r,e')
self.assertEquals(['m', 'o', 'r', 'e'], self.conf.get('foo'))
+ def test_get_with_list_converter_embedded_spaces_many_items(self):
+ self.register_list_option('foo', None)
+ self.conf.store._load_from_string('foo=" bar", "baz "')
+ self.assertEquals([' bar', 'baz '], self.conf.get('foo'))
+
+ def test_get_with_list_converter_stripped_spaces_many_items(self):
+ self.register_list_option('foo', None)
+ self.conf.store._load_from_string('foo= bar , baz ')
+ self.assertEquals(['bar', 'baz'], self.conf.get('foo'))
+
class TestStackExpandOptions(tests.TestCaseWithTransport):
@@ -3451,6 +3462,8 @@
baz=end
list={foo},{bar},{baz}
''')
+ self.registry.register(
+ config.Option('list', from_unicode=config.list_from_store))
self.assertEquals(['start', 'middle', 'end'],
self.conf.get('list', expand=True))
@@ -3461,6 +3474,8 @@
baz=end
list={foo}
''')
+ self.registry.register(
+ config.Option('list', from_unicode=config.list_from_store))
self.assertEquals(['start', 'middle', 'end'],
self.conf.get('list', expand=True))
@@ -3473,9 +3488,11 @@
end=bar}
hidden={start}{middle}{end}
''')
- # Nope, it's either a string or a list, and the list wins as soon as a
- # ',' appears, so the string concatenation never occur.
- self.assertEquals(['{foo', '}', '{', 'bar}'],
+ # What matters is what the registration says, the conversion happens
+ # only after all expansions have been performed
+ self.registry.register(
+ config.Option('hidden', from_unicode=config.list_from_store))
+ self.assertEquals(['bin', 'go'],
self.conf.get('hidden', expand=True))
More information about the bazaar-commits
mailing list