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
1=== modified file 'NEWS'
2--- NEWS 2009-07-09 01:44:00 +0000
3+++ NEWS 2009-07-09 15:35:15 +0000
4@@ -36,6 +36,11 @@
5 behavior.
6 (Vincent Ladeuil, #206577)
7
8+* ``bzr switch --create-branch/-b`` can now be used to create and switch
9+ to a new branch. Supplying a name without a ``/`` will create the branch
10+ relative to the existing branch. (similar to how ``bzr switch name``
11+ works when the branch already exists.) (John Arbash Meinel)
12+
13
14 Bug Fixes
15 *********
16
17=== modified file 'bzrlib/builtins.py'
18--- bzrlib/builtins.py 2009-07-02 21:17:35 +0000
19+++ bzrlib/builtins.py 2009-07-09 15:35:16 +0000
20@@ -5299,10 +5299,13 @@
21
22 takes_args = ['to_location']
23 takes_options = [Option('force',
24- help='Switch even if local commits will be lost.')
25+ help='Switch even if local commits will be lost.'),
26+ Option('create-branch', short_name='b',
27+ help='Create the target branch from this one before'
28+ ' switching to it.'),
29 ]
30
31- def run(self, to_location, force=False):
32+ def run(self, to_location, force=False, create_branch=False):
33 from bzrlib import switch
34 tree_location = '.'
35 control_dir = bzrdir.BzrDir.open_containing(tree_location)[0]
36@@ -5310,13 +5313,33 @@
37 branch = control_dir.open_branch()
38 had_explicit_nick = branch.get_config().has_explicit_nickname()
39 except errors.NotBranchError:
40+ branch = None
41 had_explicit_nick = False
42- try:
43- to_branch = Branch.open(to_location)
44- except errors.NotBranchError:
45- this_url = self._get_branch_location(control_dir)
46- to_branch = Branch.open(
47- urlutils.join(this_url, '..', to_location))
48+ if create_branch:
49+ if branch is None:
50+ raise errors.BzrCommandError('cannot create branch without'
51+ ' source branch')
52+ if '/' not in to_location and '\\' not in to_location:
53+ # This path is meant to be relative to the existing branch
54+ this_url = self._get_branch_location(control_dir)
55+ to_location = urlutils.join(this_url, '..', to_location)
56+ to_branch = branch.bzrdir.sprout(to_location,
57+ possible_transports=[branch.bzrdir.root_transport],
58+ source_branch=branch).open_branch()
59+ # try:
60+ # from_branch = control_dir.open_branch()
61+ # except errors.NotBranchError:
62+ # raise BzrCommandError('Cannot create a branch from this'
63+ # ' location when we cannot open this branch')
64+ # from_branch.bzrdir.sprout(
65+ pass
66+ else:
67+ try:
68+ to_branch = Branch.open(to_location)
69+ except errors.NotBranchError:
70+ this_url = self._get_branch_location(control_dir)
71+ to_branch = Branch.open(
72+ urlutils.join(this_url, '..', to_location))
73 switch.switch(control_dir, to_branch, force)
74 if had_explicit_nick:
75 branch = control_dir.open_branch() #get the new branch!
76
77=== modified file 'bzrlib/tests/blackbox/test_switch.py'
78--- bzrlib/tests/blackbox/test_switch.py 2009-06-10 03:56:49 +0000
79+++ bzrlib/tests/blackbox/test_switch.py 2009-07-09 15:35:16 +0000
80@@ -1,4 +1,4 @@
81-# Copyright (C) 2007 Canonical Ltd
82+# Copyright (C) 2007, 2008, 2009 Canonical Ltd
83 # -*- coding: utf-8 -*-
84 #
85 # This program is free software; you can redistribute it and/or modify
86@@ -150,3 +150,36 @@
87 self.run_bzr('switch --force branch1', working_dir='tree')
88 branch_location = WorkingTree.open('tree').branch.base
89 self.assertEndsWith(branch_location, 'branch1/')
90+
91+ def test_create_branch_no_branch(self):
92+ self.prepare_lightweight_switch()
93+ self.run_bzr_error(['cannot create branch without source branch'],
94+ 'switch --create-branch ../branch2', working_dir='tree')
95+
96+ def test_create_branch(self):
97+ branch = self.make_branch('branch')
98+ tree = branch.create_checkout('tree', lightweight=True)
99+ tree.commit('one', rev_id='rev-1')
100+ self.run_bzr('switch --create-branch ../branch2', working_dir='tree')
101+ tree = WorkingTree.open('tree')
102+ self.assertEndsWith(tree.branch.base, '/branch2/')
103+
104+ def test_create_branch_local(self):
105+ branch = self.make_branch('branch')
106+ tree = branch.create_checkout('tree', lightweight=True)
107+ tree.commit('one', rev_id='rev-1')
108+ self.run_bzr('switch --create-branch branch2', working_dir='tree')
109+ tree = WorkingTree.open('tree')
110+ # The new branch should have been created at the same level as
111+ # 'branch', because we did not have a '/' segment
112+ self.assertEqual(branch.base[:-1] + '2/', tree.branch.base)
113+
114+ def test_create_branch_short_name(self):
115+ branch = self.make_branch('branch')
116+ tree = branch.create_checkout('tree', lightweight=True)
117+ tree.commit('one', rev_id='rev-1')
118+ self.run_bzr('switch -b branch2', working_dir='tree')
119+ tree = WorkingTree.open('tree')
120+ # The new branch should have been created at the same level as
121+ # 'branch', because we did not have a '/' segment
122+ self.assertEqual(branch.base[:-1] + '2/', tree.branch.base)