[MERGE] PreviewTree updates

John Arbash Meinel john at arbash-meinel.com
Wed Jul 2 18:22:55 BST 2008


John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
Looking at the other iter_entries_by_dir, I'm not sure this is correct:
+            parent = todo.pop()
+            parent_file_id = self._transform.final_file_id(parent)
+            children = list(self._all_children(parent))
+            paths = dict(zip(children, 
self._final_paths.get_paths(children)))
+            children.sort(key=paths.get)
+            todo.extend(children)
+            for trans_id in children:
+                ordered_ids.append((trans_id, parent_file_id))

Most of them sort by todo.extend(reversed(children)). Specifically, if 
you have:

a/
b/
c/

doing stack.extend([a, b, c])
stack.pop() => 'c'

When you want: stack.pop() => 'a'.

We may need to be adding another iter_entries_by_dir() implementation 
test to expose this.

Also, don't you only want to add children that are directories? Or is it 
just that the follow up call for

   children = list(self._all_children(parent))

Will return an empty list, so it gets filtered after the fact. I can't 
help but thinking pre-filtering would be more efficient. But as long as 
it is *correct*. :)

So we need a test that iter_entries_by_dir() does the right thing when 
there are multiple child directories with contents. (You'll get the 
children ordering correct, but the grandchildren will be wrong.)
Otherwise approve.


For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C4863D54C.4000509%40aaronbentley.com%3E



More information about the bazaar mailing list