Merge lp:~ajeans/bzr/status-callback into lp:bzr

[Arnaud Jeansen]

(mbp, for ajeans) refactor status code

As explained on the mailing list, the "status" command output is implemented inside TreeDelta, which makes customization difficult (see https://lists.ubuntu.com/archives/bazaar/2010q1/067157.html )
This branch is a simple effort to :
- relocate the output from delta.py to status.py and pass the behaviour to TreeDelta with a callback
- the output code handles normal and short style, but the short style is never called (and would not pass the unit tests if called). As reported on the list, the short style is handled by the _ChangeReporter class (see https://lists.ubuntu.com/archives/bazaar/2010q1/067271.html )

[Martin Pool (mbp)]

Nice cleanup, thanks very much.

Can you please execute our contributor agreement at <http://www.canonical.com/contributors>?

review: Approve

[Martin Pool (mbp)]

agreement received

[Martin Pool (mbp)]

Fed to pqm

star-merge http://bazaar.launchpad.net/~ajeans/bzr/status-callback http://bazaar.launchpad.net/~bzr-pqm/bzr/bzr.dev

[Martin Pool (mbp)]

This seems to fail like so:

Traceback (most recent call last):
 File "/usr/lib/python2.4/site-packages/testtools/runtest.py", line 128, in _run_user
   return fn(*args)
 File "/usr/lib/python2.4/site-packages/testtools/testcase.py", line 368, in _run_test_method
   testMethod()
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/blackbox/test_log.py", line 424, in test_log_long_verbose
   self.assertUseLongDeltaFormat(['log', '--long', '-v'])
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/blackbox/test_log.py", line 410, in assertUseLongDeltaFormat
   log = self.run_bzr(cmd)[0]
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 1842, in run_bzr
   working_dir=working_dir,
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 1741, in _run_bzr_autosplit
   encoding=encoding, stdin=stdin, working_dir=working_dir,
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 1775, in _run_bzr_core
   args)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 2080, in apply_redirected
   return a_callable(*args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1146, in run_bzr_catch_user_errors
   return run_bzr(argv)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1039, in run_bzr
   ret = run(*run_argv)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 643, in run_argv_aliases
   return self.run_direct(**all_cmd_args)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 647, in run_direct
   return self._operation.run_simple(*args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/cleanup.py", line 121, in run_simple
   return _do_with_cleanups(
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/cleanup.py", line 156, in _do_with_cleanups
   result = func(*args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1054, in ignore_pipe
   result = func(*args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/builtins.py", line 2412, in run
   Logger(b, rqst).show(lf)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/log.py", line 335, in show
   self._show_body(lf)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/log.py", line 360, in _show_body
   lf.log_revision(lr)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/log.py", line 1521, in log_revision
   revision.delta.show(to_file, self.show_ids, indent=indent,
AttributeError: 'TreeDelta' object has no attribute 'show'

[Arnaud Jeansen (ajeans)]

> This seems to fail like so:
>
> Traceback (most recent call last):
> File "/usr/lib/python2.4/site-packages/testtools/runtest.py", line 128, in
> _run_user
> return fn(*args)
> File "/usr/lib/python2.4/site-packages/testtools/testcase.py", line 368, in
> _run_test_method
> testMethod()
> File "/home/pqm/bzr-pqm-
> workdir/home/+trunk/bzrlib/tests/blackbox/test_log.py", line 424, in
> test_log_long_verbose
> self.assertUseLongDeltaFormat(['log', '--long', '-v'])
> File "/home/pqm/bzr-pqm-
> workdir/home/+trunk/bzrlib/tests/blackbox/test_log.py", line 410, in
> assertUseLongDeltaFormat
> log = self.run_bzr(cmd)[0]
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line
> 1842, in run_bzr
> working_dir=working_dir,
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line
> 1741, in _run_bzr_autosplit
> encoding=encoding, stdin=stdin, working_dir=working_dir,
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line
> 1775, in _run_bzr_core
> args)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line
> 2080, in apply_redirected
> return a_callable(*args, **kwargs)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1146,
> in run_bzr_catch_user_errors
> return run_bzr(argv)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1039,
> in run_bzr
> ret = run(*run_argv)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 643, in
> run_argv_aliases
> return self.run_direct(**all_cmd_args)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 647, in
> run_direct
> return self._operation.run_simple(*args, **kwargs)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/cleanup.py", line 121, in
> run_simple
> return _do_with_cleanups(
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/cleanup.py", line 156, in
> _do_with_cleanups
> result = func(*args, **kwargs)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1054,
> in ignore_pipe
> result = func(*args, **kwargs)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/builtins.py", line 2412,
> in run
> Logger(b, rqst).show(lf)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/log.py", line 335, in show
> self._show_body(lf)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/log.py", line 360, in
> _show_body
> lf.log_revision(lr)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/log.py", line 1521, in
> log_revision
> revision.delta.show(to_file, self.show_ids, indent=indent,
> AttributeError: 'TreeDelta' object has no attribute 'show'

Martin,

I didn't see that "log" also called the TreeDelta.show(). It is probably used to do "log -v" and I didn't run the log unit tests (I only checked that all the status unit tests still passed).

Can you please revert the change for now, I will look at porting the log code as soon as I can, and update the merge proposal when it is done.

Sorry for the problem.
Arnaud

[Martin Pool (mbp)]

No need to revert, this is just not merged for now. See if you can get it to pass the log tests in your branch, then push to Launchpad again and add a note on this proposal. If you want help fixing the tests just ask.

[Arnaud Jeansen (ajeans)]

> No need to revert, this is just not merged for now. See if you can get it to
> pass the log tests in your branch, then push to Launchpad again and add a note
> on this proposal. If you want help fixing the tests just ask.

Martin,
It is impressive to see pre_merge hooks in action. At least I do not have to feel too guilty if I break something. :)

I updated the branch and tested it with a bigger subset of unit tests (including the one that failed). I also grepped through the tree to make sure I left nothing behind.
I'll let the whole test suite run on my laptop overnight, but last time I tried, it core dumped in the middle of it so I am not entirely sure that it will be useful.

As for the code, its only goal now is to expose status output as a callback (the dead code was not dead yet). However, I will probably have to file a bug regarding the different short outputs between status and log :
status uses ChangeReporter on a change_iterator whereas log uses a method on a TreeDelta. And they disagree on a few things (is short for 'added' a 'N' or a 'A', ...)

Thanks for your patience.

[Arnaud Jeansen (ajeans)]

The test suite on my computer is definitely not working well. It core dumped during the night because of a lack of free memory.

Aborted (core dumped), 854 failed] bzrlib.plugins.svn.tests.test_config.SvnBpCo

Among the tests at the beginning, some fail saying that they can't operate on a bare repo, and others saying that they couldn't create a new thread.

[Martin Pool (mbp)]

On 20 March 2010 18:33, Arnaud Jeansen <email address hidden> wrote:
> The test suite on my computer is definitely not working well. It core dumped during the night because of a lack of free memory.
>
> Aborted (core dumped), 854 failed] bzrlib.plugins.svn.tests.test_config.SvnBpCo
>
> Among the tests at the beginning, some fail saying that they can't operate on a bare repo, and others saying that they couldn't create a new thread.

If I were you I would try running with 'bzr --no-plugins selftest',
and also just selectively run the tests with 'bzr selftest log'.

If you get stuck just ask.

--
Martin <http://launchpad.net/~mbp/>

[Arnaud Jeansen (ajeans)]

Martin,

I ran "./bzr --no-plugins selftest", which finished on the branch (yay) but with 612 errors (all of those due to no new thread (apparently due to thread leaks in
bzrlib.tests.blackbox.test_branch.TestBranchStacked.test_branch_stacked_from_smart_server).
None of the failures look related to the changes here.

I locally tested with the following:
./bzr selftest -v -s bzrlib.tests.test_status.TestStatus -s bzrlib.tests.blackbox.test_status.CheckoutStatus -s bzrlib.tests.blackbox.test_status.BranchStatus -s bzrlib.tests.blackbox.test_status.TestStatus -s bzrlib.tests.blackbox.test_status.TestStatusEncodings -s bzrlib.tests.test_delta.TestReportChanges -s bzrlib.tests.test_delta.TestDeltaShow -s bzrlib.tests.blackbox.test_log.TestLogVerbose -s bzrlib.tests.test_errors.TestErrors

This works fine here.

Martin, do you think this is enough ?

[Martin Pool (mbp)]

This looks reasonable to me.

review: Approve

[Martin Pool (mbp)]

fails again, sorry:

(it's not immediately obvious to me why your code would fail here)

Traceback (most recent call last):
 File "/usr/lib/python2.4/site-packages/testtools/runtest.py", line 128, in _run_user
   return fn(*args)
 File "/usr/lib/python2.4/site-packages/testtools/testcase.py", line 368, in _run_test_method
   testMethod()
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/blackbox/test_branch.py", line 301, in test_branch_stacked
   out, err = self.run_bzr(['branch', '--stacked', 'mainline',
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 1844, in run_bzr
   working_dir=working_dir,
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 1743, in _run_bzr_autosplit
   encoding=encoding, stdin=stdin, working_dir=working_dir,
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 1777, in _run_bzr_core
   args)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 2082, in apply_redirected
   return a_callable(*args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1190, in run_bzr_catch_user_errors
   return run_bzr(argv)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1082, in run_bzr
   ret = run(*run_argv)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 686, in run_argv_aliases
   return self.run_direct(**all_cmd_args)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 690, in run_direct
   return self._operation.run_simple(*args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/cleanup.py", line 121, in run_simple
   return _do_with_cleanups(
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/cleanup.py", line 156, in _do_with_cleanups
   result = func(*args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/builtins.py", line 1264, in run
   source_branch=br_from)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/bzrdir.py", line 1214, in sprout
   revision_id=revision_id, repository_policy=repository_policy)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/decorators.py", line 140, in read_locked
   result = unbound(self, *args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/branch.py", line 1168, in sprout
   repository_policy.configure_branch(result)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/bzrdir.py", line 3626, in configure_branch
   branch.set_stacked_on_url(stack_on)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/decorators.py", line 194, in write_locked
   result = unbound(self, *args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/branch.py", line 705, in set_stacked_on_url
   self._activate_fallback_location(url)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/branch.py", line 108, in _activate_fallback_location
   repo = self._get_fallback_repository(url)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/branch.py", line 219, in _get_fallback_repository
   url = urlutils.join(self.base, url)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/urlutils.py", line 122, in join
   path = base_split.path
AttributeError: 'tuple' object has ...

[Martin Pool (mbp)]

uh, it's unobvious because I pasted the wrong failure; it should be

Traceback (most recent call last):
 File "/usr/lib/python2.4/site-packages/testtools/runtest.py", line 128, in _run_user
   return fn(*args)
 File "/usr/lib/python2.4/site-packages/testtools/testcase.py", line 368, in _run_test_method
   testMethod()
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/test_delta.py", line 246, in test_kind_change
   self.show_string(delta))
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/test_delta.py", line 224, in show_string
   delta.show(to_file, *args, **kwargs)
AttributeError: 'TreeDelta' object has no attribute 'show'

[Arnaud Jeansen (ajeans)]

Martin,

Well this is a lot more painful than I expected for such a small change. I hope I am not taking too much of your time with the failed merge attempts...

Anyway, sorry for the slip, I didn't call bzrlib.tests.test_delta.TestChangesFrom in my tests. The fix is a one liner in test_delta.

Tested with
$./bzr selftest -v -s bzrlib.tests.test_status.TestStatus \
-s bzrlib.tests.blackbox.test_status.CheckoutStatus \
-s bzrlib.tests.blackbox.test_status.BranchStatus \
-s bzrlib.tests.blackbox.test_status.TestStatus \
-s bzrlib.tests.blackbox.test_status.TestStatusEncodings \
-s bzrlib.tests.test_delta.TestReportChanges \
-s bzrlib.tests.test_delta.TestChangesFrom \
-s bzrlib.tests.test_delta.TestDeltaShow \
-s bzrlib.tests.blackbox.test_log.TestLogVerbose \
-s bzrlib.tests.test_errors.TestErrors
[...]
----------------------------------------------------------------------
Ran 147 tests in 10.011s

OK

Can you feed it again to pqm ? I grepped for more ".show(" calls, and nothing more turned up. It *should* be ok.

Thanks,
Arnaud

[Martin Pool (mbp)]

that's ok, thanks for your persistence

=== modified file 'bzrlib/delta.py'
--- bzrlib/delta.py	2010-02-17 17:11:16 +0000
+++ bzrlib/delta.py	2010-03-30 17:54:35 +0000
@@ -107,112 +107,11 @@
                 return True
         return False
 
-
-    def show(self, to_file, show_ids=False, show_unchanged=False,
-             short_status=False, indent='',
-             filter=None):
-        """Output this delta in status-like form to to_file.
-
-        :param to_file: A file-like object where the output is displayed.
-
-        :param show_ids: Output the file ids if True.
-
-        :param show_unchanged: Output the unchanged files if True.
-
-        :param short_status: Single-line status if True.
-
-        :param indent: Added at the beginning of all output lines (for merged
-            revisions).
-
-        :param filter: A callable receiving a path and a file id and
-            returning True if the path should be displayed.
-        """
-
-        def decorate_path(path, kind, meta_modified=None):
-            if kind == 'directory':
-                path += '/'
-            elif kind == 'symlink':
-                path += '@'
-            if meta_modified:
-                path += '*'
-            return path
-
-        def show_more_renamed(item):
-            (oldpath, file_id, kind,
-             text_modified, meta_modified, newpath) = item
-            dec_new_path = decorate_path(newpath, kind, meta_modified)
-            to_file.write(' => %s' % dec_new_path)
-            if text_modified or meta_modified:
-                extra_modified.append((newpath, file_id, kind,
-                                       text_modified, meta_modified))
-
-        def show_more_kind_changed(item):
-            (path, file_id, old_kind, new_kind) = item
-            to_file.write(' (%s => %s)' % (old_kind, new_kind))
-
-        def show_path(path, file_id, kind, meta_modified,
-                      default_format, with_file_id_format):
-            dec_path = decorate_path(path, kind, meta_modified)
-            if show_ids:
-                to_file.write(with_file_id_format % dec_path)
-            else:
-                to_file.write(default_format % dec_path)
-
-        def show_list(files, long_status_name, short_status_letter,
-                      default_format='%s', with_file_id_format='%-30s',
-                      show_more=None):
-            if files:
-                header_shown = False
-                if short_status:
-                    prefix = short_status_letter
-                else:
-                    prefix = ''
-                prefix = indent + prefix + '  '
-
-                for item in files:
-                    path, file_id, kind = item[:3]
-                    if (filter is not None and not filter(path, file_id)):
-                        continue
-                    if not header_shown and not short_status:
-                        to_file.write(indent + long_status_name + ':\n')
-                        header_shown = True
-                    meta_modified = None
-                    if len(item) == 5:
-                        meta_modified = item[4]
-
-                    to_file.write(prefix)
-                    show_path(path, file_id, kind, meta_modified,
-                              default_format, with_file_id_format)
-                    if show_more is not None:
-                        show_more(item)
-                    if show_ids:
-                        to_file.write(' %s' % file_id)
-                    to_file.write('\n')
-
-        show_list(self.removed, 'removed', 'D')#
-        show_list(self.added, 'added', 'A')
-        extra_modified = []
-        # Reorder self.renamed tuples so that all lists share the same
-        # order for their 3 first fields and that they also begin like
-        # the self.modified tuples
-        renamed = [(p, i, k, tm, mm, np)
-                   for  p, np, i, k, tm, mm  in self.renamed]
-        show_list(renamed, 'renamed', 'R', with_file_id_format='%s',
-                  show_more=show_more_renamed)
-        show_list(self.kind_changed, 'kind changed', 'K',
-                  with_file_id_format='%s',
-                  show_more=show_more_kind_changed)
-        show_list(self.modified + extra_modified, 'modified', 'M')
-        if show_unchanged:
-            show_list(self.unchanged, 'unchanged', 'S')
-
-        show_list(self.unversioned, 'unknown', ' ')
-
     def get_changes_as_text(self, show_ids=False, show_unchanged=False,
-             short_status=False):
+                            short_status=False):
         import StringIO
         output = StringIO.StringIO()
-        self.show(output, show_ids, show_unchanged, short_status)
+        report_delta(output, self, short_status, show_ids, show_unchanged)
         return output.getvalue()
 
 
@@ -393,7 +292,6 @@
         self.output("%s%s%s %s%s", rename, self.modified_map[modified], exe,
                     old_path, path)
 
-
 def report_changes(change_iterator, reporter):
     """Report the changes from a change iterator.
 
@@ -437,3 +335,105 @@
         versioned_change = versioned_change_map[versioned]
         reporter.report(file_id, path, versioned_change, renamed, modified,
                         exe_change, kind)
+
+def report_delta(to_file, delta, short_status=False, show_ids=False, 
+         show_unchanged=False, indent='', filter=None):
+    """Output this delta in status-like form to to_file.
+
+    :param to_file: A file-like object where the output is displayed.
+
+    :param delta: A TreeDelta containing the changes to be displayed
+
+    :param short_status: Single-line status if True.
+
+    :param show_ids: Output the file ids if True.
+
+    :param show_unchanged: Output the unchanged files if True.
+
+    :param indent: Added at the beginning of all output lines (for merged
+        revisions).
+
+    :param filter: A callable receiving a path and a file id and
+        returning True if the path should be displayed.
+    """
+
+    def decorate_path(path, kind, meta_modified=None):
+        if kind == 'directory':
+            path += '/'
+        elif kind == 'symlink':
+            path += '@'
+        if meta_modified:
+            path += '*'
+        return path
+
+    def show_more_renamed(item):
+        (oldpath, file_id, kind,
+         text_modified, meta_modified, newpath) = item
+        dec_new_path = decorate_path(newpath, kind, meta_modified)
+        to_file.write(' => %s' % dec_new_path)
+        if text_modified or meta_modified:
+            extra_modified.append((newpath, file_id, kind,
+                                   text_modified, meta_modified))
+
+    def show_more_kind_changed(item):
+        (path, file_id, old_kind, new_kind) = item
+        to_file.write(' (%s => %s)' % (old_kind, new_kind))
+
+    def show_path(path, file_id, kind, meta_modified,
+                  default_format, with_file_id_format):
+        dec_path = decorate_path(path, kind, meta_modified)
+        if show_ids:
+            to_file.write(with_file_id_format % dec_path)
+        else:
+            to_file.write(default_format % dec_path)
+
+    def show_list(files, long_status_name, short_status_letter,
+                  default_format='%s', with_file_id_format='%-30s',
+                  show_more=None):
+        if files:
+            header_shown = False
+            if short_status:
+                prefix = short_status_letter
+            else:
+                prefix = ''
+            prefix = indent + prefix + '  '
+
+            for item in files:
+                path, file_id, kind = item[:3]
+                if (filter is not None and not filter(path, file_id)):
+                    continue
+                if not header_shown and not short_status:
+                    to_file.write(indent + long_status_name + ':\n')
+                    header_shown = True
+                meta_modified = None
+                if len(item) == 5:
+                    meta_modified = item[4]
+
+                to_file.write(prefix)
+                show_path(path, file_id, kind, meta_modified,
+                          default_format, with_file_id_format)
+                if show_more is not None:
+                    show_more(item)
+                if show_ids:
+                    to_file.write(' %s' % file_id)
+                to_file.write('\n')
+
+    show_list(delta.removed, 'removed', 'D')
+    show_list(delta.added, 'added', 'A')
+    extra_modified = []
+    # Reorder delta.renamed tuples so that all lists share the same
+    # order for their 3 first fields and that they also begin like
+    # the delta.modified tuples
+    renamed = [(p, i, k, tm, mm, np)
+               for  p, np, i, k, tm, mm  in delta.renamed]
+    show_list(renamed, 'renamed', 'R', with_file_id_format='%s',
+              show_more=show_more_renamed)
+    show_list(delta.kind_changed, 'kind changed', 'K',
+              with_file_id_format='%s',
+              show_more=show_more_kind_changed)
+    show_list(delta.modified + extra_modified, 'modified', 'M')
+    if show_unchanged:
+        show_list(delta.unchanged, 'unchanged', 'S')
+
+    show_list(delta.unversioned, 'unknown', ' ')
+

=== modified file 'bzrlib/log.py'
--- bzrlib/log.py	2010-03-19 10:16:06 +0000
+++ bzrlib/log.py	2010-03-30 17:54:35 +0000
@@ -1526,9 +1526,10 @@
         to_file = self.to_file
         to_file.write("%s%s\n" % (indent, ('\n' + indent).join(lines)))
         if revision.delta is not None:
-            # We don't respect delta_format for compatibility
-            revision.delta.show(to_file, self.show_ids, indent=indent,
-                                short_status=False)
+            # Use the standard status output to display changes
+            from bzrlib.delta import report_delta
+            report_delta(to_file, revision.delta, short_status=False, 
+                         show_ids=self.show_ids, indent=indent)
         if revision.diff is not None:
             to_file.write(indent + 'diff:\n')
             to_file.flush()
@@ -1597,8 +1598,11 @@
                 to_file.write(indent + offset + '%s\n' % (l,))
 
         if revision.delta is not None:
-            revision.delta.show(to_file, self.show_ids, indent=indent + offset,
-                                short_status=self.delta_format==1)
+            # Use the standard status output to display changes
+            from bzrlib.delta import report_delta
+            report_delta(to_file, revision.delta, 
+                         short_status=self.delta_format==1, 
+                         show_ids=self.show_ids, indent=indent + offset)
         if revision.diff is not None:
             self.show_diff(self.to_exact_file, revision.diff, '      ')
         to_file.write('\n')

=== modified file 'bzrlib/status.py'
--- bzrlib/status.py	2010-02-17 17:11:16 +0000
+++ bzrlib/status.py	2010-03-30 17:54:35 +0000
@@ -34,6 +34,54 @@
 # if known, but only if really going to the terminal (not into a file)
 
 
+def report_changes(to_file, old, new, specific_files, 
+                   show_short_reporter, show_long_callback, 
+                   short=False, want_unchanged=False, 
+                   want_unversioned=False, show_ids=False):
+    """Display summary of changes.
+
+    This compares two trees with regards to a list of files, and delegates 
+    the display to underlying elements.
+
+    For short output, it creates an iterator on all changes, and lets a given
+    reporter display these changes.
+
+    For stantard output, it creates a delta of the changes, and forwards it
+    to a callback
+
+    :param to_file: If set, write to this file (default stdout.)
+    :param old: Start tree for the comparison
+    :param end: End tree for the comparison
+    :param specific_files: If set, a list of filenames whose status should be
+        shown.  It is an error to give a filename that is not in the working
+        tree, or in the working inventory or in the basis inventory.
+    :param show_short_reporter: Reporter in charge of display for short output
+    :param show_long_callback: Callback in charge of display for normal output
+    :param short: If True, gives short SVN-style status lines.
+    :param want_unchanged: Deprecated parameter. If set, includes unchanged
+        files.
+    :param show_ids: If set, includes each file's id.
+    :param want_unversioned: If False, only shows versioned files.
+    """
+
+    if short:
+        changes = new.iter_changes(old, want_unchanged, specific_files,
+            require_versioned=False, want_unversioned=want_unversioned)
+        _mod_delta.report_changes(changes, show_short_reporter)
+        
+    else:
+        delta = new.changes_from(old, want_unchanged=want_unchanged,
+                              specific_files=specific_files,
+                              want_unversioned=want_unversioned)
+        # filter out unknown files. We may want a tree method for
+        # this
+        delta.unversioned = [unversioned for unversioned in
+            delta.unversioned if not new.is_ignored(unversioned[0])]
+        show_long_callback(to_file, delta, 
+                           show_ids=show_ids,
+                           show_unchanged=want_unchanged)
+
+
 def show_tree_status(wt, show_unchanged=None,
                      specific_files=None,
                      show_ids=False,
@@ -42,7 +90,8 @@
                      revision=None,
                      short=False,
                      verbose=False,
-                     versioned=False):
+                     versioned=False,
+                     show_long_callback=_mod_delta.report_delta):
     """Display summary of changes.
 
     By default this compares the working tree to a previous revision.
@@ -71,6 +120,8 @@
     :param verbose: If True, show all merged revisions, not just
         the merge tips
     :param versioned: If True, only shows versioned files.
+    :param show_long_callback: A callback: message = show_long_callback(to_file, delta, 
+        show_ids, show_unchanged, indent, filter), only used with the long output
     """
     if show_unchanged is not None:
         warn("show_tree_status with show_unchanged has been deprecated "
@@ -106,24 +157,15 @@
             specific_files, nonexistents \
                 = _filter_nonexistent(specific_files, old, new)
             want_unversioned = not versioned
-            if short:
-                changes = new.iter_changes(old, show_unchanged, specific_files,
-                    require_versioned=False, want_unversioned=want_unversioned)
-                reporter = _mod_delta._ChangeReporter(output_file=to_file,
-                    unversioned_filter=new.is_ignored)
-                _mod_delta.report_changes(changes, reporter)
-            else:
-                delta = new.changes_from(old, want_unchanged=show_unchanged,
-                                      specific_files=specific_files,
-                                      want_unversioned=want_unversioned)
-                # filter out unknown files. We may want a tree method for
-                # this
-                delta.unversioned = [unversioned for unversioned in
-                    delta.unversioned if not new.is_ignored(unversioned[0])]
-                delta.show(to_file,
-                           show_ids=show_ids,
-                           show_unchanged=show_unchanged,
-                           short_status=False)
+
+            # Reporter used for short outputs
+            reporter = _mod_delta._ChangeReporter(output_file=to_file,
+                unversioned_filter=new.is_ignored)
+            report_changes(to_file, old, new, specific_files, 
+                           reporter, show_long_callback, 
+                           short=short, want_unchanged=show_unchanged, 
+                           want_unversioned=want_unversioned, show_ids=show_ids)
+
             # show the new conflicts only for now. XXX: get them from the
             # delta.
             conflicts = new.conflicts()

=== modified file 'bzrlib/tests/test_delta.py'
--- bzrlib/tests/test_delta.py	2009-03-23 14:59:43 +0000
+++ bzrlib/tests/test_delta.py	2010-03-30 17:54:35 +0000
@@ -221,7 +221,7 @@
 
     def show_string(self, delta, *args,  **kwargs):
         to_file = StringIO()
-        delta.show(to_file, *args, **kwargs)
+        _mod_delta.report_delta(to_file, delta, *args, **kwargs)
         return to_file.getvalue()
 
     def test_kind_change(self):
@@ -307,13 +307,13 @@
     def test_delta_show_short_status_no_filter(self):
         d, long_status, short_status = self._get_delta()
         out = StringIO()
-        d.show(out, short_status=True)
+        _mod_delta.report_delta(out, d, short_status=True)
         self.assertEquals(short_status, out.getvalue())
 
     def test_delta_show_long_status_no_filter(self):
         d, long_status, short_status = self._get_delta()
         out = StringIO()
-        d.show(out, short_status=False)
+        _mod_delta.report_delta(out, d, short_status=False)
         self.assertEquals(long_status, out.getvalue())
 
     def test_delta_show_no_filter(self):
@@ -321,7 +321,7 @@
         out = StringIO()
         def not_a_filter(path, file_id):
             return True
-        d.show(out, short_status=True, filter=not_a_filter)
+        _mod_delta.report_delta(out, d, short_status=True, filter=not_a_filter)
         self.assertEquals(short_status, out.getvalue())
 
     def test_delta_show_short_status_single_file_filter(self):
@@ -329,7 +329,7 @@
         out = StringIO()
         def only_f2(path, file_id):
             return path == 'f2'
-        d.show(out, short_status=True, filter=only_f2)
+        _mod_delta.report_delta(out, d, short_status=True, filter=only_f2)
         self.assertEquals("A  f2\n", out.getvalue())
 
     def test_delta_show_long_status_single_file_filter(self):
@@ -337,7 +337,7 @@
         out = StringIO()
         def only_f2(path, file_id):
             return path == 'f2'
-        d.show(out, short_status=False, filter=only_f2)
+        _mod_delta.report_delta(out, d, short_status=False, filter=only_f2)
         self.assertEquals("added:\n  f2\n", out.getvalue())
 
     def test_delta_show_short_status_single_file_id_filter(self):
@@ -345,6 +345,6 @@
         out = StringIO()
         def only_f2_id(path, file_id):
             return file_id == 'f2-id'
-        d.show(out, short_status=True, filter=only_f2_id)
+        _mod_delta.report_delta(out, d, short_status=True, filter=only_f2_id)
         self.assertEquals("A  f2\n", out.getvalue())