Rev 5768: Using iterators is even clearer. in file:///home/vila/src/bzr/experimental/config/

Vincent Ladeuil v.ladeuil+lp at free.fr
Sat Apr 9 20:01:11 UTC 2011


At file:///home/vila/src/bzr/experimental/config/

------------------------------------------------------------
revno: 5768
revision-id: v.ladeuil+lp at free.fr-20110409200111-c5ivo0e8yl7tj23r
parent: v.ladeuil+lp at free.fr-20110408132006-mapwsxq0ffto7bjg
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: refactor-locationconfig-matching
timestamp: Sat 2011-04-09 22:01:11 +0200
message:
  Using iterators is even clearer.
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-04-08 13:20:06 +0000
+++ b/bzrlib/config.py	2011-04-09 20:01:11 +0000
@@ -967,20 +967,20 @@
         super(LockableConfig, self).remove_user_option(option_name,
                                                        section_name)
 
-def _filter_for_location_by_parts(sections, location):
+def _iter_for_location_by_parts(sections, location):
     """Keep only the sessions matching the specified location.
 
     :param sections: An iterable of section names.
 
     :param location: An url or a local path to match against.
 
-    :returns: A list of (nb_parts, section, extra_path) where nb is the number
-        of path components in the section name, section is the section name and
-        extra_path is the difference between location and the section name.
+    :returns: An iterator of (section, extra_path, nb_parts) where nb is the
+        number of path components in the section name, section is the section
+        name and extra_path is the difference between location and the section
+        name.
     """
     location_parts = location.rstrip('/').split('/')
 
-    matches = []
     for section in sections:
         # location is a local path if possible, so we need
         # to convert 'file://' urls to local paths if necessary.
@@ -1020,8 +1020,7 @@
             continue
         # build the path difference between the section and the location
         extra_path = '/'.join(location_parts[len(section_parts):])
-        matches.append((len(section_parts), section, extra_path))
-    return matches
+        yield section, extra_path, len(section_parts)
 
 
 class LocationConfig(LockableConfig):
@@ -1057,21 +1056,20 @@
 
     def _get_matching_sections(self):
         """Return an ordered list of section names matching this location."""
-        sections = self._get_parser()
-
-        matches = _filter_for_location_by_parts(sections, self.location)
+        matches = list(_iter_for_location_by_parts(self._get_parser(),
+                                                   self.location))
         # put the longest (aka more specific) locations first
-        matches.sort(reverse=True)
-        sections = []
-        for (length, section, extra_path) in matches:
-            sections.append((section, extra_path))
+        matches.sort(
+            key=lambda (section, extra_path, length): (length, section),
+            reverse=True)
+        for (section, extra_path, length) in matches:
+            yield section, extra_path
             # should we stop looking for parent configs here?
             try:
                 if self._get_parser()[section].as_bool('ignore_parents'):
                     break
             except KeyError:
                 pass
-        return sections
 
     def _get_sections(self, name=None):
         """See IniBasedConfig._get_sections()."""

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-04-05 12:11:26 +0000
+++ b/bzrlib/tests/test_config.py	2011-04-09 20:01:11 +0000
@@ -1264,55 +1264,52 @@
         self.failUnless(isinstance(global_config, config.GlobalConfig))
         self.failUnless(global_config is my_config._get_global_config())
 
+    def assertLocationMatching(self, expected):
+        self.assertEqual(expected,
+                         list(self.my_location_config._get_matching_sections()))
+
     def test__get_matching_sections_no_match(self):
         self.get_branch_config('/')
-        self.assertEqual([], self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([])
 
     def test__get_matching_sections_exact(self):
         self.get_branch_config('http://www.example.com')
-        self.assertEqual([('http://www.example.com', '')],
-                         self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([('http://www.example.com', '')])
 
     def test__get_matching_sections_suffix_does_not(self):
         self.get_branch_config('http://www.example.com-com')
-        self.assertEqual([], self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([])
 
     def test__get_matching_sections_subdir_recursive(self):
         self.get_branch_config('http://www.example.com/com')
-        self.assertEqual([('http://www.example.com', 'com')],
-                         self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([('http://www.example.com', 'com')])
 
     def test__get_matching_sections_ignoreparent(self):
         self.get_branch_config('http://www.example.com/ignoreparent')
-        self.assertEqual([('http://www.example.com/ignoreparent', '')],
-                         self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([('http://www.example.com/ignoreparent',
+                                      '')])
 
     def test__get_matching_sections_ignoreparent_subdir(self):
         self.get_branch_config(
             'http://www.example.com/ignoreparent/childbranch')
-        self.assertEqual([('http://www.example.com/ignoreparent',
-                           'childbranch')],
-                         self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([('http://www.example.com/ignoreparent',
+                                      'childbranch')])
 
     def test__get_matching_sections_subdir_trailing_slash(self):
         self.get_branch_config('/b')
-        self.assertEqual([('/b/', '')],
-                         self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([('/b/', '')])
 
     def test__get_matching_sections_subdir_child(self):
         self.get_branch_config('/a/foo')
-        self.assertEqual([('/a/*', ''), ('/a/', 'foo')],
-                         self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([('/a/*', ''), ('/a/', 'foo')])
 
     def test__get_matching_sections_subdir_child_child(self):
         self.get_branch_config('/a/foo/bar')
-        self.assertEqual([('/a/*', 'bar'), ('/a/', 'foo/bar')],
-                         self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([('/a/*', 'bar'), ('/a/', 'foo/bar')])
 
     def test__get_matching_sections_trailing_slash_with_children(self):
         self.get_branch_config('/a/')
-        self.assertEqual([('/a/', '')],
-                         self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([('/a/', '')])
 
     def test__get_matching_sections_explicit_over_glob(self):
         # XXX: 2006-09-08 jamesh
@@ -1320,8 +1317,7 @@
         # was a config section for '/a/?', it would get precedence
         # over '/a/c'.
         self.get_branch_config('/a/c')
-        self.assertEqual([('/a/c', ''), ('/a/*', ''), ('/a/', 'c')],
-                         self.my_location_config._get_matching_sections())
+        self.assertLocationMatching([('/a/c', ''), ('/a/*', ''), ('/a/', 'c')])
 
     def test__get_option_policy_normal(self):
         self.get_branch_config('http://www.example.com')



More information about the bazaar-commits mailing list