[patch] #36963 better messages on commit

[Martin Pool]

This patch fixes the slightly silly "modified/renamed/reparented"
message shown in commit output.

I'd like to clean up some of this code rather more; the function which
is fixed seems a bit redundant.  But this should do for now, and it adds
more tests and documentation which will make it easier to refactor in the future.

-- 
Martin
-------------- next part --------------
=== modified file 'a/bzrlib/inventory.py'
--- a/bzrlib/inventory.py	
+++ b/bzrlib/inventory.py	
@@ -1,4 +1,4 @@
-# (C) 2005 Canonical Ltd
+# Copyright (C) 2005, 2006 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -348,18 +348,36 @@
         raise BzrCheckError('unknown entry kind %r in revision {%s}' % 
                             (self.kind, rev_id))
 
-
     def copy(self):
         """Clone this inventory entry."""
         raise NotImplementedError
 
-    def _get_snapshot_change(self, previous_entries):
+    def _describe_snapshot_change(self, previous_entries):
+        """Describe how this entry will have changed in a new commit.
+
+        :param previous_entries: Dictionary from revision_id to inventory entry.
+
+        :returns: One-word description: "merged", "added", "renamed", "modified".
+        """
+        # XXX: This assumes that the file *has* changed -- it should probably
+        # be fused with whatever does that detection.  Why not just a single
+        # thing to compare the entries?
+        #
+        # TODO: Return some kind of object describing all the possible
+        # dimensions that can change, not just a string.  That can then give
+        # both old and new names for renames, etc.
+        #
         if len(previous_entries) > 1:
             return 'merged'
         elif len(previous_entries) == 0:
             return 'added'
-        else:
-            return 'modified/renamed/reparented'
+        the_parent, = previous_entries.values()
+        if self.parent_id != the_parent.parent_id:
+            # actually, moved to another directory
+            return 'renamed'
+        elif self.name != the_parent.name:
+            return 'renamed'
+        return 'modified'
 
     def __repr__(self):
         return ("%s(%r, %r, parent_id=%r)"
@@ -384,15 +402,23 @@
                 mutter("found unchanged entry")
                 self.revision = parent_ie.revision
                 return "unchanged"
-        return self.snapshot_revision(revision, previous_entries, 
-                                      work_tree, weave_store, transaction)
-
-    def snapshot_revision(self, revision, previous_entries, work_tree,
-                          weave_store, transaction):
-        """Record this revision unconditionally."""
-        mutter('new revision for {%s}', self.file_id)
+        return self._snapshot_into_revision(revision, previous_entries, 
+                                            work_tree, weave_store, transaction)
+
+    def _snapshot_into_revision(self, revision, previous_entries, work_tree,
+                                weave_store, transaction):
+        """Record this revision unconditionally into a store.
+
+        The entry's last-changed revision property (`revision`) is updated to 
+        that of the new revision.
+        
+        :param revision: id of the new revision that is being recorded.
+
+        :returns: String description of the commit (e.g. "merged", "modified"), etc.
+        """
+        mutter('new revision {%s} for {%s}', revision, self.file_id)
         self.revision = revision
-        change = self._get_snapshot_change(previous_entries)
+        change = self._describe_snapshot_change(previous_entries)
         self._snapshot_text(previous_entries, work_tree, weave_store,
                             transaction)
         return change

=== modified file 'a/bzrlib/tests/blackbox/test_commit.py'
--- a/bzrlib/tests/blackbox/test_commit.py	
+++ b/bzrlib/tests/blackbox/test_commit.py	
@@ -17,14 +17,12 @@
 
 """Tests for the commit CLI of bzr."""
 
-from cStringIO import StringIO
 import os
 import re
-import shutil
 import sys
 
 from bzrlib.branch import Branch
-import bzrlib.bzrdir as bzrdir
+from bzrlib.bzrdir import BzrDir
 from bzrlib.errors import BzrCommandError
 from bzrlib.tests.blackbox import ExternalBase
 from bzrlib.workingtree import WorkingTree
@@ -50,12 +48,54 @@
                          'Committed revision 1.\n',
                          err)
 
-    def test_15_verbose_commit_with_unknown(self):
+    def prepare_simple_history(self):
+        """Prepare and return a working tree with one commit of one file"""
+        # Commit with modified file should say so
+        wt = BzrDir.create_standalone_workingtree('.')
+        self.build_tree(['hello.txt', 'extra.txt'])
+        wt.add(['hello.txt'])
+        wt.commit(message='added')
+        return wt
+
+    def test_verbose_commit_modified(self):
+        # Verbose commit of modified file should say so
+        wt = self.prepare_simple_history()
+        self.build_tree_contents([('hello.txt', 'new contents')])
+        out, err = self.run_bzr("commit", "-m", "modified")
+        self.assertEqual('', out)
+        self.assertEqual('modified hello.txt\n'
+                         'Committed revision 2.\n',
+                         err)
+
+    def test_verbose_commit_renamed(self):
+        # Verbose commit of renamed file should say so
+        wt = self.prepare_simple_history()
+        wt.rename_one('hello.txt', 'gutentag.txt')
+        out, err = self.run_bzr("commit", "-m", "renamed")
+        self.assertEqual('', out)
+        self.assertEqual('renamed gutentag.txt\n'
+                         'Committed revision 2.\n',
+                         err)
+
+    def test_verbose_commit_moved(self):
+        # Verbose commit of file moved to new directory should say so
+        wt = self.prepare_simple_history()
+        os.mkdir('subdir')
+        wt.add(['subdir'])
+        wt.rename_one('hello.txt', 'subdir/hello.txt')
+        out, err = self.run_bzr("commit", "-m", "renamed")
+        self.assertEqual('', out)
+        self.assertEqualDiff('added subdir\n'
+                             'renamed subdir/hello.txt\n'
+                             'Committed revision 2.\n',
+                             err)
+
+    def test_verbose_commit_with_unknown(self):
         """Unknown files should not be listed by default in verbose output"""
         # Is that really the best policy?
-        self.runbzr("init")
+        wt = BzrDir.create_standalone_workingtree('.')
         self.build_tree(['hello.txt', 'extra.txt'])
-        self.runbzr("add hello.txt")
+        wt.add(['hello.txt'])
         out,err = self.run_bzr("commit", "-m", "added")
         self.assertEqual('', out)
         self.assertEqual('added hello.txt\n'