Rev 6093: Disable list_values for config.Store, using a dedicated configobj object to trigger the string -> list conversion on-demand (via the option registration) only. in file:///home/vila/src/bzr/experimental/expand-in-stack/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/experimental/expand-in-stack/

------------------------------------------------------------
revno: 6093
revision-id: v.ladeuil+lp at free.fr-20110902143023-61h8n8cm3r96lov9
parent: v.ladeuil+lp at free.fr-20110902062132-dq9y0slcwe28op73
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: expand-in-stack
timestamp: Fri 2011-09-02 16:30:23 +0200
message:
  Disable list_values for config.Store, using a dedicated configobj object to trigger the string -> list conversion on-demand (via the option registration) only.
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-09-02 06:21:32 +0000
+++ b/bzrlib/config.py	2011-09-02 14:30:23 +0000
@@ -2422,18 +2422,28 @@
 
 
 def list_from_store(unicode_str):
+    if not isinstance(unicode_str, basestring):
+        raise TypeError
+    # Use a an empty dict to initialize an empty configobj avoiding all
+    # parsing and encoding checks
+    c = ConfigObj({}, encoding='utf-8')
+    # 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.
+    c._parse([u"list=%s" % (unicode_str,)])
+    co_list_or_not = c['list']
     # ConfigObj return '' instead of u''. Use 'str' below to catch all cases.
-    if isinstance(unicode_str, (str, unicode)):
-        if unicode_str:
+    if isinstance(co_list_or_not, basestring):
+        if co_list_or_not:
             # A single value, most probably the user forgot (or didn't care to
             # add) the final ','
-            l = [unicode_str]
+            l = [co_list_or_not]
         else:
             # The empty string, convert to empty list
             l = []
     else:
         # We rely on ConfigObj providing us with a list already
-        l = unicode_str
+        l = co_list_or_not
     return l
 
 
@@ -2705,7 +2715,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())

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-09-02 06:21:32 +0000
+++ b/bzrlib/tests/test_config.py	2011-09-02 14:30:23 +0000
@@ -2443,8 +2443,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()
@@ -2457,10 +2460,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):
@@ -2869,8 +2868,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])
@@ -3329,6 +3330,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):
 
@@ -3413,6 +3424,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))
 
@@ -3423,6 +3436,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))
 
@@ -3435,9 +3450,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 happends
+        # 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))