[MERGE] Re: Implementation and tests for -r revno:N:path

[John Arbash Meinel]

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

Matthieu Moy wrote:
...

> 
> Please, consider merging a fix and testcases for this here:
> 
> http://www-verimag.imag.fr/~moy/bzr/bzr/bzr.diff-outside-tree-fix/
> 

You can simplify your if/elif/else raise code quite a bit. Instead of:
- -            tree1, tree2 = None, None
+            if revision is None:
+                raise
+            elif len(revision) == 1:
+                raise
+            elif len(revision) == 2:
+                if revision[0].needs_tree() or revision[1].needs_tree():
+                    raise
+                else:
+                    tree1, tree2 = None, None
+            else:
+                raise

What about:

if (revision is not None and len(revision) == 2
    and not revision[0].needs_tree()
    and not revision[1].needs_tree()):
    tree1, tree2 = None, None
else:
    raise

I also think that 'needs_tree' is an inaccurate statement. It should be
'needs_branch'.
None of them need a Tree, they just need a Branch to define the revision
history they are operating on.

Also, the revision specs for 'branch:' and 'ancestor:' don't need a
branch, since they define one internally.

You need a blank line before 'def test_diff_outside_tree'.

You have a couple of lines that exceed 79 characters.

In general, I want to merge something before we release 0.9, since it is
a regression.

If you don't have time to clean it up. Let me know, and I'll put
something together.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEx9R6JdeBCYSNAAMRAu37AJ9u8PfdBW7/31MIg3RVpSgDmwV0KgCfdMwO
Sg0/WYNGF1Qwfyip0uB2gIs=
=vtSq
-----END PGP SIGNATURE-----