[configuration] closes #185648 : load options in config file order

Option should be processed in file order, not declaration order.

related to issue #87 on pylint : https://bitbucket.org/logilab/pylint/issue/87

3 files changed, 60 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index f64e1fc..0560fef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,7 @@ ChangeLog for logilab.common
 --
    * shellutils: restore py 2.5 compat by removing usage of class decorator
    * pytest: drop broken --coverage option
+   * configuration: load options in config file order (#185648)
 
 2014-03-07  --  0.62.0
    * modutils: cleanup_sys_modules returns the list of cleaned modules
diff --git a/configuration.py b/configuration.py
index a508bc8..e5ce210 100644
--- a/configuration.py
+++ b/configuration.py
@@ -96,7 +96,15 @@ Quick start: simplest usage
   multiple=4,5,6
 
   number=3
-  >>>
+
+  Note : starting with Python 2.7 ConfigParser is able to take into
+  account the order of occurrences of the options into a file (by
+  using an OrderedDict). If you have two options changing some common
+  state, like a 'disable-all-stuff' and a 'enable-some-stuff-a', their
+  order of appearance will be significant : the last specified in the
+  file wins. For earlier version of python and logilab.common newer
+  than 0.61 the behaviour is unspecified.
+
 """
 __docformat__ = "restructuredtext en"
 
@@ -655,13 +663,13 @@ class OptionsManagerMixIn(object):
         options provider)
         """
         parser = self.cfgfile_parser
-        for provider in self.options_providers:
-            for section, option, optdict in provider.all_options():
-                try:
-                    value = parser.get(section, option)
-                    provider.set_option(option, value, optdict=optdict)
-                except (NoSectionError, NoOptionError), ex:
-                    continue
+        for section in parser.sections():
+             for option, value in parser.items(section):
+                  try:
+                       self.global_set_option(option, value)
+                  except (KeyError, OptionError):
+                       # TODO handle here undeclared options appearing in the config file
+                       continue
 
     def load_configuration(self, **kwargs):
         """override configuration according to given parameters
diff --git a/test/unittest_configuration.py b/test/unittest_configuration.py
index a8e3c0f..edcb5db 100644
--- a/test/unittest_configuration.py
+++ b/test/unittest_configuration.py
@@ -1,4 +1,4 @@
-# copyright 2003-2011 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of logilab-common.
@@ -25,7 +25,7 @@ from sys import version_info
 
 from logilab.common.testlib import TestCase, unittest_main
 from logilab.common.optik_ext import OptionValueError
-from logilab.common.configuration import Configuration, \
+from logilab.common.configuration import Configuration, OptionError, \
      OptionsManagerMixIn, OptionsProviderMixIn, Method, read_old_config, \
      merge_options
 
@@ -47,6 +47,8 @@ OPTIONS = [('dothis', {'type':'yn', 'action': 'store', 'default': True, 'metavar
 
            ('diffgroup', {'type':'string', 'default':'pouet', 'metavar': '<key=val>',
                           'group': 'agroup'}),
+           ('reset-value', {'type': 'string', 'metavar': '<string>', 'short': 'r',
+                            'dest':'value'}),
 
            ]
 
@@ -149,6 +151,38 @@ diffgroup=zou
         finally:
             os.remove(file)
 
+    def test_option_order(self):
+        """ Check that options are taken into account in the command line order
+            and not in the order they are defined in the Configuration object.
+        """
+        file = tempfile.mktemp()
+        stream = open(file, 'w')
+        try:
+            stream.write("""[Test]
+reset-value=toto
+value=tata
+""")
+            stream.close()
+            self.cfg.load_file_configuration(file)
+        finally:
+            os.remove(file)
+        self.assertEqual(self.cfg['value'], 'tata')
+
+    def test_unsupported_options(self):
+        file = tempfile.mktemp()
+        stream = open(file, 'w')
+        try:
+            stream.write("""[Test]
+whatever=toto
+value=tata
+""")
+            stream.close()
+            self.cfg.load_file_configuration(file)
+        finally:
+            os.remove(file)
+        self.assertEqual(self.cfg['value'], 'tata')
+        self.assertRaises(OptionError, self.cfg.__getitem__, 'whatever')
+
     def test_generate_config(self):
         stream = StringIO()
         self.cfg.generate_config(stream)
@@ -170,6 +204,8 @@ multiple-choice=yo,ye
 
 named=key:val
 
+#reset-value=
+
 
 [AGROUP]
 
@@ -197,6 +233,8 @@ multiple-choice=yo,ye
 
 named=key:val
 
+reset-value='    '
+
 
 [AGROUP]
 
@@ -251,6 +289,7 @@ Options:
   --choice=<yo|ye>
   --multiple-choice=<yo|ye>
   --named=<key=val>
+  -r <string>, --reset-value=<string>
 
   Agroup:
     --diffgroup=<key=val>
@@ -309,6 +348,8 @@ multiple-choice=yo,ye
 
 named=key:val
 
+reset-value='    '
+
 
 [AGROUP]