[MERGE][#144421] Fix `bzr st -rbranch:PATH_TO_BRANCH`
ian.clatworthy at internode.on.net
Thu Sep 25 00:35:36 BST 2008
Aaron Bentley wrote:
>> +def _get_revision_tree(command_name, revision, branch=None, tree=None):
> I think it would be clearer to call this '_get_one_revision_tree'.
> Also, 'revision_specs' or even 'revisions' would be clearer than 'revision'.
Here's a record of what I tweaked and what I didn't ...
I renamed the method to _get_one_revision_tree and the parameter to
>> @@ -1836,11 +1852,8 @@
>> relpath = u''
>> elif relpath:
>> relpath += '/'
>> - if revision is not None:
>> - tree = branch.repository.revision_tree(
>> - revision.as_revision_id(branch))
>> - elif tree is None:
>> - tree = branch.basis_tree()
>> + if revision is not None or tree is None:
>> + tree = _get_revision_tree('ls', revision, branch=branch)
> This kind of logic is very common, and it makes me think that there
> should be a _get_one_tree (not revision_tree) function.
> def _get_one_tree(revisions, branch, tree)
> if revisions is None:
> if tree is not None:
> return tree
> return branch.basis_tree()
> return revision.as_revision_tree(branch)
This tweak is a good idea. It wasn't required to land the change
though so I didn't make it while merging. (Adding the method would
have been easy enough but chasing down everywhere where it should
be called was a larger task that a small tweak.)
>> === modified file 'bzrlib/revisionspec.py'
>> --- bzrlib/revisionspec.py 2008-07-19 15:00:27 +0000
>> +++ bzrlib/revisionspec.py 2008-08-28 13:41:15 +0000
>> @@ -252,6 +252,25 @@
>> return self.in_history(context_branch).rev_id
>> + def as_tree(self, context_branch):
>> + """Return the tree object for this revisions spec.
>> + Some revision specs require a context_branch to be able to determine
>> + the revision id and access the repository. Not all specs will make
>> + use of it.
>> + """
> It would be nice if, as well as 'context_branch', an optional
> 'context_tree' could be supplied.
>> === modified file 'bzrlib/tests/blackbox/test_ls.py'
>> --- bzrlib/tests/blackbox/test_ls.py 2008-05-02 00:48:17 +0000
>> +++ bzrlib/tests/blackbox/test_ls.py 2008-08-28 13:41:15 +0000
>> @@ -181,6 +181,7 @@
>> self.ls_equals('', '--revision 1')
>> def test_ls_branch(self):
>> """If a branch is specified, files are listed from it"""
>> self.build_tree(['subdir/', 'subdir/b'])
> ^^^ There should be only one blank line, according to PEP8.
>> === modified file 'bzrlib/tests/blackbox/test_status.py'
>> --- bzrlib/tests/blackbox/test_status.py 2008-08-16 22:28:01 +0000
>> +++ bzrlib/tests/blackbox/test_status.py 2008-08-28 13:41:15 +0000
>> @@ -298,6 +298,21 @@
>> self.assertEqual("working tree is out of date, run 'bzr update'\n",
>> + def test_status_write_lock(self):
>> + """That that status works without fetching history and
>> + having a write lock.
>> + See https://bugs.launchpad.net/bzr/+bug/149270
>> + """
>> + mkdir('branch1')
>> + wt = self.make_branch_and_tree('branch1')
> ^^^ mkdir is redundant with make_branch_and_tree
Hmm - the test failed without the mkdir so I ended up leaving it in.
>> + b = wt.branch
>> + wt.commit('Empty commit 1')
>> + wt2 = b.bzrdir.sprout('branch2').open_workingtree()
>> + wt2.commit('Empty commit 2', allow_pointless=True)
> ^^^ allow_pointless defaults to True.
More information about the bazaar