Rev 3180: Review feedback. in http://people.ubuntu.com/~robertc/baz2.0/breadth-first-ghosts

[Robert Collins]

At http://people.ubuntu.com/~robertc/baz2.0/breadth-first-ghosts

------------------------------------------------------------
revno: 3180
revision-id:robertc at robertcollins.net-20080114231145-bv85r8wufwkfm9ee
parent: robertc at robertcollins.net-20080114005456-d4a5iief649hmtiy
committer: Robert Collins <robertc at robertcollins.net>
branch nick: breadth-first-ghosts
timestamp: Tue 2008-01-15 10:11:45 +1100
message:
  Review feedback.
modified:
  bzrlib/graph.py                graph_walker.py-20070525030359-y852guab65d4wtn0-1
  bzrlib/tests/test_graph.py     test_graph_walker.py-20070525030405-enq4r60hhi9xrujc-1
=== modified file 'bzrlib/graph.py'
--- a/bzrlib/graph.py	2008-01-14 00:54:56 +0000
+++ b/bzrlib/graph.py	2008-01-14 23:11:45 +0000
@@ -510,7 +510,7 @@
         self._next_query = set(revisions)
         self.seen = set()
         self._parents_provider = parents_provider
-        self._returning = 'checked'
+        self._returning = 'next_with_ghosts'
 
     def __repr__(self):
         if self._iterations:
@@ -534,9 +534,9 @@
 
         :return: A set of revision_ids.
         """
-        if self._returning != 'query':
+        if self._returning != 'next':
             # switch to returning the query, not the results.
-            self._returning = 'query'
+            self._returning = 'next'
             self._iterations += 1
             self.seen.update(self._next_query)
         else:
@@ -550,13 +550,14 @@
         
         Ancestors are returned in the order they are seen in a breadth-first
         traversal.  No ancestor will be returned more than once. Ancestors are
-        returned only after asking for their parents, which can
+        returned only after asking for their parents, which allows us to detect
+        which revisions are ghosts and which are not.
 
         :return: A tuple with (present ancestors, ghost ancestors) sets.
         """
-        if self._returning != 'checked':
+        if self._returning != 'next_with_ghosts':
             # switch to returning the results, not the current query.
-            self._returning = 'checked'
+            self._returning = 'next_with_ghosts'
             self._advance()
         if len(self._next_query) == 0:
             raise StopIteration()
@@ -567,7 +568,7 @@
         """Advance the search.
 
         Updates self.seen, self._next_query, self._current_present,
-        self._current_ghosts.
+        self._current_ghosts, self._current_parents and self._iterations.
         """
         self._iterations += 1
         found, ghosts, next, parents = self._do_query(self._next_query)
@@ -616,7 +617,7 @@
         search list.  In this case, the call is a no-op.
         """
         revisions = frozenset(revisions)
-        if self._returning == 'query':
+        if self._returning == 'next':
             stopped = self._next_query.intersection(revisions)
             self._next_query = self._next_query.difference(revisions)
         else:
@@ -635,7 +636,7 @@
                     stop_rev_references[parent_id] += 1
             # if only the stopped revisions reference it, the ref count will be
             # 0 after this loop
-            for rev, parents in self._current_parents.iteritems():
+            for parents in self._current_parents.itervalues():
                 for parent_id in parents:
                     try:
                         stop_rev_references[parent_id] -= 1
@@ -657,7 +658,7 @@
         ghost/not ghost status of revisions. (A tuple (present, ghosted)).
         """
         revisions = frozenset(revisions)
-        if self._returning == 'query':
+        if self._returning == 'next':
             self._next_query.update(revisions.difference(self.seen))
             self.seen.update(revisions)
         else:

=== modified file 'bzrlib/tests/test_graph.py'
--- a/bzrlib/tests/test_graph.py	2008-01-14 00:54:56 +0000
+++ b/bzrlib/tests/test_graph.py	2008-01-14 23:11:45 +0000
@@ -691,8 +691,8 @@
         self.assertRaises(StopIteration, search.next)
 
     def test_breadth_first_search_change_next_to_next_with_ghosts(self):
-        # To make the API robust, we allow changing from next() to
-        # next_with_ghosts() and vice verca.
+        # To make the API robust, we allow calling both next() and
+        # next_with_ghosts() on the same searcher.
         parent_graph = {
             'head':['present'],
             'present':['child', 'ghost'],
@@ -701,14 +701,14 @@
         parents_provider = InstrumentedParentsProvider(
             _mod_graph.DictParentsProvider(parent_graph))
         graph = _mod_graph.Graph(parents_provider)
-        # with_ghosts reports the ghosts
+        # start with next_with_ghosts
         search = graph._make_breadth_first_searcher(['head'])
         self.assertEqual((set(['head']), set()), search.next_with_ghosts())
         self.assertEqual(set(['present']), search.next())
         self.assertEqual((set(['child']), set(['ghost'])),
             search.next_with_ghosts())
         self.assertRaises(StopIteration, search.next)
-        # next includes them
+        # start with next
         search = graph._make_breadth_first_searcher(['head'])
         self.assertEqual(set(['head']), search.next())
         self.assertEqual((set(['present']), set()), search.next_with_ghosts())
@@ -717,8 +717,7 @@
         self.assertRaises(StopIteration, search.next_with_ghosts)
 
     def test_breadth_first_change_search(self):
-        # To make the API robust, we allow changing from next() to
-        # next_with_ghosts() and vice verca.
+        # Changing the search should work with both next and next_with_ghosts.
         parent_graph = {
             'head':['present'],
             'present':['stopped'],