Rev 6600: (richard-wilbur) Split diff format option parser into a separate function, in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

[Patch Queue Manager]

At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6600 [merge]
revision-id: pqm at pqm.ubuntu.com-20140922194230-y32j0sq621bxhp7c
parent: pqm at pqm.ubuntu.com-20140922191434-6bbnpnxi5jab4vim
parent: richard.wilbur at gmail.com-20140916134223-sgipeo145y7i4bri
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2014-09-22 19:42:30 +0000
message:
  (richard-wilbur) Split diff format option parser into a separate function,
   update to include all format options for GNU diff v3.2,
   and test parser.  Fixes lp:1370435 (Richard Wilbur)
modified:
  bzrlib/diff.py                 diff.py-20050309040759-26944fbbf2ebbf36
  bzrlib/tests/test_diff.py      testdiff.py-20050727164403-d1a3496ebb12e339
=== modified file 'bzrlib/diff.py'
--- a/bzrlib/diff.py	2012-07-24 16:56:07 +0000
+++ b/bzrlib/diff.py	2014-09-16 13:42:23 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005-2011 Canonical Ltd.
+# Copyright (C) 2005-2014 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
@@ -119,7 +119,7 @@
 
 
 def _spawn_external_diff(diffcmd, capture_errors=True):
-    """Spawn the externall diff process, and return the child handle.
+    """Spawn the external diff process, and return the child handle.
 
     :param diffcmd: The command list to spawn
     :param capture_errors: Capture stderr as well as setting LANG=C
@@ -154,6 +154,40 @@
 
     return pipe
 
+# diff style options as of GNU diff v3.2
+style_option_list = ['-c', '-C', '--context',
+                     '-e', '--ed',
+                     '-f', '--forward-ed',
+                     '-q', '--brief',
+                     '--normal',
+                     '-n', '--rcs',
+                     '-u', '-U', '--unified',
+                     '-y', '--side-by-side',
+                     '-D', '--ifdef']
+
+def default_style_unified(diff_opts):
+    """Default to unified diff style if alternative not specified in diff_opts.
+
+        diff only allows one style to be specified; they don't override.
+        Note that some of these take optargs, and the optargs can be
+        directly appended to the options.
+        This is only an approximate parser; it doesn't properly understand
+        the grammar.
+
+    :param diff_opts: List of options for external (GNU) diff.
+    :return: List of options with default style=='unified'.
+    """
+    for s in style_option_list:
+        for j in diff_opts:
+            if j.startswith(s):
+                break
+        else:
+            continue
+        break
+    else:
+        diff_opts.append('-u')
+    return diff_opts
+
 
 def external_diff(old_filename, oldlines, new_filename, newlines, to_file,
                   diff_opts):
@@ -195,26 +229,7 @@
                    '--binary',
                   ]
 
-        # diff only allows one style to be specified; they don't override.
-        # note that some of these take optargs, and the optargs can be
-        # directly appended to the options.
-        # this is only an approximate parser; it doesn't properly understand
-        # the grammar.
-        for s in ['-c', '-u', '-C', '-U',
-                  '-e', '--ed',
-                  '-q', '--brief',
-                  '--normal',
-                  '-n', '--rcs',
-                  '-y', '--side-by-side',
-                  '-D', '--ifdef']:
-            for j in diff_opts:
-                if j.startswith(s):
-                    break
-            else:
-                continue
-            break
-        else:
-            diffcmd.append('-u')
+        diff_opts = default_style_unified(diff_opts)
 
         if diff_opts:
             diffcmd.extend(diff_opts)
@@ -265,7 +280,7 @@
                 msg = 'exit code %d' % rc
 
             raise errors.BzrError('external diff failed with %s; command: %r'
-                                  % (rc, diffcmd))
+                                  % (msg, diffcmd))
 
 
     finally:
@@ -282,7 +297,7 @@
                         old_abspath, e)
         try:
             os.remove(new_abspath)
-        except OSError:
+        except OSError, e:
             if e.errno not in (errno.ENOENT,):
                 warning('Failed to delete temporary file: %s %s',
                         new_abspath, e)

=== modified file 'bzrlib/tests/test_diff.py'
--- a/bzrlib/tests/test_diff.py	2012-07-12 15:59:35 +0000
+++ b/bzrlib/tests/test_diff.py	2014-09-16 13:42:23 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005-2011 Canonical Ltd
+# Copyright (C) 2005-2014 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
@@ -143,6 +143,19 @@
                           'old', ['boo\n'], 'new', ['goo\n'],
                           StringIO(), diff_opts=['-u'])
 
+    def test_default_style_unified(self):
+        """Check for default style '-u' only if no other style specified
+        in 'diff-options'.
+        """
+        # Verify that style defaults to unified, id est '-u' appended
+        # to option list, in the absence of an alternative style.
+        self.assertEqual(['-a', '-u'], diff.default_style_unified(["-a"]))
+        # Verify that for all valid style options, '-u' is not
+        # appended to option list.
+        for s in diff.style_option_list:
+            ret_opts = diff.default_style_unified(diff_opts=["%s" % (s,)])
+            self.assertEqual(["%s" % (s,)], ret_opts)
+
     def test_internal_diff_default(self):
         # Default internal diff encoding is utf8
         output = StringIO()
@@ -1391,7 +1404,7 @@
         diff_obj._execute('old', 'new')
         self.assertEqual(output.getvalue().rstrip(), 'old new')
 
-    def test_excute_missing(self):
+    def test_execute_missing(self):
         diff_obj = diff.DiffFromTool(['a-tool-which-is-unlikely-to-exist'],
                                      None, None, None)
         self.addCleanup(diff_obj.finish)