Rev 6097: Replace ugly default value declarations with ad-hoc and limited conversion to unicode strings. in file:///home/vila/src/bzr/experimental/convert-default-values/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/experimental/convert-default-values/

------------------------------------------------------------
revno: 6097
revision-id: v.ladeuil+lp at free.fr-20110825211732-4hfcz3tyqp4qpztb
parent: v.ladeuil+lp at free.fr-20110824185559-e9wx4bkl64xxgbd2
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: convert-default-values
timestamp: Thu 2011-08-25 23:17:32 +0200
message:
  Replace ugly default value declarations with ad-hoc and limited conversion to unicode strings.
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-08-24 18:55:59 +0000
+++ b/bzrlib/config.py	2011-08-25 21:17:32 +0000
@@ -2322,8 +2322,9 @@
         :param name: the name used to refer to the option.
 
         :param default: the default value to use when none exist in the config
-            stores. This must be a unicode string as provided by the config
-            stores.
+            stores. This is either a string that ``from_unicode`` will convert
+            into the proper type or a python object that can be stringified (so
+            only the empty list is supported for example).
 
         :param default_from_env: A list of environment variables which can
            provide a default value. 'default' will be used only if none of the
@@ -2345,7 +2346,23 @@
         if default_from_env is None:
             default_from_env = []
         self.name = name
-        self.default = default
+        # Convert the default value to a unicode string so all values are
+        # strings internally before conversion (via from_unicode) is attempted.
+        if default is None:
+            self.default = None
+        elif isinstance(default, list):
+            # Only the empty list is supported
+            if default:
+                raise AssertionError(
+                    'Only empty lists are supported as default values')
+            self.default = u','
+        elif isinstance(default, (str, unicode, bool, int)):
+            # Rely on python to convert strings, booleans and integers
+            self.default = u'%s' % (default,)
+        else:
+            # other python objects are not expected
+            raise AssertionError('%r is not supported as a default value'
+                                 % (default,))
         self.default_from_env = default_from_env
         self.help = help
         self.from_unicode = from_unicode
@@ -2464,7 +2481,7 @@
 # Registered options in lexicographical order
 
 option_registry.register(
-    Option('bzr.workingtree.worth_saving_limit', default=u'10',
+    Option('bzr.workingtree.worth_saving_limit', default=10,
            from_unicode=int_from_store,  invalid='warning',
            help='''\
 How many changes before saving the dirstate.
@@ -2476,7 +2493,7 @@
 a file has been touched.
 '''))
 option_registry.register(
-    Option('dirstate.fdatasync', default=u'True',
+    Option('dirstate.fdatasync', default=True,
            from_unicode=bool_from_store,
            help='''\
 Flush dirstate changes onto physical disk?
@@ -2486,16 +2503,16 @@
 should not be lost if the machine crashes.  See also repository.fdatasync.
 '''))
 option_registry.register(
-    Option('debug_flags', default=u'', from_unicode=list_from_store,
+    Option('debug_flags', default=[], from_unicode=list_from_store,
            help='Debug flags to activate.'))
 option_registry.register(
-    Option('default_format', default=u'2a',
+    Option('default_format', default='2a',
            help='Format used when creating branches.'))
 option_registry.register(
     Option('editor',
            help='The command called to launch an editor to enter a message.'))
 option_registry.register(
-    Option('ignore_missing_extensions', default=u'False',
+    Option('ignore_missing_extensions', default=False,
            from_unicode=bool_from_store,
            help='''\
 Control the missing extensions warning display.
@@ -2506,7 +2523,7 @@
     Option('language',
            help='Language to translate messages into.'))
 option_registry.register(
-    Option('locks.steal_dead', default=u'False', from_unicode=bool_from_store,
+    Option('locks.steal_dead', default=False, from_unicode=bool_from_store,
            help='''\
 Steal locks that appears to be dead.
 
@@ -2522,7 +2539,7 @@
            help= 'Unicode encoding for output'
            ' (terminal encoding if not specified).'))
 option_registry.register(
-    Option('repository.fdatasync', default=u'True',
+    Option('repository.fdatasync', default=True,
            from_unicode=bool_from_store,
            help='''\
 Flush repository changes onto physical disk?

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-08-22 14:15:36 +0000
+++ b/bzrlib/tests/test_config.py	2011-08-25 21:17:32 +0000
@@ -2247,6 +2247,13 @@
         # The first env var set wins
         self.assertEquals('foo', opt.get_default())
 
+    def test_not_supported_list_default_value(self):
+        self.assertRaises(AssertionError, config.Option, 'foo', default=[1])
+
+    def test_not_supported_object_default_value(self):
+        self.assertRaises(AssertionError, config.Option, 'foo',
+                          default=object())
+
 
 class TestOptionConverterMixin(object):
 
@@ -3132,10 +3139,10 @@
         self.assertEquals(True, self.conf.get('foo'))
 
     def test_get_default_bool_False(self):
-        self.register_bool_option('foo', u'False')
+        self.register_bool_option('foo', False)
         self.assertEquals(False, self.conf.get('foo'))
 
-    def test_get_default_bool_False(self):
+    def test_get_default_bool_False_as_string(self):
         self.register_bool_option('foo', u'False')
         self.assertEquals(False, self.conf.get('foo'))
 
@@ -3161,6 +3168,10 @@
         self.assertEquals(None, self.conf.get('foo'))
 
     def test_get_default_integer(self):
+        self.register_integer_option('foo', 42)
+        self.assertEquals(42, self.conf.get('foo'))
+
+    def test_get_default_integer_as_string(self):
         self.register_integer_option('foo', u'42')
         self.assertEquals(42, self.conf.get('foo'))
 
@@ -3194,12 +3205,12 @@
         self.assertEquals([], self.conf.get('foo'))
 
     def test_get_with_list_converter_no_item(self):
-        self.register_list_option('foo', [1])
+        self.register_list_option('foo', None)
         self.conf.store._load_from_string('foo=,')
         self.assertEquals([], self.conf.get('foo'))
 
     def test_get_with_list_converter_many_items(self):
-        self.register_list_option('foo', [1])
+        self.register_list_option('foo', None)
         self.conf.store._load_from_string('foo=m,o,r,e')
         self.assertEquals(['m', 'o', 'r', 'e'], self.conf.get('foo'))