Rev 5458: Implement ``bzr config --remove <option>``. in file:///home/vila/src/bzr/experimental/config/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Mon Oct 4 18:24:53 BST 2010
At file:///home/vila/src/bzr/experimental/config/
------------------------------------------------------------
revno: 5458
revision-id: v.ladeuil+lp at free.fr-20101004172452-unu266taysseifa4
parent: v.ladeuil+lp at free.fr-20101004140811-jsdrmydty0qsmhoi
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-modify
timestamp: Mon 2010-10-04 19:24:52 +0200
message:
Implement ``bzr config --remove <option>``.
* bzrlib/tests/test_config.py:
(TestConfigRemoveOption): Complete the tests.
* bzrlib/tests/blackbox/test_config.py:
(TestConfigRemoveOption.test_unknown_config): Test the cmd_config
option removal.
* bzrlib/config.py:
(IniBasedConfig.get_sections): Add the configuration id to the
returned tuples or we can't implement option removal for
BranchConfig.
(IniBasedConfig.remove_user_option): Default implementation.
(LockableConfig.remove_user_option): Implementation for lockable
configs.
(BranchConfig._get_branch_data_config): Force the id for the
branch data config to avoid forcing it in TreeConfig (oh my...).
(TreeConfig.remove_option): Use a different signature to avoid
confusion with remove_user_option (geez).
(TransportConfig.remove_option): The other side of
remove_user_option for BranchConfig.
(cmd_config._remove_config_option): The working implementation for
cmd config and two unsuccesful
approaches (BranchConfig/TreeConfig/TransportConfig is a mess when
it comes to provide precise access).
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2010-10-04 14:08:11 +0000
+++ b/bzrlib/config.py 2010-10-04 17:24:52 +0000
@@ -456,17 +456,17 @@
:param name: The section name. If None is supplied, the default
configurations are yielded.
- :return: A tuple (name, section) for all sections that will we walked
- by user_get_option() in the 'right' order. The first one is where
- set_user_option() will update the value.
+ :return: A tuple (name, section, config_id) for all sections that will
+ be walked by user_get_option() in the 'right' order. The first one
+ is where set_user_option() will update the value.
"""
parser = self._get_parser()
if name is not None:
- yield (name, parser[name])
+ yield (name, parser[name], self.id())
else:
# No section name has been given so we fallback to the configobj
# itself which holds the variables defined outside of any section.
- yield (None, parser)
+ yield (None, parser, self.id())
def get_options(self, sections=None):
"""Return an ordered list of (name, value, section, config_id) tuples.
@@ -590,6 +590,26 @@
def _get_nickname(self):
return self.get_user_option('nickname')
+ def remove_user_option(self, option_name, section_name=None):
+ """Remove a user option and save the configuration file.
+
+ :param option_name: The option to be removed.
+
+ :param section_name: The section the option is defined in, default to
+ the default section.
+ """
+ self.reload()
+ parser = self._get_parser()
+ if section_name is None:
+ section = parser
+ else:
+ section = parser[section_name]
+ try:
+ del section[option_name]
+ except KeyError:
+ raise errors.NoSuchConfigOption(option_name)
+ self._write_config_file()
+
def _write_config_file(self):
if self.file_name is None:
raise AssertionError('We cannot save, self.file_name is None')
@@ -658,6 +678,11 @@
def break_lock(self):
self._lock.break_lock()
+ @needs_write_lock
+ def remove_user_option(self, option_name, section_name=None):
+ super(LockableConfig, self).remove_user_option(option_name,
+ section_name)
+
def _write_config_file(self):
if self._lock is None or not self._lock.is_held:
# NB: if the following exception is raised it probably means a
@@ -733,8 +758,19 @@
# This could happen for an empty file where the DEFAULT section
# doesn't exist yet. So we force DEFAULT when yielding
name = 'DEFAULT'
- parser[name]= {}
- yield (name, parser[name])
+ if 'DEFAULT' not in parser:
+ parser['DEFAULT']= {}
+ yield (name, parser[name], self.id())
+
+ @needs_write_lock
+ def remove_user_option(self, option_name, section_name=None):
+ if section_name is None:
+ # We need to force the default section.
+ section_name = 'DEFAULT'
+ # We need to avoid the LockableConfig implementation or we'll lock
+ # twice
+ super(LockableConfig, self).remove_user_option(option_name,
+ section_name)
class LocationConfig(LockableConfig):
@@ -820,7 +856,7 @@
# the location path and we don't expose embedded sections either.
parser = self._get_parser()
for name, extra_path in self._get_matching_sections():
- yield (name, parser[name])
+ yield (name, parser[name], self.id())
def _get_option_policy(self, section, option_name):
"""Return the policy for the given (section, option_name) pair."""
@@ -912,6 +948,7 @@
def _get_branch_data_config(self):
if self._branch_data_config is None:
self._branch_data_config = TreeConfig(self.branch)
+ self._branch_data_config.id = self.id
return self._branch_data_config
def _get_location_config(self):
@@ -1033,6 +1070,9 @@
trace.warning('Value "%s" is masked by "%s" from'
' branch.conf', value, mask_value)
+ def remove_user_option(self, option_name, section_name=None):
+ self._get_branch_data_config().remove_option(option_name, section_name)
+
def _gpg_signing_command(self):
"""See Config.gpg_signing_command."""
return self._get_safe_value('_gpg_signing_command')
@@ -1196,12 +1236,23 @@
def set_option(self, value, name, section=None):
"""Set a per-branch configuration option"""
+ # FIXME: We shouldn't need to lock explicitly here but rather rely on
+ # higher levels providing the right lock -- vila 20101004
self.branch.lock_write()
try:
self._config.set_option(value, name, section)
finally:
self.branch.unlock()
+ def remove_option(self, option_name, section_name=None):
+ # FIXME: We shouldn't need to lock explicitly here but rather rely on
+ # higher levels providing the right lock -- vila 20101004
+ self.branch.lock_write()
+ try:
+ self._config.remove_option(option_name, section_name)
+ finally:
+ self.branch.unlock()
+
class AuthenticationConfig(object):
"""The authentication configuration file based on a ini file.
@@ -1690,6 +1741,14 @@
configobj.setdefault(section, {})[name] = value
self._set_configobj(configobj)
+ def remove_option(self, option_name, section_name=None):
+ configobj = self._get_configobj()
+ if section_name is None:
+ del configobj[option_name]
+ else:
+ del configobj[section_name][option_name]
+ self._set_configobj(configobj)
+
def _get_config_file(self):
try:
return StringIO(self._transport.get_bytes(self._filename))
@@ -1793,12 +1852,46 @@
def _remove_config_option(self, name, directory, force):
removed = False
for conf in self._get_configs(directory, force):
- # We use the first section in the first config to remove the option
- for section in conf.get_sections():
- if name in section:
- raise NotImplementeErro(self._remove_config_option)
- del section[name]
- conf._write_config_file()
+ for (section_name, section, conf_id) in conf.get_sections():
+ if force is not None and conf_id != force:
+ # Not the right configuration file
+ continue
+ if name in section:
+ if conf_id != conf.id():
+ conf = self._get_configs(directory, conf_id).next()
+ # We use the first section in the first config where the
+ # option is defined to remove it
+ conf.remove_user_option(name, section_name)
+ removed = True
+ break
+ break
+ else:
+ raise errors.NoSuchConfig(force)
+ if not removed:
+ raise errors.NoSuchConfigOption(name)
+
+ def x_remove_config_option(self, name, directory, force):
+ removed = False
+ for conf in self._get_configs(directory, force):
+ trace.mutter('conf: %r' % (conf,))
+ # We use the first config to remove the option
+ conf.remove_user_option(name)
+ break
+ else:
+ raise errors.NoSuchConfig(force)
+ if not removed:
+ raise errors.NoSuchConfigOption(name)
+
+ def xx_remove_config_option(self, name, directory, force):
+ removed = False
+ for conf in self._get_configs(directory, force):
+ trace.mutter('conf: %r' % (conf,))
+ for (section_name, section, conf_id) in conf.get_sections():
+ trace.mutter('section: %s, %r' % (section_name, section))
+ if name in section:
+ # We use the first section in the first config where the
+ # option is defined to remove it
+ conf.remove_user_option(name, section_name)
removed = True
break
break
=== modified file 'bzrlib/tests/blackbox/test_config.py'
--- a/bzrlib/tests/blackbox/test_config.py 2010-10-04 14:08:11 +0000
+++ b/bzrlib/tests/blackbox/test_config.py 2010-10-04 17:24:52 +0000
@@ -41,6 +41,9 @@
self.assertEquals('', out)
self.assertEquals('', err)
+ def test_unknown_option(self):
+ self.run_bzr_error(['The "file" configuration option does not exist',],
+ ['config', '--remove', 'file'])
class TestConfigDisplay(tests.TestCaseWithTransport):
@@ -77,10 +80,10 @@
''')
-class TestConfigSet(tests.TestCaseWithTransport):
+class TestConfigSetOption(tests.TestCaseWithTransport):
def setUp(self):
- super(TestConfigSet, self).setUp()
+ super(TestConfigSetOption, self).setUp()
_t_config.create_configs(self)
def test_unknown_config(self):
@@ -128,12 +131,74 @@
''')
-class TestConfigRemove(tests.TestCaseWithTransport):
+class TestConfigRemoveOption(tests.TestCaseWithTransport):
def setUp(self):
- super(TestConfigRemove, self).setUp()
+ super(TestConfigRemoveOption, self).setUp()
_t_config.create_configs_with_file_option(self)
- def test_unknown_option(self):
- self.run_bzr_error(['The "file" configuration option does not exist',],
- ['config', '--remove', 'file'])
+ def test_unknown_config(self):
+ self.run_bzr_error(['The "moon" configuration does not exist'],
+ ['config', '--force', 'moon', '--remove', 'file'])
+
+ def test_bazaar_config_outside_branch(self):
+ script.run_script(self, '''
+$ bzr config --force bazaar --remove file
+$ bzr config -d tree file
+locations:
+ file = locations
+branch:
+ file = branch
+''')
+
+ def test_bazaar_config_inside_branch(self):
+ script.run_script(self, '''
+$ bzr config -d tree --force bazaar --remove file
+$ bzr config -d tree file
+locations:
+ file = locations
+branch:
+ file = branch
+''')
+
+ def test_locations_config_inside_branch(self):
+ script.run_script(self, '''
+$ bzr config -d tree --force locations --remove file
+$ bzr config -d tree file
+branch:
+ file = branch
+bazaar:
+ file = bazaar
+''')
+
+ def test_branch_config_default(self):
+ script.run_script(self, '''
+$ bzr config -d tree --remove file
+$ bzr config -d tree file
+branch:
+ file = branch
+bazaar:
+ file = bazaar
+''')
+ script.run_script(self, '''
+$ bzr config -d tree --remove file
+$ bzr config -d tree file
+bazaar:
+ file = bazaar
+''')
+
+ def test_branch_config_forcing_branch(self):
+ script.run_script(self, '''
+$ bzr config -d tree --force branch --remove file
+$ bzr config -d tree file
+locations:
+ file = locations
+bazaar:
+ file = bazaar
+''')
+ script.run_script(self, '''
+$ bzr config -d tree --remove file
+$ bzr config -d tree file
+bazaar:
+ file = bazaar
+''')
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2010-10-04 08:42:17 +0000
+++ b/bzrlib/tests/test_config.py 2010-10-04 17:24:52 +0000
@@ -1515,7 +1515,7 @@
actual = list(conf.get_options())
self.assertEqual(expected, actual)
- # One variable in non of the above
+ # One variable in none of the above
def test_no_variable(self):
# Using branch should query branch, locations and bazaar
self.assertOptions([], self.branch_config)
@@ -1569,6 +1569,31 @@
super(TestConfigRemoveOption, self).setUp()
create_configs_with_file_option(self)
+ def assertOptions(self, expected, conf):
+ actual = list(conf.get_options())
+ self.assertEqual(expected, actual)
+
+ def test_remove_in_locations(self):
+ self.locations_config.remove_user_option('file', self.tree.basedir)
+ self.assertOptions(
+ [('file', 'branch', 'DEFAULT', 'branch'),
+ ('file', 'bazaar', 'DEFAULT', 'bazaar'),],
+ self.branch_config)
+
+ def test_remove_in_branch(self):
+ self.branch_config.remove_user_option('file')
+ self.assertOptions(
+ [('file', 'locations', self.tree.basedir, 'locations'),
+ ('file', 'bazaar', 'DEFAULT', 'bazaar'),],
+ self.branch_config)
+
+ def test_remove_in_bazaar(self):
+ self.bazaar_config.remove_user_option('file')
+ self.assertOptions(
+ [('file', 'locations', self.tree.basedir, 'locations'),
+ ('file', 'branch', 'DEFAULT', 'branch'),],
+ self.branch_config)
+
class TestConfigGetSections(tests.TestCaseWithTransport):
More information about the bazaar-commits
mailing list