[patch] small refactoring of cmd_diff

Martin Pool mbp at canonical.com
Wed Dec 20 11:18:54 GMT 2006


Here is a small and straightforward cleanup of cmd_diff to concentrate
the ui dwim behaviour.

Perhaps I should just remove the old parameters from diff_cmd_helper?

-- 
Martin
-------------- next part --------------
# Bazaar revision bundle v0.8
#
# message:
#   Refactor cmd_diff
#   
# committer: Martin Pool <mbp at sourcefrog.net>
# date: Wed 2006-12-20 21:58:12.331000090 +1100

=== modified file bzrlib/builtins.py
--- bzrlib/builtins.py
+++ bzrlib/builtins.py
@@ -1213,9 +1213,13 @@
     #       deleted files.
 
     # TODO: This probably handles non-Unix newlines poorly.
-    
+
     takes_args = ['file*']
-    takes_options = ['revision', 'diff-options', 'prefix']
+    takes_options = ['revision', 'diff-options',
+        Option('prefix', type=str,
+               help='Set prefixes to added to old and new filenames, as '
+                    'two values separated by a colon.'),
+        ]
     aliases = ['di', 'dif']
     encoding_type = 'exact'
 
@@ -1231,11 +1235,15 @@
         elif prefix == '1':
             old_label = 'old/'
             new_label = 'new/'
-        else:
-            if not ':' in prefix:
-                 raise BzrCommandError(
-                     "--diff-prefix expects two values separated by a colon")
+        elif ':' in prefix:
             old_label, new_label = prefix.split(":")
+        else:
+            raise BzrCommandError(
+                "--prefix expects two values separated by a colon")
+
+        if revision and len(revision) > 2:
+            raise errors.BzrCommandError('bzr diff --revision takes exactly'
+                                         ' one or two revision specifiers')
         
         try:
             tree1, file_list = internal_tree_files(file_list)
@@ -1261,29 +1269,23 @@
                 tree1, tree2 = None, None
             else:
                 raise
-        if revision is not None:
-            if tree2 is not None:
-                raise errors.BzrCommandError("Can't specify -r with two branches")
-            if (len(revision) == 1) or (revision[1].spec is None):
-                return diff_cmd_helper(tree1, file_list, diff_options,
-                                       revision[0], 
-                                       old_label=old_label, new_label=new_label)
-            elif len(revision) == 2:
-                return diff_cmd_helper(tree1, file_list, diff_options,
-                                       revision[0], revision[1],
-                                       old_label=old_label, new_label=new_label)
-            else:
-                raise errors.BzrCommandError('bzr diff --revision takes exactly'
-                                             ' one or two revision identifiers')
-        else:
-            if tree2 is not None:
-                return show_diff_trees(tree1, tree2, sys.stdout, 
-                                       specific_files=file_list,
-                                       external_diff_options=diff_options,
-                                       old_label=old_label, new_label=new_label)
-            else:
-                return diff_cmd_helper(tree1, file_list, diff_options,
-                                       old_label=old_label, new_label=new_label)
+
+        if tree2 is not None:
+            if revision is not None:
+                # FIXME: but there should be a clean way to diff between
+                # non-default versions of two trees, it's not hard to do
+                # internally...
+                raise errors.BzrCommandError(
+                        "Sorry, diffing arbitrary revisions across branches "
+                        "is not implemented yet")
+            return show_diff_trees(tree1, tree2, sys.stdout, 
+                                   specific_files=file_list,
+                                   external_diff_options=diff_options,
+                                   old_label=old_label, new_label=new_label)
+
+        return diff_cmd_helper(tree1, file_list, diff_options,
+                               revision_specs=revision,
+                               old_label=old_label, new_label=new_label)
 
 
 class cmd_deleted(Command):

=== modified file bzrlib/diff.py
--- bzrlib/diff.py
+++ bzrlib/diff.py
@@ -306,26 +306,32 @@
 
 def diff_cmd_helper(tree, specific_files, external_diff_options, 
                     old_revision_spec=None, new_revision_spec=None,
+                    revision_specs=None,
                     old_label='a/', new_label='b/'):
     """Helper for cmd_diff.
 
-   tree 
+    :param tree:
         A WorkingTree
 
-    specific_files
+    :param specific_files:
         The specific files to compare, or None
 
-    external_diff_options
+    :param external_diff_options:
         If non-None, run an external diff, and pass it these options
 
-    old_revision_spec
+    :param old_revision_spec:
         If None, use basis tree as old revision, otherwise use the tree for
         the specified revision. 
 
-    new_revision_spec
+    :param new_revision_spec:
         If None, use working tree as new revision, otherwise use the tree for
         the specified revision.
     
+    :param revision_specs: 
+        Zero, one or two RevisionSpecs from the command line, saying what revisions 
+        to compare.  This can be passed as an alternative to the old_revision_spec 
+        and new_revision_spec parameters.
+
     The more general form is show_diff_trees(), where the caller
     supplies any two trees.
     """
@@ -337,15 +343,26 @@
         revision_id = revision.rev_id
         branch = revision.branch
         return branch.repository.revision_tree(revision_id)
+
+    if revision_specs is not None:
+        assert (old_revision_spec is None
+                and new_revision_spec is None)
+        if len(revision_specs) > 0:
+            old_revision_spec = revision_specs[0]
+        if len(revision_specs) > 1:
+            new_revision_spec = revision_specs[1]
+
     if old_revision_spec is None:
         old_tree = tree.basis_tree()
     else:
         old_tree = spec_tree(old_revision_spec)
 
-    if new_revision_spec is None:
+    if (new_revision_spec is None
+        or new_revision_spec.spec is None):
         new_tree = tree
     else:
         new_tree = spec_tree(new_revision_spec)
+
     if new_tree is not tree:
         extra_trees = (tree,)
     else:

=== modified file bzrlib/tests/blackbox/test_diff.py
--- bzrlib/tests/blackbox/test_diff.py
+++ bzrlib/tests/blackbox/test_diff.py
@@ -118,6 +118,10 @@
         out, err = self.runbzr('diff does-not-exist', retcode=3)
         self.assertContainsRe(err, 'not versioned.*does-not-exist')
 
+    def test_diff_illegal_revision_specifiers(self):
+        out, err = self.runbzr('diff -r 1..23..123', retcode=3)
+        self.assertContainsRe(err, 'one or two revision specifiers')
+
     def test_diff_unversioned(self):
         # Get an error when diffing a non-versioned file.
         # (Malone #3619)

=== modified directory  // last-changed:mbp at sourcefrog.net-20061220105812-7uob9
... 1i2tl5qfksd
# revision id: mbp at sourcefrog.net-20061220105812-7uob91i2tl5qfksd
# sha1: 08972a9f247650b6fc8de224af9d7e5131d0aacd
# inventory sha1: 00e4e1caf6a1000876f20c87d81fdfbff4f33a25
# parent ids:
#   pqm at pqm.ubuntu.com-20061219065855-deb1b86e0b392f15
# base id: pqm at pqm.ubuntu.com-20061219065855-deb1b86e0b392f15
# properties:
#   branch-nick: 56299-dash-c



More information about the bazaar mailing list