[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