Bazaar

Merge lp:~jameinel/bzr/1.18-switch-branch into lp:~bzr/bzr/trunk-old

  1. 1.18-switch-branch
  2. Merge into trunk-old
Proposed by John A Meinel
Status: Merged
Approved by: Ian Clatworthy
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~jameinel/bzr/1.18-switch-branch
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 122 lines
To merge this branch: bzr merge lp:~jameinel/bzr/1.18-switch-branch
Reviewer Review Type Date Requested Status
Ian Clatworthy Approve
Review via email: mp+8469@code.launchpad.net
To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

from the commit comment:
  'bzr switch -b' can now be used to create the branch while you switch to it.

  This is just a convenience thing, but it shortens:
    bzr branch . ../new-branch
    bzr switch ../new-branch
  into a single:
    bzr switch -b ../new-branch

Revision history for this message
Robert Collins (lifeless) wrote :

On Thu, 2009-07-09 at 15:25 +0000, John A Meinel wrote:
> John A Meinel has proposed merging lp:~jameinel/bzr/1.18-switch-branch into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> from the commit comment:
> 'bzr switch -b' can now be used to create the branch while you switch to it.
>
> This is just a convenience thing, but it shortens:
> bzr branch . ../new-branch
> bzr switch ../new-branch
> into a single:
> bzr switch -b ../new-branch

We have a search path for branches. I'd like to suggest we use that here
so that you can do
bzr switch -b new-branch
and have that turn up in wt.branch.base + ../ + $arg if arg is neither
absolute, or relative to cwd (doesn't start with ..).

Revision history for this message
Robert Collins (lifeless) wrote :

I should add, my suggestion will make this _much_ better for folk with
lightweight checkouts away from the branch. Rather
than ../../../../path/to/new-branch, they can use 'new-branch'.

-Rob

Revision history for this message
John A Meinel (jameinel) wrote :

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

Robert Collins wrote:
> I should add, my suggestion will make this _much_ better for folk with
> lightweight checkouts away from the branch. Rather
> than ../../../../path/to/new-branch, they can use 'new-branch'.
>
> -Rob
>

I'm not sure if you noticed that I already check: "if '/' in to_location".

I'm not sure what more than that you need. (I even added tests for this
case....)

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

iEYEARECAAYFAkpXRZwACgkQJdeBCYSNAANMcACgjSDNhh14oqr3NV3lD5FMy9do
5xIAnjReCxMvAkIe38FGyQKga6Dm/foZ
=ugdy
-----END PGP SIGNATURE-----

Revision history for this message
Robert Collins (lifeless) wrote :

On Fri, 2009-07-10 at 13:45 +0000, John A Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Robert Collins wrote:
> > I should add, my suggestion will make this _much_ better for folk with
> > lightweight checkouts away from the branch. Rather
> > than ../../../../path/to/new-branch, they can use 'new-branch'.
> >
> > -Rob
> >
>
> I'm not sure if you noticed that I already check: "if '/' in to_location".
>
> I'm not sure what more than that you need. (I even added tests for this
> case....)

Oh oops :P. However I think thats more prohibitive than needed - in
particular I use subdirs not a flat branch space. What do you thank of
the slightly more sophisticated heuristic I proposed?

-Rob

Revision history for this message
John A Meinel (jameinel) wrote :

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

Robert Collins wrote:
> On Fri, 2009-07-10 at 13:45 +0000, John A Meinel wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Robert Collins wrote:
>>> I should add, my suggestion will make this _much_ better for folk with
>>> lightweight checkouts away from the branch. Rather
>>> than ../../../../path/to/new-branch, they can use 'new-branch'.
>>>
>>> -Rob
>>>
>> I'm not sure if you noticed that I already check: "if '/' in to_location".
>>
>> I'm not sure what more than that you need. (I even added tests for this
>> case....)
>
> Oh oops :P. However I think thats more prohibitive than needed - in
> particular I use subdirs not a flat branch space. What do you thank of
> the slightly more sophisticated heuristic I proposed?
>
> -Rob
>

You're welcome to write it?

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpXtmIACgkQJdeBCYSNAAOZnQCgpGYnQo1UGFH0TVSNkKw4q8sK
i7AAnAhmzjS1FGOBGP46CjeI+aOKA6K6
=ZyoD
-----END PGP SIGNATURE-----

Revision history for this message
John A Meinel (jameinel) wrote :

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

John A Meinel wrote:
...

>>>>
>>> I'm not sure if you noticed that I already check: "if '/' in to_location".
>>>
>>> I'm not sure what more than that you need. (I even added tests for this
>>> case....)
>> Oh oops :P. However I think thats more prohibitive than needed - in
>> particular I use subdirs not a flat branch space. What do you thank of
>> the slightly more sophisticated heuristic I proposed?
>
>> -Rob
>
>
> You're welcome to write it?
>
> John
> =:->
>

More seriously, I think it would be reasonable to do so, but if you
really do use nested values, then when you want to do "../another/dir"
it will look like you are doing something relative to the current dir.

I've proposed in the past that if you did:

  bzr switch bar/baz

That it would treat it as
  bzr switch $ORIG/../../bar/baz

However, we don't do that *today* so what I wrote is consistent with
what we have.

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

iEYEARECAAYFAkpXuVsACgkQJdeBCYSNAANCCACgsV1IqHX0CERE0SgqB/8rPptr
wLYAoJ/M5VYsWRVLL/q5ZdpG1MKoE4f/
=JxmI
-----END PGP SIGNATURE-----

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

There's a comment and a pass command in builtins.py that can be deleted. Otherwise, this looks good to land.

FWIW, I like the idea of the branch name implicitly being relative to the right origin (unless the name is absolute or starts with '.') but that can come later.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-07-09 01:44:00 +0000
+++ NEWS 2009-07-09 15:35:15 +0000
@@ -36,6 +36,11 @@
36 behavior.36 behavior.
37 (Vincent Ladeuil, #206577)37 (Vincent Ladeuil, #206577)
3838
39* ``bzr switch --create-branch/-b`` can now be used to create and switch
40 to a new branch. Supplying a name without a ``/`` will create the branch
41 relative to the existing branch. (similar to how ``bzr switch name``
42 works when the branch already exists.) (John Arbash Meinel)
43
3944
40Bug Fixes45Bug Fixes
41*********46*********
4247
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2009-07-02 21:17:35 +0000
+++ bzrlib/builtins.py 2009-07-09 15:35:16 +0000
@@ -5299,10 +5299,13 @@
52995299
5300 takes_args = ['to_location']5300 takes_args = ['to_location']
5301 takes_options = [Option('force',5301 takes_options = [Option('force',
5302 help='Switch even if local commits will be lost.')5302 help='Switch even if local commits will be lost.'),
5303 Option('create-branch', short_name='b',
5304 help='Create the target branch from this one before'
5305 ' switching to it.'),
5303 ]5306 ]
53045307
5305 def run(self, to_location, force=False):5308 def run(self, to_location, force=False, create_branch=False):
5306 from bzrlib import switch5309 from bzrlib import switch
5307 tree_location = '.'5310 tree_location = '.'
5308 control_dir = bzrdir.BzrDir.open_containing(tree_location)[0]5311 control_dir = bzrdir.BzrDir.open_containing(tree_location)[0]
@@ -5310,13 +5313,33 @@
5310 branch = control_dir.open_branch()5313 branch = control_dir.open_branch()
5311 had_explicit_nick = branch.get_config().has_explicit_nickname()5314 had_explicit_nick = branch.get_config().has_explicit_nickname()
5312 except errors.NotBranchError:5315 except errors.NotBranchError:
5316 branch = None
5313 had_explicit_nick = False5317 had_explicit_nick = False
5314 try:5318 if create_branch:
5315 to_branch = Branch.open(to_location)5319 if branch is None:
5316 except errors.NotBranchError:5320 raise errors.BzrCommandError('cannot create branch without'
5317 this_url = self._get_branch_location(control_dir)5321 ' source branch')
5318 to_branch = Branch.open(5322 if '/' not in to_location and '\\' not in to_location:
5319 urlutils.join(this_url, '..', to_location))5323 # This path is meant to be relative to the existing branch
5324 this_url = self._get_branch_location(control_dir)
5325 to_location = urlutils.join(this_url, '..', to_location)
5326 to_branch = branch.bzrdir.sprout(to_location,
5327 possible_transports=[branch.bzrdir.root_transport],
5328 source_branch=branch).open_branch()
5329 # try:
5330 # from_branch = control_dir.open_branch()
5331 # except errors.NotBranchError:
5332 # raise BzrCommandError('Cannot create a branch from this'
5333 # ' location when we cannot open this branch')
5334 # from_branch.bzrdir.sprout(
5335 pass
5336 else:
5337 try:
5338 to_branch = Branch.open(to_location)
5339 except errors.NotBranchError:
5340 this_url = self._get_branch_location(control_dir)
5341 to_branch = Branch.open(
5342 urlutils.join(this_url, '..', to_location))
5320 switch.switch(control_dir, to_branch, force)5343 switch.switch(control_dir, to_branch, force)
5321 if had_explicit_nick:5344 if had_explicit_nick:
5322 branch = control_dir.open_branch() #get the new branch!5345 branch = control_dir.open_branch() #get the new branch!
53235346
=== modified file 'bzrlib/tests/blackbox/test_switch.py'
--- bzrlib/tests/blackbox/test_switch.py 2009-06-10 03:56:49 +0000
+++ bzrlib/tests/blackbox/test_switch.py 2009-07-09 15:35:16 +0000
@@ -1,4 +1,4 @@
1# Copyright (C) 2007 Canonical Ltd1# Copyright (C) 2007, 2008, 2009 Canonical Ltd
2# -*- coding: utf-8 -*-2# -*- coding: utf-8 -*-
3#3#
4# This program is free software; you can redistribute it and/or modify4# This program is free software; you can redistribute it and/or modify
@@ -150,3 +150,36 @@
150 self.run_bzr('switch --force branch1', working_dir='tree')150 self.run_bzr('switch --force branch1', working_dir='tree')
151 branch_location = WorkingTree.open('tree').branch.base151 branch_location = WorkingTree.open('tree').branch.base
152 self.assertEndsWith(branch_location, 'branch1/')152 self.assertEndsWith(branch_location, 'branch1/')
153
154 def test_create_branch_no_branch(self):
155 self.prepare_lightweight_switch()
156 self.run_bzr_error(['cannot create branch without source branch'],
157 'switch --create-branch ../branch2', working_dir='tree')
158
159 def test_create_branch(self):
160 branch = self.make_branch('branch')
161 tree = branch.create_checkout('tree', lightweight=True)
162 tree.commit('one', rev_id='rev-1')
163 self.run_bzr('switch --create-branch ../branch2', working_dir='tree')
164 tree = WorkingTree.open('tree')
165 self.assertEndsWith(tree.branch.base, '/branch2/')
166
167 def test_create_branch_local(self):
168 branch = self.make_branch('branch')
169 tree = branch.create_checkout('tree', lightweight=True)
170 tree.commit('one', rev_id='rev-1')
171 self.run_bzr('switch --create-branch branch2', working_dir='tree')
172 tree = WorkingTree.open('tree')
173 # The new branch should have been created at the same level as
174 # 'branch', because we did not have a '/' segment
175 self.assertEqual(branch.base[:-1] + '2/', tree.branch.base)
176
177 def test_create_branch_short_name(self):
178 branch = self.make_branch('branch')
179 tree = branch.create_checkout('tree', lightweight=True)
180 tree.commit('one', rev_id='rev-1')
181 self.run_bzr('switch -b branch2', working_dir='tree')
182 tree = WorkingTree.open('tree')
183 # The new branch should have been created at the same level as
184 # 'branch', because we did not have a '/' segment
185 self.assertEqual(branch.base[:-1] + '2/', tree.branch.base)