Code review comment for lp:~guilhem-bichot/bzr/bzr-pull-verbose-shows-tip-before-pull

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

GuilhemBichot wrote:
> Hi John. By "Only show revids when there isn't another way" do you mean that "bzr pull -v" should display the old tip's revid only if this revision isn't present in the branch anymore?
> Or do you mean that we should just print revid of old and new tips, and no revnos, no matter if the old revision exists or not? Or something else (if you have an example)?

Print the dotted revno unless the old tip is no longer in the history,
then display revid:...

Just catch the exception, and print a different message indicating that
the old revision isn't in the ancestry any more, and this is what it was.

I'll put a simple example in my branch:
> === modified file 'bzrlib/builtins.py'
> --- bzrlib/builtins.py 2009-12-08 17:48:04 +0000
> +++ bzrlib/builtins.py 2009-12-09 17:29:36 +0000
> @@ -1031,11 +1031,16 @@
> % (target, result.old_revid, result.new_revid))
> result.report(self.outf)
> if verbose and result.old_revid != result.new_revid:
> - old_dotted_revno = branch_to.revision_id_to_dotted_revno(
> - result.old_revid)
> - old_dotted_revno = '.'.join([str(i) for i in old_dotted_revno])
> - self.outf.write('Tip revision before pull was: %s\n'
> - % (old_dotted_revno,))
> + try:
> + old_revno_in_to = branch_to.revision_id_to_dotted_revno(
> + result.old_revid)
> + old_revstr = '.'.join([str(i) for i in old_revno_in_to])
> + except errors.NoSuchRevision:
> + self.outf.write('Tip revision no longer in ancestry.'
> + ' Was revid:%s\n' % (result.old_revid,))
> + else:
> + self.outf.write('Tip revision before pull was: %s\n'
> + % (old_revstr,))
> log.show_branch_change(
> branch_to, self.outf, result.old_revno,
> result.old_revid)
>
> === modified file 'bzrlib/tests/blackbox/test_pull.py'
> --- bzrlib/tests/blackbox/test_pull.py 2009-12-08 17:41:06 +0000
> +++ bzrlib/tests/blackbox/test_pull.py 2009-12-09 17:29:36 +0000
> @@ -415,6 +415,23 @@
> out = self.run_bzr('pull -d target -v')[0]
> self.assertContainsRe(out, 'Tip revision before pull was: 1.1.2')
>
> + def test_pull_verbose_prints_not_in_ancestry_tip(self):
> + builder = self.make_branch_builder('source')
> + builder.start_series()
> + builder.build_snapshot('A', None, [
> + ('add', ('', 'TREE_ROOT', 'directory', None))])
> + builder.build_snapshot('B', ['A'], [])
> + builder.build_snapshot('C', ['B'], [])
> + builder.build_snapshot('D', ['A', 'B'], [])
> + builder.finish_series()
> + b = builder.get_branch()
> + b.lock_write()
> + self.addCleanup(b.unlock)
> + b.bzrdir.sprout('target', revision_id='C')
> + out = self.run_bzr('pull -d target -v --overwrite')[0]
> + self.assertContainsRe(out, 'Tip revision no longer in ancestry.'
> + ' Was revid:C')
> +
> def test_pull_verbose_does_not_print_tip_when_nothing_is_pulled(self):
> parent = self.make_branch_and_tree('parent')
> rev1 = parent.commit(message='first commit', rev_id='number-one')

You can get it from lp:///~jameinel/bzr/2.1.0b4-pull-v

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

iEYEARECAAYFAksf3tMACgkQJdeBCYSNAANZiACfR+fiyif2MlHBeXQkmz0U2sJR
PWgAnRwvmuj0Ic4o/Z93uxxTphF5Xx6k
=0p75
-----END PGP SIGNATURE-----