[MERGE][#144421] Fix `bzr st -rbranch:PATH_TO_BRANCH`

[Ian Clatworthy]

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
revisions.

>> @@ -1836,11 +1852,8 @@
>>              relpath = u''
>>          elif relpath:
>>              relpath += '/'
>> -        if revision is not None:
>> -            tree = branch.repository.revision_tree(
>> -                revision[0].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.
> 
> e.g.
> def _get_one_tree(revisions, branch, tree)
>     if revisions is None:
>         if tree is not None:
>             return tree
>         return branch.basis_tree()
>     else:
>         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.

Ditto.


>> === 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 @@
>>          os.chdir('subdir')
>>          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.

tweaked.

>> === 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",
>>                           err)
> 
>> +    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.

Fixed.

Ian C.