[patch] fix bug 30190 "'bzr cat' requires full path in old revision"

[Martin Pool]

On 13 Oct 2006, Cheuksan Edward Wang <wang02139 at gmail.com> wrote:
> This changeset fixes bug 30190 "'bzr cat' requires full path in old revision".
> 
> ``bzr cat`` can look up contents of removed or renamed files. If the
> pathname is ambiguous, i.e. the files in the old and new trees have
> different id's, the user needs to use "--old" or "--new" to distinguish
> between them. If the pathname refers to a single file, the options are
> unnecessary.

Thanks.

> -        b.print_file(relpath, revision_id)
> +
> +        cur_file_id = tree.path2id(relpath)
> +        rev_tree = b.repository.revision_tree(revision_id)
> +        old_file_id = rev_tree.path2id(relpath)
> +        
> +        if new:
> +            if cur_file_id is None:
> +                raise errors.BzrCommandError("%r is not present in revision %s"
> +                                                % (filename, revision_id))
> +            else:
> +                rev_tree.print_file(cur_file_id)
> +                return
> +
> +        if old:
> +            if old_file_id is None:
> +                raise errors.BzrCommandError("%r is not present in revision %s"
> +                                                % (filename, revision_id))
> +            else:
> +                rev_tree.print_file(old_file_id)
> +                return
> +        
> +        if cur_file_id is not None:
> +            if old_file_id is not None:
> +                if cur_file_id == old_file_id:
> +                    rev_tree.print_file(cur_file_id)
> +                else:
> +                    raise errors.BzrCommandError('Old and new files are'
> +                                ' different. Please specify --old or --new')
> +            else:
> +                rev_tree.print_file(cur_file_id)
> +        elif old_file_id is not None:
> +            rev_tree.print_file(old_file_id)
> +        else:
> +            raise errors.BzrCommandError("%r is not present in revision %s" %
> +                                         (filename, revision_id))

Perhaps it's better to have

  if new
  elif old
  elif ...

than returning from multiple points - I think it makes the
alternatives a little more clear, but it's not a big deal.

> +    def test_cat_different_id(self): 
> +        """'cat' works with old and new files"""
> +
> +        def bzr(*args, **kwargs):
> +            return self.run_bzr(*args, **kwargs)[0]
> +
> +        bzr('init')
> +        open('a', 'wb').write('foo\n')
> +        open('c', 'wb').write('baz\n')
> +        open('d', 'wb').write('bar\n')
> +        bzr('add', 'a', 'c', 'd')
> +        bzr('commit', '-m', '1')
> +
> +        os.remove('d')
> +        bzr('remove', 'd');
> +        bzr('rename', 'a', 'b');
> +        bzr('rename', 'c', 'a');
> +        bzr('commit', '-m', '2')
> +
> +        # get to the old file automatically
> +        self.assertEquals(bzr('cat', 'd', '-r', '1'), 'bar\n')
> +
> +        self.assertEquals(bzr('cat', 'a', '-r', '1', '--old'), 'foo\n')
> +        self.assertEquals(bzr('cat', 'a', '-r', '1', '--new'), 'baz\n')

So, to be strictly TDD, if it's worth raising exceptions in the other
cases then you should add tests for them too.  Possibly these are so
obvious it's not worth testing but on the other hand they're easy to
test.

So +0.8 from me, pending more tests -- anyone else?

-- 
Martin