Rev 5544: (vila) ``bzr config`` properly displays list values (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Nov 18 20:45:42 GMT 2010


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5544 [merge]
revision-id: pqm at pqm.ubuntu.com-20101118204540-un6e3t40melfjp9v
parent: pqm at pqm.ubuntu.com-20101118190918-33ysfcqtzq6g0mki
parent: v.ladeuil+lp at free.fr-20101118193828-d9erh6rqr7d25gp7
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2010-11-18 20:45:40 +0000
message:
  (vila) ``bzr config`` properly displays list values (Vincent Ladeuil)
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/tests/blackbox/test_config.py test_config.py-20100927150753-x6rf54uibd08r636-1
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
  doc/en/release-notes/bzr-2.3.txt NEWS-20050323055033-4e00b5db738777ff
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2010-11-17 11:45:46 +0000
+++ b/bzrlib/config.py	2010-11-17 15:51:10 +0000
@@ -496,7 +496,8 @@
         config_id = self.config_id()
         for (section_name, section) in sections:
             for (name, value) in section.iteritems():
-                yield (name, value, section_name, config_id)
+                yield (name, parser._quote(value), section_name,
+                       config_id, parser)
 
     def _get_option_policy(self, section, option_name):
         """Return the policy for the given (section, option_name) pair."""
@@ -1042,7 +1043,8 @@
         config_id = self.config_id()
         for (section_name, section) in sections:
             for (name, value) in section.iteritems():
-                yield (name, value, section_name, config_id)
+                yield (name, value, section_name,
+                       config_id, branch_config._get_parser())
         # Then the global options
         for option in self._get_global_config()._get_options():
             yield option
@@ -1862,14 +1864,18 @@
         for c in self._get_configs(directory, scope):
             if displayed:
                 break
-            for (oname, value, section, conf_id) in c._get_options():
+            for (oname, value, section, conf_id, parser) in c._get_options():
                 if name == oname:
                     # Display only the first value and exit
+
                     # FIXME: We need to use get_user_option to take policies
                     # into account and we need to make sure the option exists
-                    # too (hence the two for loops), this needs a better API --
-                    # vila 20101117
-                    self.outf.write('%s\n' % c.get_user_option(name))
+                    # too (hence the two for loops), this needs a better API
+                    # -- vila 20101117
+                    value = c.get_user_option(name)
+                    # Quote the value appropriately
+                    value = parser._quote(value)
+                    self.outf.write('%s\n' % (value,))
                     displayed = True
                     break
         if not displayed:
@@ -1883,7 +1889,7 @@
         cur_conf_id = None
         cur_section = None
         for c in self._get_configs(directory, scope):
-            for (oname, value, section, conf_id) in c._get_options():
+            for (oname, value, section, conf_id, parser) in c._get_options():
                 if name.search(oname):
                     if cur_conf_id != conf_id:
                         # Explain where the options are defined
@@ -1894,7 +1900,7 @@
                         and cur_section != section):
                         # Display the section if it's not the default (or only)
                         # one.
-                        self.outf.write('  [%s]\n' % section)
+                        self.outf.write('  [%s]\n' % (section,))
                         cur_section = section
                     self.outf.write('  %s = %s\n' % (oname, value))
 

=== modified file 'bzrlib/tests/blackbox/test_config.py'
--- a/bzrlib/tests/blackbox/test_config.py	2010-11-08 16:22:55 +0000
+++ b/bzrlib/tests/blackbox/test_config.py	2010-11-09 16:26:07 +0000
@@ -74,6 +74,40 @@
         super(TestConfigDisplay, self).setUp()
         _t_config.create_configs(self)
 
+    def test_multiline_all_values(self):
+        self.bazaar_config.set_user_option('multiline', '1\n2\n')
+        script.run_script(self, """\
+            $ bzr config -d tree
+            bazaar:
+              multiline = '''1
+            2
+            '''
+            """)
+
+    def test_multiline_value_only(self):
+        self.bazaar_config.set_user_option('multiline', '1\n2\n')
+        script.run_script(self, """\
+            $ bzr config -d tree multiline
+            '''1
+            2
+            '''
+            """)
+
+    def test_list_all_values(self):
+        self.bazaar_config.set_user_option('list', [1, 'a', 'with, a comma'])
+        script.run_script(self, '''\
+            $ bzr config -d tree
+            bazaar:
+              list = 1, a, "with, a comma"
+            ''')
+
+    def test_list_value_only(self):
+        self.bazaar_config.set_user_option('list', [1, 'a', 'with, a comma'])
+        script.run_script(self, '''\
+            $ bzr config -d tree list
+            1, a, "with, a comma"
+            ''')
+
     def test_bazaar_config(self):
         self.bazaar_config.set_user_option('hello', 'world')
         script.run_script(self, '''\

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2010-11-08 16:22:55 +0000
+++ b/bzrlib/tests/test_config.py	2010-11-17 15:52:03 +0000
@@ -21,6 +21,9 @@
 import sys
 import threading
 
+
+from testtools import matchers
+
 #import bzrlib specific imports here
 from bzrlib import (
     branch,
@@ -125,6 +128,49 @@
 """
 
 
+def create_configs(test):
+    """Create configuration files for a given test.
+
+    This requires creating a tree (and populate the ``test.tree`` attribute)
+    and its associated branch and will populate the following attributes:
+
+    - branch_config: A BranchConfig for the associated branch.
+
+    - locations_config : A LocationConfig for the associated branch
+
+    - bazaar_config: A GlobalConfig.
+
+    The tree and branch are created in a 'tree' subdirectory so the tests can
+    still use the test directory to stay outside of the branch.
+    """
+    tree = test.make_branch_and_tree('tree')
+    test.tree = tree
+    test.branch_config = config.BranchConfig(tree.branch)
+    test.locations_config = config.LocationConfig(tree.basedir)
+    test.bazaar_config = config.GlobalConfig()
+
+
+def create_configs_with_file_option(test):
+    """Create configuration files with a ``file`` option set in each.
+
+    This builds on ``create_configs`` and add one ``file`` option in each
+    configuration with a value which allows identifying the configuration file.
+    """
+    create_configs(test)
+    test.bazaar_config.set_user_option('file', 'bazaar')
+    test.locations_config.set_user_option('file', 'locations')
+    test.branch_config.set_user_option('file', 'branch')
+
+
+class TestOptionsMixin:
+
+    def assertOptions(self, expected, conf):
+        # We don't care about the parser (as it will make tests hard to write
+        # and error-prone anyway)
+        self.assertThat([opt[:4] for opt in conf._get_options()],
+                        matchers.Equals(expected))
+
+
 class InstrumentedConfigObj(object):
     """A config obj look-enough-alike to record calls made to it."""
 
@@ -902,7 +948,7 @@
         self.assertIs(None, new_config.get_alias('commit'))
 
 
-class TestLocationConfig(tests.TestCaseInTempDir):
+class TestLocationConfig(tests.TestCaseInTempDir, TestOptionsMixin):
 
     def test_constructs(self):
         my_config = config.LocationConfig('http://example.com')
@@ -1025,11 +1071,11 @@
 [/dir/subdir]
 other_url = /other-subdir
 """)
-        self.assertEqual(
+        self.assertOptions(
             [(u'other_url', u'/other-subdir', u'/dir/subdir', 'locations'),
              (u'other_url', u'/other-dir', u'/dir', 'locations'),
              (u'other_url:policy', u'appendpath', u'/dir', 'locations')],
-            list(self.my_location_config._get_options()))
+            self.my_location_config)
 
     def test_location_without_username(self):
         self.get_branch_config('http://www.example.com/ignoreparent')
@@ -1485,50 +1531,12 @@
         self.assertIs(None, bzrdir_config.get_default_stack_on())
 
 
-def create_configs(test):
-    """Create configuration files for a given test.
-
-    This requires creating a tree (and populate the ``test.tree`` attribute)
-    and its associated branch and will populate the following attributes:
-
-    - branch_config: A BranchConfig for the associated branch.
-
-    - locations_config : A LocationConfig for the associated branch
-
-    - bazaar_config: A GlobalConfig.
-
-    The tree and branch are created in a 'tree' subdirectory so the tests can
-    still use the test directory to stay outside of the branch.
-    """
-    tree = test.make_branch_and_tree('tree')
-    test.tree = tree
-    test.branch_config = config.BranchConfig(tree.branch)
-    test.locations_config = config.LocationConfig(tree.basedir)
-    test.bazaar_config = config.GlobalConfig()
-
-
-def create_configs_with_file_option(test):
-    """Create configuration files with a ``file`` option set in each.
-
-    This builds on ``create_configs`` and add one ``file`` option in each
-    configuration with a value which allows identifying the configuration file.
-    """
-    create_configs(test)
-    test.bazaar_config.set_user_option('file', 'bazaar')
-    test.locations_config.set_user_option('file', 'locations')
-    test.branch_config.set_user_option('file', 'branch')
-
-
-class TestConfigGetOptions(tests.TestCaseWithTransport):
+class TestConfigGetOptions(tests.TestCaseWithTransport, TestOptionsMixin):
 
     def setUp(self):
         super(TestConfigGetOptions, self).setUp()
         create_configs(self)
 
-    def assertOptions(self, expected, conf):
-        actual = list(conf._get_options())
-        self.assertEqual(expected, actual)
-
     # One variable in none of the above
     def test_no_variable(self):
         # Using branch should query branch, locations and bazaar
@@ -1577,16 +1585,12 @@
             self.branch_config)
 
 
-class TestConfigRemoveOption(tests.TestCaseWithTransport):
+class TestConfigRemoveOption(tests.TestCaseWithTransport, TestOptionsMixin):
 
     def setUp(self):
         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(

=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- a/doc/en/release-notes/bzr-2.3.txt	2010-11-18 19:09:18 +0000
+++ b/doc/en/release-notes/bzr-2.3.txt	2010-11-18 19:38:28 +0000
@@ -48,6 +48,8 @@
 * Better message if there is an error while setting ownership of
   ``.bazaar`` directory. (Parth Malwankar, #657553)
 
+* ``bzr config`` properly displays list values. (Vincent Ladeuil, #672382)
+
 * ``bzr config`` will now respect option policies when displaying the value
   and display the definition sections when appropriate.
   (Vincent Ladeuil, #671050)




More information about the bazaar-commits mailing list