Rev 5509: Take review comments into account and drive-by fix bug #670251 in file:///home/vila/src/bzr/experimental/config/

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Nov 5 11:13:05 GMT 2010


At file:///home/vila/src/bzr/experimental/config/

------------------------------------------------------------
revno: 5509
revision-id: v.ladeuil+lp at free.fr-20101105111305-cvp1ov5zrp3oakux
parent: v.ladeuil+lp at free.fr-20101029173824-ocxeka2cc6smn2yp
fixes bug(s): https://launchpad.net/bugs/670251
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-active
timestamp: Fri 2010-11-05 12:13:05 +0100
message:
  Take review comments into account and drive-by fix bug #670251
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2010-10-29 17:38:24 +0000
+++ b/bzrlib/config.py	2010-11-05 11:13:05 +0000
@@ -1772,19 +1772,20 @@
 class cmd_config(commands.Command):
     __doc__ = """Display, set or remove a configuration option.
 
-    Display the MATCHING configuration options mentioning their scope (the
-    configuration file they are defined in). The active value that bzr will
-    take into account is the first one displayed.
-
-    To display *only* the active value, use the --active option.
+    Display the active value for a given option.
+
+    If --all is specified, NAME is interpreted as a regular expression and all
+    matching options are displayed mentioning their scope. The active value
+    that bzr will take into account is the first one displayed for each option.
+
+    If no NAME is given, --all .* is implied.
 
     Setting a value is achieved by using name=value without spaces. The value
     is set in the most relevant scope and can be checked by displaying the
     option again.
     """
 
-    aliases = ['conf']
-    takes_args = ['matching?']
+    takes_args = ['name?']
 
     takes_options = [
         'directory',
@@ -1793,38 +1794,41 @@
         commands.Option('scope', help='Reduce the scope to the specified'
                         ' configuration file',
                         type=unicode),
-        commands.Option('active', short_name='1',
-            help='Display the active value for the option.',
+        commands.Option('all',
+            help='Display all the defined values for the matching options.',
             ),
         commands.Option('remove', help='Remove the option from'
                         ' the configuration file'),
         ]
 
     @commands.display_command
-    def run(self, matching=None, directory=None, scope=None,
-            remove=False, active=False):
+    def run(self, name=None, all=False, directory=None, scope=None,
+            remove=False):
         if directory is None:
             directory = '.'
         directory = urlutils.normalize_url(directory)
-        if remove and active:
+        if remove and all:
             raise errors.BzrError(
-                '--active and --remove are mutually exclusive.')
+                '--all and --remove are mutually exclusive.')
         elif remove:
             # Delete the option in the given scope
-            self._remove_config_option(matching, directory, scope)
-        elif matching is None:
-            if active:
-                raise errors.BzrError(
-                    '--active expects an option to display its value.')
-            # Display all the options values
-            self._show_config('*', directory, scope, active)
+            self._remove_config_option(name, directory, scope)
+        elif name is None:
+            # Defaults to all options
+            self._show_matching_options('.*', directory, scope)
         else:
             try:
-                name, value = matching.split('=', 1)
+                name, value = name.split('=', 1)
             except ValueError:
                 # Display the option(s) value(s)
-                self._show_config(matching, directory, scope, active)
+                if all:
+                    self._show_matching_options(name, directory, scope)
+                else:
+                    self._show_value(name, directory, scope)
             else:
+                if all:
+                    raise errors.BzrError(
+                        'Only one option can be set.')
                 # Set the option value
                 self._set_config_option(name, value, directory, scope)
 
@@ -1853,29 +1857,34 @@
                 yield LocationConfig(directory)
                 yield GlobalConfig()
 
-    def _show_config(self, matching, directory, scope, active):
-        # Turn the glob into a regexp
-        matching_re = re.compile(fnmatch.translate(matching))
-        cur_conf_id = None
+    def _show_value(self, name, directory, scope):
         displayed = False
         for c in self._get_configs(directory, scope):
             if displayed:
                 break
-            for (name, value, section, conf_id) in c._get_options():
-                if matching_re.search(name):
-                    if not active and cur_conf_id != conf_id:
+            for (oname, value, section, conf_id) in c._get_options():
+                if name == oname:
+                    # Display only the first value and exit
+                    self.outf.write('%s\n' % (value))
+                    displayed = True
+                    break
+        if not displayed:
+            raise errors.NoSuchConfigOption(name)
+
+    def _show_matching_options(self, name, directory, scope):
+        name = re.compile(name)
+        # We want any error in the regexp to be raised *now* so we need to
+        # avoid the delay introduced by the lazy regexp.
+        name._compile_and_collapse()
+        cur_conf_id = None
+        for c in self._get_configs(directory, scope):
+            for (oname, value, section, conf_id) in c._get_options():
+                if name.search(oname):
+                    if cur_conf_id != conf_id:
                         # Explain where the options are defined
                         self.outf.write('%s:\n' % (conf_id,))
                         cur_conf_id = conf_id
-                    if active:
-                        # Display only the first value and exit
-                        self.outf.write('%s\n' % (value))
-                        displayed = True
-                        break
-                    else:
-                        self.outf.write('  %s = %s\n' % (name, value))
-        if active and not displayed:
-            raise errors.NoSuchConfigOption(matching)
+                    self.outf.write('  %s = %s\n' % (oname, value))
 
     def _set_config_option(self, name, value, directory, scope):
         for conf in self._get_configs(directory, scope):

=== modified file 'bzrlib/tests/blackbox/test_config.py'
--- a/bzrlib/tests/blackbox/test_config.py	2010-10-29 17:38:24 +0000
+++ b/bzrlib/tests/blackbox/test_config.py	2010-11-05 11:13:05 +0000
@@ -31,35 +31,41 @@
 
 class TestWithoutConfig(tests.TestCaseWithTransport):
 
-    def test_no_config(self):
+    def test_config_all(self):
         out, err = self.run_bzr(['config'])
         self.assertEquals('', out)
         self.assertEquals('', err)
 
-    def test_all_variables_no_config(self):
-        out, err = self.run_bzr(['config', '*'])
-        self.assertEquals('', out)
-        self.assertEquals('', err)
-
     def test_remove_unknown_option(self):
         self.run_bzr_error(['The "file" configuration option does not exist',],
                            ['config', '--remove', 'file'])
 
-    def test_active_remove_exclusive(self):
-        self.run_bzr_error(['--active and --remove are mutually exclusive.',],
-                           ['config', '--remove', '--active'])
+    def test_all_remove_exclusive(self):
+        self.run_bzr_error(['--all and --remove are mutually exclusive.',],
+                           ['config', '--remove', '--all'])
+
+    def test_all_set_exclusive(self):
+        self.run_bzr_error(['Only one option can be set.',],
+                           ['config', '--all', 'hello=world'])
 
     def test_remove_no_option(self):
         self.run_bzr_error(['--remove expects an option to remove.',],
                            ['config', '--remove'])
 
-    def test_active_no_option(self):
-        self.run_bzr_error(['--active expects an option to display.',],
-                           ['config', '--active'])
-
-    def test_active_unknown_option(self):
+    def test_unknown_option(self):
         self.run_bzr_error(['The "file" configuration option does not exist',],
-                           ['config', '--active', 'file'])
+                           ['config', 'file'])
+
+    def test_unexpected_regexp(self):
+        self.run_bzr_error(
+            ['The "\*file" configuration option does not exist',],
+            ['config', '*file'])
+
+    def test_wrong_regexp(self):
+        self.run_bzr_error(
+            ['Invalid pattern\(s\) found. "\*file" nothing to repeat',],
+            ['config', '--all', '*file'])
+
 
 
 class TestConfigDisplay(tests.TestCaseWithTransport):
@@ -96,6 +102,7 @@
               hello = world
             ''')
 
+
 class TestConfigActive(tests.TestCaseWithTransport):
 
     def setUp(self):
@@ -104,13 +111,13 @@
 
     def test_active_in_locations(self):
         script.run_script(self, '''\
-            $ bzr config -d tree --active file
+            $ bzr config -d tree file
             locations
             ''')
 
     def test_active_in_bazaar(self):
         script.run_script(self, '''\
-            $ bzr config -d tree --scope bazaar --active file
+            $ bzr config -d tree --scope bazaar file
             bazaar
             ''')
 
@@ -119,7 +126,7 @@
         # one
         script.run_script(self, '''\
             $ bzr config -d tree --remove file
-            $ bzr config -d tree --active file
+            $ bzr config -d tree file
             branch
             ''')
 
@@ -137,7 +144,7 @@
     def test_bazaar_config_outside_branch(self):
         script.run_script(self, '''\
             $ bzr config --scope bazaar hello=world
-            $ bzr config -d tree hello
+            $ bzr config -d tree --all hello
             bazaar:
               hello = world
             ''')
@@ -145,7 +152,7 @@
     def test_bazaar_config_inside_branch(self):
         script.run_script(self, '''\
             $ bzr config -d tree --scope bazaar hello=world
-            $ bzr config -d tree hello
+            $ bzr config -d tree --all hello
             bazaar:
               hello = world
             ''')
@@ -153,7 +160,7 @@
     def test_locations_config_inside_branch(self):
         script.run_script(self, '''\
             $ bzr config -d tree --scope locations hello=world
-            $ bzr config -d tree hello
+            $ bzr config -d tree --all hello
             locations:
               hello = world
             ''')
@@ -161,7 +168,7 @@
     def test_branch_config_default(self):
         script.run_script(self, '''\
             $ bzr config -d tree hello=world
-            $ bzr config -d tree hello
+            $ bzr config -d tree --all hello
             branch:
               hello = world
             ''')
@@ -169,7 +176,7 @@
     def test_branch_config_forcing_branch(self):
         script.run_script(self, '''\
             $ bzr config -d tree --scope branch hello=world
-            $ bzr config -d tree hello
+            $ bzr config -d tree --all hello
             branch:
               hello = world
             ''')
@@ -188,7 +195,7 @@
     def test_bazaar_config_outside_branch(self):
         script.run_script(self, '''\
             $ bzr config --scope bazaar --remove file
-            $ bzr config -d tree file
+            $ bzr config -d tree --all file
             locations:
               file = locations
             branch:
@@ -198,7 +205,7 @@
     def test_bazaar_config_inside_branch(self):
         script.run_script(self, '''\
             $ bzr config -d tree --scope bazaar --remove file
-            $ bzr config -d tree file
+            $ bzr config -d tree --all file
             locations:
               file = locations
             branch:
@@ -208,7 +215,7 @@
     def test_locations_config_inside_branch(self):
         script.run_script(self, '''\
             $ bzr config -d tree --scope locations --remove file
-            $ bzr config -d tree file
+            $ bzr config -d tree --all file
             branch:
               file = branch
             bazaar:
@@ -218,7 +225,7 @@
     def test_branch_config_default(self):
         script.run_script(self, '''\
             $ bzr config -d tree --remove file
-            $ bzr config -d tree file
+            $ bzr config -d tree --all file
             branch:
               file = branch
             bazaar:
@@ -226,7 +233,7 @@
             ''')
         script.run_script(self, '''\
             $ bzr config -d tree --remove file
-            $ bzr config -d tree file
+            $ bzr config -d tree --all file
             bazaar:
               file = bazaar
             ''')
@@ -234,7 +241,7 @@
     def test_branch_config_forcing_branch(self):
         script.run_script(self, '''\
             $ bzr config -d tree --scope branch --remove file
-            $ bzr config -d tree file
+            $ bzr config -d tree --all file
             locations:
               file = locations
             bazaar:
@@ -242,7 +249,7 @@
             ''')
         script.run_script(self, '''\
             $ bzr config -d tree --remove file
-            $ bzr config -d tree file
+            $ bzr config -d tree --all file
             bazaar:
               file = bazaar
             ''')

=== modified file 'doc/en/whats-new/whats-new-in-2.3.txt'
--- a/doc/en/whats-new/whats-new-in-2.3.txt	2010-10-29 16:53:22 +0000
+++ b/doc/en/whats-new/whats-new-in-2.3.txt	2010-11-05 11:13:05 +0000
@@ -123,11 +123,10 @@
 
 ``bzr`` can be configured via environment variables, command-line options
 and configurations files. We've started working on unifying this and give
-access to more options. The first step is a new ``bzr config`` command
-that can be used to display the active configuration options in the
-current working tree or branch as well as the ability to set or remove an
-option. Scripts can also use ``--active`` to get only the value for a
-given option.
+access to more options. The first step is a new ``bzr config`` command that
+can be used to display the active configuration options in the current
+working tree or branch as well as the ability to set or remove an
+option. Scripts can also use it to get only the value for a given option.
 
 Expected releases for the 2.3 series
 ************************************



More information about the bazaar-commits mailing list