Rev 6130: (vila) Simplify option expansion. (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Tue Sep 6 11:53:51 UTC 2011


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

------------------------------------------------------------
revno: 6130 [merge]
revision-id: pqm at pqm.ubuntu.com-20110906115348-itl2uhj34oitw3as
parent: pqm at pqm.ubuntu.com-20110906110130-8e1m8kc6h1hlt6t5
parent: v.ladeuil+lp at free.fr-20110906105055-lcomuvwi01hsnyru
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2011-09-06 11:53:48 +0000
message:
  (vila) Simplify option expansion. (Vincent Ladeuil)
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-09-02 08:34:51 +0000
+++ b/bzrlib/config.py	2011-09-06 11:53:48 +0000
@@ -2426,19 +2426,33 @@
     return int(unicode_str)
 
 
+# Use a an empty dict to initialize an empty configobj avoiding all
+# parsing and encoding checks
+_list_converter_config = configobj.ConfigObj(
+    {}, encoding='utf-8', list_values=True, interpolation=False)
+
+
 def list_from_store(unicode_str):
+    if not isinstance(unicode_str, basestring):
+        raise TypeError
+    # 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.
+    _list_converter_config.reset()
+    _list_converter_config._parse([u"list=%s" % (unicode_str,)])
+    maybe_list = _list_converter_config['list']
     # ConfigObj return '' instead of u''. Use 'str' below to catch all cases.
-    if isinstance(unicode_str, (str, unicode)):
-        if unicode_str:
+    if isinstance(maybe_list, basestring):
+        if maybe_list:
             # A single value, most probably the user forgot (or didn't care to
             # add) the final ','
-            l = [unicode_str]
+            l = [maybe_list]
         else:
             # The empty string, convert to empty list
             l = []
     else:
         # We rely on ConfigObj providing us with a list already
-        l = unicode_str
+        l = maybe_list
     return l
 
 
@@ -2715,7 +2729,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())
@@ -3047,35 +3062,29 @@
         except KeyError:
             # Not registered
             opt = None
+        def expand_and_convert(val):
+            # This may need to be called twice if the value is None or ends up
+            # being None during expansion or conversion.
+            if val is not None:
+                if expand:
+                    if isinstance(val, basestring):
+                        val = self._expand_options_in_string(val)
+                    else:
+                        trace.warning('Cannot expand "%s":'
+                                      ' %s does not support option expansion'
+                                      % (name, type(val)))
+                if opt is not None:
+                    val = opt.convert_from_unicode(val)
+            return val
+        value = expand_and_convert(value)
         if opt is not None and value is None:
             # If the option is registered, it may provide a default value
             value = opt.get_default()
-        if expand:
-            value = self._expand_option_value(value)
-        if opt is not None and value is not None:
-            value = opt.convert_from_unicode(value)
-            if value is None:
-                # The conversion failed, fallback to the default value
-                value = opt.get_default()
-                if expand:
-                    value = self._expand_option_value(value)
-                value = opt.convert_from_unicode(value)
+            value = expand_and_convert(value)
         for hook in ConfigHooks['get']:
             hook(self, name, value)
         return value
 
-    def _expand_option_value(self, value):
-        """Expand the option value depending on its type."""
-        if isinstance(value, list):
-            value = self._expand_options_in_list(value)
-        elif isinstance(value, dict):
-            trace.warning('Cannot expand "%s":'
-                          ' Dicts do not support option expansion'
-                          % (name,))
-        elif isinstance(value, (str, unicode)):
-            value = self._expand_options_in_string(value)
-        return value
-
     def expand_options(self, string, env=None):
         """Expand option references in the string in the configuration context.
 
@@ -3088,29 +3097,6 @@
         """
         return self._expand_options_in_string(string, env)
 
-    def _expand_options_in_list(self, slist, env=None, _refs=None):
-        """Expand options in  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 _refs: Private list (FIFO) containing the options being
-            expanded to detect loops.
-
-        :returns: The flatten list of expanded strings.
-        """
-        # expand options in each value separately flattening lists
-        result = []
-        for s in slist:
-            value = self._expand_options_in_string(s, env, _refs)
-            if isinstance(value, list):
-                result.extend(value)
-            else:
-                result.append(value)
-        return result
-
     def _expand_options_in_string(self, string, env=None, _refs=None):
         """Expand options in the string in the configuration context.
 
@@ -3139,15 +3125,11 @@
                 # 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 chunk_is_ref:
-                    if chunk:
-                        # Keep only non-empty strings (or we get bogus empty
-                        # slots when a list value is involved).
-                        chunks.append(chunk)
+                    chunks.append(chunk)
                     chunk_is_ref = True
                 else:
                     name = chunk[1:-1]
@@ -3157,22 +3139,10 @@
                     value = self._expand_option(name, env, _refs)
                     if value is None:
                         raise errors.ExpandingUnknownOption(name, string)
-                    if isinstance(value, list):
-                        list_value = True
-                        chunks.extend(value)
-                    else:
-                        chunks.append(value)
+                    chunks.append(value)
                     _refs.pop()
                     chunk_is_ref = False
-            if list_value:
-                # Once a list appears as the result of an expansion, all
-                # callers will get a list result. This allows a consistent
-                # behavior even when some options in the expansion chain
-                # defined as strings (no comma in their value) but their
-                # expanded value is a list.
-                return self._expand_options_in_list(chunks, env, _refs)
-            else:
-                result = ''.join(chunks)
+            result = ''.join(chunks)
         return result
 
     def _expand_option(self, name, env, _refs):
@@ -3187,10 +3157,7 @@
             # configs, getting the option value should restart from the top
             # config, not the current one) -- vila 20101222
             value = self.get(name, expand=False)
-            if isinstance(value, list):
-                value = self._expand_options_in_list(value, env, _refs)
-            else:
-                value = self._expand_options_in_string(value, env, _refs)
+            value = self._expand_options_in_string(value, env, _refs)
         return value
 
     def _get_mutable_section(self):

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-09-02 08:34:51 +0000
+++ b/bzrlib/tests/test_config.py	2011-09-06 11:53:48 +0000
@@ -2462,8 +2462,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()
@@ -2476,10 +2479,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):
@@ -2660,7 +2659,7 @@
 
 
 class TestIniFileStoreContent(tests.TestCaseWithTransport):
-    """Simulate loading a config store without content of various encodings.
+    """Simulate loading a config store with content of various encodings.
 
     All files produced by bzr are in utf8 content.
 
@@ -2719,7 +2718,7 @@
 
 
 class TestIniConfigContent(tests.TestCaseWithTransport):
-    """Simulate loading a IniBasedConfig without content of various encodings.
+    """Simulate loading a IniBasedConfig with content of various encodings.
 
     All files produced by bzr are in utf8 content.
 
@@ -2907,8 +2906,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])
@@ -3367,6 +3368,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):
 
@@ -3451,6 +3462,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))
 
@@ -3461,6 +3474,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))
 
@@ -3473,9 +3488,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 happens
+        # 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))
 
 




More information about the bazaar-commits mailing list