[MERGE] Enable merging into PreviewTree

Aaron Bentley aaron at aaronbentley.com
Mon Sep 29 13:14:54 BST 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> Martin Pool has voted tweak.
> Status is now: Conditionally approved

Thanks.

>      def from_revision_ids(pb, tree, other, base=None, other_branch=None,
> -                          base_branch=None, revision_graph=None):
> +                          base_branch=None, revision_graph=None,
> +                          tree_branch=None):
>          """Return a Merger for revision-ids.
> 
> Could you please document tree_branch?

Sure.

> 
> -            return trans_id
> +            try:
> +                self._tree.iter_entries_by_dir([file_id]).next()
> +            except StopIteration:
> +                if file_id in self._non_present_ids:
> 
> Does this 'except' branch mean the directory exists and is empty?
> I'm not sure why this is happening.

StopIteration is raised if file_id is not present in the inventory.

This is just a transformation of the previously-existing code so that it
does not use Tree.inventory.


        elif file_id in self._tree.inventory:
            return self.trans_id_tree_file_id(file_id)

becomes

         else:
+            try:
+                self._tree.iter_entries_by_dir([file_id]).next()
+            except StopIteration:

...

+            else:
+                return self.trans_id_tree_file_id(file_id)




        elif file_id in self._non_present_ids:
            return self._non_present_ids[file_id]

becomes

+            except StopIteration:
+                if file_id in self._non_present_ids:
+                    return self._non_present_ids[file_id]


> 
> +            from_entry =
> self._tree.iter_entries_by_dir([file_id]).next()[1]
> 
> That's kind of a long expression to have occur twice in your patch and it
> smells a bit like those cases where we've intentionally provided only an
> iterator method but in fact need to query for just a single item...

That's pretty much what it is.  We actually have the single-item method,
but it is Inventory.__getitem__, not a Tree method at all, and we want
to avoid using Tree.inventory.  Because .inventory is semi-deprecated,
PreviewTree has no .inventory.

A fair amount of my PreviewTree work has involved replacing Inventory
usage with other methods, so getting a single entry via
iter_entries_by_dir is not uncommon.  That said, I'd be happy to make
Tree.get_inventory_entry a Tree method.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFI4Ma90F+nu1YWqI0RAgYRAJ9KNGIfoicG8FmvZXa2bJ74ge/JawCdE9YY
8KtYhwv4eH6bgHi3KT6pFLY=
=+V2P
-----END PGP SIGNATURE-----



More information about the bazaar mailing list