Merge lp:~nmb/bzr/lp-open-containing into lp:bzr

[Neil Martinsen-Burrell]

[Neil Martinsen-Burrell (nmb)]

launchpad-open doesn't work when called from a subdirectory of a branch (for example when hacking in bzr.dev/bzrlib/plugins/launchpad, bzr lp-open raises "NotLaunchpadBranch") This fixes that by using open_containing instead of open. The included test fails without the change and passes with it.

[Vincent Ladeuil (vila)]

Excellent, one test, failing before, passing after. No brainer :)

I don't approve yet because I'm pretty sure there a bug for that, so if you can find it and update the NEWS entry, I'll be happy to approve. If not, I'll try to find it back myself but I thought you may have started from it and just forget to mention it.

review: Needs Information

[Neil Martinsen-Burrell (nmb)]

489102, updated in that branch

[Vincent Ladeuil (vila)]

Thank and again: well done !

review: Approve

[John A Meinel (jameinel)]

+
45 + def test_launchpad_branch_subdirectory(self):
46 + # lp-open in a subdirectory of a registered branch should work
47 + wt = self.make_branch_and_tree('lp')
48 + wt.branch.set_push_location(
49 + 'bzr+ssh://bazaar.launchpad.net/~foo/bar/baz')
50 + self.build_tree(['lp/a/'])
51 + os.chdir('lp/a')
52 + self.assertEqual(
53 + ['Opening https://code.edge.launchpad.net/~foo/bar/baz in web '
54 + 'browser'],
55 + self.run_open('.'))

^- did you need 'os.chdir()' rather than 'self.run_open('lp/a')' ?

Anyway, approve regardless, and I'll merge, but I prefer to avoid chdir when possible. (As it is changing global state, and is something we try not to do withing bzrlib. We certainly have tests that do it, but we've moved some of that out into run_bzr(working_dir=...))

review: Approve

[Vincent Ladeuil (vila)]

Test fixed in a later submission because postponed test updates never occur otherwise :D

=== modified file 'NEWS'
--- NEWS	2009-12-07 10:38:09 +0000
+++ NEWS	2009-12-07 17:11:12 +0000
@@ -89,6 +89,10 @@
   passed to the external diff tool. This allows the file to be edited if the
   diff tool provides for this. (Gary van der Merwe, #490738)
   
+* The launchpad-open command can now be used from a subdirectory of a
+  branch, not just from the root of the branch. 
+  (Neil Martinsen-Burrell, #489102)
+
 
 Improvements
 ************

=== modified file 'bzrlib/plugins/launchpad/__init__.py'
--- bzrlib/plugins/launchpad/__init__.py	2009-10-30 14:44:52 +0000
+++ bzrlib/plugins/launchpad/__init__.py	2009-12-07 17:11:12 +0000
@@ -170,7 +170,7 @@
         """Yield possible external locations for the branch at 'location'."""
         yield location
         try:
-            branch = _mod_branch.Branch.open(location)
+            branch = _mod_branch.Branch.open_containing(location)[0]
         except NotBranchError:
             return
         branch_url = branch.get_public_branch()

=== modified file 'bzrlib/plugins/launchpad/test_lp_open.py'
--- bzrlib/plugins/launchpad/test_lp_open.py	2009-03-23 14:59:43 +0000
+++ bzrlib/plugins/launchpad/test_lp_open.py	2009-12-07 17:11:12 +0000
@@ -17,6 +17,7 @@
 """Tests for the launchpad-open command."""
 
 from bzrlib.tests import TestCaseWithTransport
+import os
 
 
 class TestLaunchpadOpen(TestCaseWithTransport):
@@ -87,3 +88,15 @@
             ['Opening https://code.edge.launchpad.net/~foo/bar/baz in web '
              'browser'],
             self.run_open('bzr+ssh://bazaar.launchpad.net/~foo/bar/baz'))
+
+    def test_launchpad_branch_subdirectory(self):
+        # lp-open in a subdirectory of a registered branch should work
+        wt = self.make_branch_and_tree('lp')
+        wt.branch.set_push_location(
+            'bzr+ssh://bazaar.launchpad.net/~foo/bar/baz')
+        self.build_tree(['lp/a/'])
+        os.chdir('lp/a')
+        self.assertEqual(
+            ['Opening https://code.edge.launchpad.net/~foo/bar/baz in web '
+             'browser'],
+            self.run_open('.'))