Comment 1 for bug 608114

Revision history for this message
Jonathan Lange (jml) wrote :

Here's a cheap way to fix this bug:

=== modified file 'lib/lp/code/model/diff.py'
--- lib/lp/code/model/diff.py 2010-08-02 02:13:52 +0000
+++ lib/lp/code/model/diff.py 2010-08-03 18:32:23 +0000
@@ -174,7 +174,7 @@ class Diff(SQLBase):
         """
         diff_content = StringIO()
         show_diff_trees(from_tree, to_tree, diff_content, old_label='',
- new_label='')
+ new_label='', external_diff_options='-p')
         size = diff_content.tell()
         diff_content.seek(0)
         return klass.fromFile(diff_content, size, filename)

This will change Launchpad from using Bazaar's own diff generator to using GNU diff. That introduces two new problems:
  1. GNU diff produces worse diffs than Bazaar does, so this change would lower the overall quality of Launchpad's diffs.
  2. GNU diff produces different diffs to Bazaar's default, so this change might surprise users who were expecting their local diffs to match Launchpad's.

Personally, I don't think 2 is a big deal, but I think 1 is, which means I won't try to land the patch.

As I see it, this leaves three options:
  1. Change GNU diff to produce the same kinds of diffs Bazaar does.
  2. Change bzr diff to have support for equivalent behaviour to --show-c-functions
  3. Someone else who is more motivated than I can try to land the patch.

Hope this helps,
jml