Rev 5568: Push down interpolation at the config level (make tests slightly less in file:///home/vila/src/bzr/experimental/config/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Thu Feb 17 14:40:41 UTC 2011
At file:///home/vila/src/bzr/experimental/config/
------------------------------------------------------------
revno: 5568
revision-id: v.ladeuil+lp at free.fr-20110217144041-qdvjc0n2uslgbax2
parent: v.ladeuil+lp at free.fr-20110217000711-cwuz605hul4r8929
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: interpolate
timestamp: Thu 2011-02-17 15:40:41 +0100
message:
Push down interpolation at the config level (make tests slightly less
precise but allows a clearer definition of the behavior).
- as soon as a list appears in one result, the whole interpolation result
becomes a list,
- get_user_option interpolates by default but gain an option to disable
the interpolation for special cases (mergetools, diff, etc) needing to
access templates.
- interpolate is also available at the Config level and as such usable
for all existing configs.
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2011-02-17 00:04:25 +0000
+++ b/bzrlib/config.py 2011-02-17 14:40:41 +0000
@@ -151,9 +151,50 @@
pass
return self[section][name]
+
+
+class Config(object):
+ """A configuration policy - what username, editor, gpg needs etc."""
+
+ def __init__(self):
+ super(Config, self).__init__()
+
+ def config_id(self):
+ """Returns a unique ID for the config."""
+ raise NotImplementedError(self.config_id)
+
+ def get_editor(self):
+ """Get the users pop up editor."""
+ raise NotImplementedError
+
+ def get_change_editor(self, old_tree, new_tree):
+ from bzrlib import diff
+ cmd = self._get_change_editor()
+ if cmd is None:
+ return None
+ return diff.DiffFromTool.from_string(cmd, old_tree, new_tree,
+ sys.stdout)
+
+
+ def get_mail_client(self):
+ """Get a mail client to use"""
+ selected_client = self.get_user_option('mail_client')
+ _registry = mail_client.mail_client_registry
+ try:
+ mail_client_class = _registry.get(selected_client)
+ except KeyError:
+ raise errors.UnknownMailClient(selected_client)
+ return mail_client_class(self)
+
+ def _get_signature_checking(self):
+ """Template method to override signature checking policy."""
+
+ def _get_signing_policy(self):
+ """Template method to override signature creation policy."""
+
option_ref_re = None
- def interpolate(self, string, env=None, _ref_stack=None):
+ def interpolate(self, string, env=None):
"""Interpolate the string in the configuration context.
:param string: The string to interpolate
@@ -165,7 +206,45 @@
"""
return self._interpolate_string(string, env)
+ def _interpolate_list(self, slist, env=None, _ref_stack=None):
+ """Interpolate 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 _ref_stack: Private list containing the options being
+ interpolated to detect loops.
+
+ :returns: The flatten list of interpolated strings.
+ """
+ # interpolate each value separately flattening lists
+ result = []
+ for s in slist:
+ value = self._interpolate_string(s, env, _ref_stack)
+ if isinstance(value, list):
+ result.extend(value)
+ else:
+ result.append(value)
+ return result
+
def _interpolate_string(self, string, env=None, _ref_stack=None):
+ """Interpolate the string in the configuration context.
+
+ :param string: The string to interpolate
+
+ :param env: An option dict defining additional configuration options or
+ overriding existing ones.
+
+ :param _ref_stack: Private list containing the options being
+ interpolated to detect loops.
+
+ :returns: The interpolated string.
+ """
+ if string is None:
+ # Not much to interpolate there
+ return None
if _ref_stack is None:
# What references are currently resolved (to detect loops)
_ref_stack = []
@@ -178,33 +257,50 @@
# We need to iterate until no more refs appear ({{foo}} will need two
# iterations for example).
while True:
- is_ref = False
- raw_chunks = self.option_ref_re.split(result)
+ try:
+ raw_chunks = self.option_ref_re.split(result)
+ except TypeError:
+ import pdb; pdb.set_trace()
if len(raw_chunks) == 1:
# 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 is_ref:
- chunks.append(chunk)
- is_ref = True
+ if not chunk_is_ref:
+ if chunk:
+ # Keep only non-empty strings
+ chunks.append(chunk)
+ chunk_is_ref = True
else:
name = chunk[1:-1]
if name in _ref_stack:
raise errors.InterpolationLoop(string, _ref_stack)
_ref_stack.append(name)
- try:
- value = self._interpolate_option(name, env, _ref_stack)
- except KeyError:
+ value = self._interpolate_option(name, env, _ref_stack)
+ if value is None:
raise errors.InterpolationUnknownOption(name, string)
- chunks.append(value)
+ if isinstance(value, list):
+ list_value = True
+ chunks.extend(value)
+ else:
+ chunks.append(value)
_ref_stack.pop()
- is_ref = False
- result = ''.join(chunks)
+ chunk_is_ref = False
+ if list_value:
+ # Once a list appears as the result of an interpolation, all
+ # callers will get a list result. This allows a consistent
+ # behavior even when some options in the interpolation chain
+ # may be seen defined as strings even if their interpolated
+ # value is a list.
+ return self._interpolate_list(chunks, env, _ref_stack)
+ else:
+ result = ''.join(chunks)
return result
- def _interpolate_option(self, name, env, ref_stack):
+ def _interpolate_option(self, name, env, _ref_stack):
if env is not None and name in env:
# Special case, values provided in env takes precedence over
# anything else
@@ -213,56 +309,27 @@
# FIXME: This is a limited implementation, what we really need
# is a way to query the bzr config for the value of an option,
# respecting the scope rules -- vila 20101222
- value = self[name]
- return self._interpolate_string(value, env, ref_stack)
-
-
-class Config(object):
- """A configuration policy - what username, editor, gpg needs etc."""
-
- def __init__(self):
- super(Config, self).__init__()
-
- def config_id(self):
- """Returns a unique ID for the config."""
- raise NotImplementedError(self.config_id)
-
- def get_editor(self):
- """Get the users pop up editor."""
- raise NotImplementedError
-
- def get_change_editor(self, old_tree, new_tree):
- from bzrlib import diff
- cmd = self._get_change_editor()
- if cmd is None:
- return None
- return diff.DiffFromTool.from_string(cmd, old_tree, new_tree,
- sys.stdout)
-
-
- def get_mail_client(self):
- """Get a mail client to use"""
- selected_client = self.get_user_option('mail_client')
- _registry = mail_client.mail_client_registry
- try:
- mail_client_class = _registry.get(selected_client)
- except KeyError:
- raise errors.UnknownMailClient(selected_client)
- return mail_client_class(self)
-
- def _get_signature_checking(self):
- """Template method to override signature checking policy."""
-
- def _get_signing_policy(self):
- """Template method to override signature creation policy."""
+ value = self.get_user_option(name, interpolate=False)
+ if isinstance(value, list):
+ value = self._interpolate_list(value, env, _ref_stack)
+ else:
+ value = self._interpolate_string(value, env, _ref_stack)
+ return value
def _get_user_option(self, option_name):
"""Template method to provide a user option."""
return None
- def get_user_option(self, option_name):
+ def get_user_option(self, option_name, interpolate=True):
"""Get a generic option - no special process, no default."""
- return self._get_user_option(option_name)
+ value = self._get_user_option(option_name)
+ if interpolate:
+ if isinstance(value, list):
+ value = self._interpolate_list(value)
+ else:
+ value = self._interpolate_string(value)
+ return value
+
def get_user_option_as_bool(self, option_name):
"""Get a generic option as a boolean - no special process, no default.
@@ -289,7 +356,8 @@
"""
l = self._get_user_option(option_name)
if isinstance(l, (str, unicode)):
- # A single value, most probably the user forgot the final ','
+ # A single value, most probably the user forgot (or didn't care to
+ # add) the final ','
l = [l]
return l
@@ -439,8 +507,9 @@
# be found in the known_merge_tools if it's not found in the config.
# This should be done through the proposed config defaults mechanism
# when it becomes available in the future.
- command_line = (self.get_user_option('bzr.mergetool.%s' % name) or
- mergetools.known_merge_tools.get(name, None))
+ command_line = (self.get_user_option('bzr.mergetool.%s' % name,
+ interpolate=False)
+ or mergetools.known_merge_tools.get(name, None))
return command_line
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2011-02-16 23:58:03 +0000
+++ b/bzrlib/tests/test_config.py 2011-02-17 14:40:41 +0000
@@ -338,69 +338,6 @@
self.fail('Error in config file not detected')
-
-class TestConfigObjInterpolation(tests.TestCase):
-
- def get_config(self, string=None):
- if string is None:
- string = ''
- string = StringIO(string.encode('utf-8'))
- c = config.ConfigObj(string, encoding='utf-8')
- return c
-
- def assertInterpolate(self, expected, conf, string, env=None):
- self.assertEquals(expected, conf.interpolate(string, env))
-
- def test_no_interpolation(self):
- c = self.get_config('')
- self.assertInterpolate('foo', c, 'foo')
-
- def test_env_adding_options(self):
- c = self.get_config('')
- self.assertInterpolate('bar', c, '{foo}', {'foo': 'bar'})
-
- def test_env_overriding_options(self):
- c = self.get_config('foo=baz')
- self.assertInterpolate('bar', c, '{foo}', {'foo': 'bar'})
-
- def test_simple_ref(self):
- c = self.get_config('foo=xxx')
- self.assertInterpolate('xxx', c, '{foo}')
-
- def test_unknown_ref(self):
- c = self.get_config('')
- self.assertRaises(errors.InterpolationUnknownOption,
- c.interpolate, '{foo}')
-
- def test_indirect_ref(self):
- c = self.get_config('''
-foo=xxx
-bar={foo}
-''')
- self.assertInterpolate('xxx', c, '{bar}')
-
- def test_embedded_ref(self):
- c = self.get_config('''
-foo=xxx
-bar=foo
-''')
- self.assertInterpolate('xxx', c, '{{bar}}')
-
- def test_simple_loop(self):
- c = self.get_config('foo={foo}')
- self.assertRaises(errors.InterpolationLoop, c.interpolate, '{foo}')
-
- def test_indirect_loop(self):
- c = self.get_config('''
-foo={bar}
-bar={baz}
-baz={foo}''')
- e = self.assertRaises(errors.InterpolationLoop,
- c.interpolate, '{foo}')
- self.assertEquals('foo->bar->baz', e.refs)
- self.assertEquals('{foo}', e.string)
-
-
class TestConfig(tests.TestCase):
def test_constructs(self):
@@ -577,6 +514,7 @@
' Use IniBasedConfig(_content=xxx) instead.'],
conf._get_parser, file=config_file)
+
class TestIniConfigSaving(tests.TestCaseInTempDir):
def test_cant_save_without_a_file_name(self):
@@ -590,6 +528,116 @@
self.assertFileEqual(content, 'test.conf')
+class TestIniConfigInterpolate(tests.TestCaseInTempDir):
+
+ def test_simple_ref(self):
+ conf = config.IniBasedConfig.from_string('foo={bar}\nbar=baz\n')
+ self.assertEquals('baz', conf.get_user_option('foo'))
+
+
+class TestIniConfigInterpolation(tests.TestCase):
+ """Test interpolation from the IniConfig level.
+
+ What we really want here is to test the Config level, but the class being
+ abstract as far as storing values is concerned, this can't be done
+ properly (yet).
+ """
+ # FIXME: This should be rewritten when all configs share a storage
+ # implementation.
+
+ def get_config(self, string=None):
+ if string is None:
+ string = ''
+ c = config.IniBasedConfig.from_string(string)
+ return c
+
+ def assertInterpolate(self, expected, conf, string, env=None):
+ self.assertEquals(expected, conf.interpolate(string, env))
+
+ def test_no_interpolation(self):
+ c = self.get_config('')
+ self.assertInterpolate('foo', c, 'foo')
+
+ def test_env_adding_options(self):
+ c = self.get_config('')
+ self.assertInterpolate('bar', c, '{foo}', {'foo': 'bar'})
+
+ def test_env_overriding_options(self):
+ c = self.get_config('foo=baz')
+ self.assertInterpolate('bar', c, '{foo}', {'foo': 'bar'})
+
+ def test_simple_ref(self):
+ c = self.get_config('foo=xxx')
+ self.assertInterpolate('xxx', c, '{foo}')
+
+ def test_unknown_ref(self):
+ c = self.get_config('')
+ self.assertRaises(errors.InterpolationUnknownOption,
+ c.interpolate, '{foo}')
+
+ def test_indirect_ref(self):
+ c = self.get_config('''
+foo=xxx
+bar={foo}
+''')
+ self.assertInterpolate('xxx', c, '{bar}')
+
+ def test_embedded_ref(self):
+ c = self.get_config('''
+foo=xxx
+bar=foo
+''')
+ self.assertInterpolate('xxx', c, '{{bar}}')
+
+ def test_simple_loop(self):
+ c = self.get_config('foo={foo}')
+ self.assertRaises(errors.InterpolationLoop, c.interpolate, '{foo}')
+
+ def test_indirect_loop(self):
+ c = self.get_config('''
+foo={bar}
+bar={baz}
+baz={foo}''')
+ e = self.assertRaises(errors.InterpolationLoop,
+ c.interpolate, '{foo}')
+ self.assertEquals('foo->bar->baz', e.refs)
+ self.assertEquals('{foo}', e.string)
+
+ def test_list(self):
+ conf = self.get_config('''
+foo=start
+bar=middle
+baz=end
+list={foo},{bar},{baz}
+''')
+ self.assertEquals(['start', 'middle', 'end'],
+ conf.get_user_option('list'))
+
+ def test_cascading_list(self):
+ conf = self.get_config('''
+foo=start,{bar}
+bar=middle,{baz}
+baz=end
+list={foo}
+''')
+ self.assertEquals(['start', 'middle', 'end'],
+ conf.get_user_option('list'))
+
+ def test_pathological_hidden_list(self):
+ conf = self.get_config('''
+foo=bin
+bar=go
+start={foo
+middle=},{
+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}'],
+ conf.get_user_option('hidden'))
+
+
class TestIniBaseConfigOnDisk(tests.TestCaseInTempDir):
def test_cannot_reload_without_name(self):
@@ -1048,7 +1096,7 @@
conf = self._get_empty_config()
cmdline = conf.find_merge_tool('kdiff3')
self.assertEquals('kdiff3 {base} {this} {other} -o {result}', cmdline)
-
+
def test_find_merge_tool_override_known(self):
conf = self._get_empty_config()
conf.set_user_option('bzr.mergetool.kdiff3', 'kdiff3 blah')
More information about the bazaar-commits
mailing list