Merge lp:~anteru/bzr/234708-diff into lp:bzr

[Matthäus G. Chajdas]

accept both --diff-options and --using

Support both --using and --diff-options

[Vincent Ladeuil (vila)]

Two things:

- NEWS entries are sorted, put yours first since it starts with 'Allow',

- look at bzrlib.tests.features, there is some support available to define a feature that can check for the availability of an executable (sed, bash). That would allow you to write the test without relying on a particular path (/usr/bin/diff)

review: Needs Fixing

[Vincent Ladeuil (vila)]

I added a diff_feature as suggested above, the result is at lp:~vila/bzr/234708-diff-using-and-diff-options

You can pull or merge the above branch and resubmit.

We need a second reviewer here that could land the result if approved.

Forget my remark about NEWS, launchpad doesn't use the news_merge plugin that should take care of the conflict, your branch has a single entry in the bugs section so nothing you can do to avoid this conflict ;)

[Vincent Ladeuil (vila)]

Well done !

We need a second reviewer now.

review: Approve

[Martin Pool (mbp)]

very nice, thanks. sorry for the long delay.

review: Approve

[Martin Pool (mbp)]

sent to pqm by email

[Matthäus G. Chajdas (anteru)]

Cool, thanks -- that's my first contribution to Bazaar, and I must say that the process is really painless.

[Martin Pool (mbp)]

there were conflicts; i've addressed them and resubmitted from lp:~mbp/bzr/integration.

=== modified file 'NEWS'
--- NEWS	2010-07-21 22:29:45 +0000
+++ NEWS	2010-07-22 13:21:47 +0000
@@ -28,6 +28,7 @@
 Bug Fixes
 *********
 
+<<<<<<< TREE
 * Check if both --diff-options and --using are set, and exit with error
   in this case. (Matthäus G. Chajdas, #234708)
 
@@ -44,6 +45,10 @@
   to a symlink, now returns information about the symlink.
   (Martin Pool)
 
+=======
+* Allow using both --using and --diff-options. (Matthäus G. Chajdas, #234708)
+
+>>>>>>> MERGE-SOURCE
 Improvements
 ************
 

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2010-07-20 10:18:05 +0000
+++ bzrlib/builtins.py	2010-07-22 13:21:47 +0000
@@ -1879,12 +1879,16 @@
         Same as 'bzr diff' but prefix paths with old/ and new/::
 
             bzr diff --prefix old/:new/
+            
+        Show the differences using a custom diff program with options::
+        
+            bzr diff --using /usr/bin/diff --diff-options -wu
     """
     _see_also = ['status']
     takes_args = ['file*']
     takes_options = [
         Option('diff-options', type=str,
-               help='Pass these options to the diff program.'),
+               help='Pass these options to the external diff program.'),
         Option('prefix', type=str,
                short_name='p',
                help='Set prefixes added to old and new filenames, as '
@@ -1931,10 +1935,6 @@
                 '--prefix expects two values separated by a colon'
                 ' (eg "old/:new/")')
 
-        if using is not None and diff_options is not None:
-            raise errors.BzrCommandError(
-            '--diff-options and --using are mutually exclusive.')
-
         if revision and len(revision) > 2:
             raise errors.BzrCommandError('bzr diff --revision takes exactly'
                                          ' one or two revision specifiers')

=== modified file 'bzrlib/diff.py'
--- bzrlib/diff.py	2010-07-16 15:20:17 +0000
+++ bzrlib/diff.py	2010-07-22 13:21:47 +0000
@@ -736,9 +736,12 @@
                      path_encoding)
 
     @classmethod
-    def make_from_diff_tree(klass, command_string):
+    def make_from_diff_tree(klass, command_string, external_diff_options=None):
         def from_diff_tree(diff_tree):
-            return klass.from_string(command_string, diff_tree.old_tree,
+            full_command_string = [command_string]
+            if external_diff_options is not None:
+                full_command_string += ' ' + external_diff_options
+            return klass.from_string(full_command_string, diff_tree.old_tree,
                                      diff_tree.new_tree, diff_tree.to_file)
         return from_diff_tree
 
@@ -912,7 +915,7 @@
         :param using: Commandline to use to invoke an external diff tool
         """
         if using is not None:
-            extra_factories = [DiffFromTool.make_from_diff_tree(using)]
+            extra_factories = [DiffFromTool.make_from_diff_tree(using, external_diff_options)]
         else:
             extra_factories = []
         if external_diff_options:

=== modified file 'bzrlib/tests/blackbox/test_diff.py'
--- bzrlib/tests/blackbox/test_diff.py	2010-07-18 06:54:01 +0000
+++ bzrlib/tests/blackbox/test_diff.py	2010-07-22 13:21:47 +0000
@@ -29,6 +29,9 @@
     DiffTree,
     format_registry as diff_format_registry,
     )
+from bzrlib.tests import (
+    features,
+    )
 
 
 def subst_dates(string):
@@ -155,10 +158,6 @@
         self.assertContainsRe(err,
             "Requested revision: '1.1' does not exist in branch:")
 
-    def test_diff_diff_options_and_using(self):
-        out, err = self.run_bzr('diff --diff-options -wu --using /usr/bin/diff', retcode=3,
-          error_regexes=('are mutually exclusive.',))
-
     def test_diff_unversioned(self):
         # Get an error when diffing a non-versioned file.
         # (Malone #3619)
@@ -403,6 +402,16 @@
         self.assertEndsWith(out, "\n@@ -0,0 +1 @@\n"
                                  "+baz\n\n")
 
+    def test_external_diff_options_and_using(self):
+        """Test that the options are passed correctly to an external diff process"""
+        self.requireFeature(features.diff_feature)
+        self.make_example_branch()
+        self.build_tree_contents([('hello', 'Foo\n')])
+        out, err = self.run_bzr('diff --diff-options -i --using diff',
+                                    retcode=1)
+        self.assertEquals("=== modified file 'hello'\n", out)
+        self.assertEquals('', err)
+
 
 class TestDiffOutput(DiffBase):
 

=== modified file 'bzrlib/tests/features.py'
--- bzrlib/tests/features.py	2010-07-05 12:40:28 +0000
+++ bzrlib/tests/features.py	2010-07-22 13:21:47 +0000
@@ -114,3 +114,4 @@
 
 bash_feature = ExecutableFeature('bash')
 sed_feature = ExecutableFeature('sed')
+diff_feature = ExecutableFeature('diff')