Are you talking about the diffs in Differential only (for source code) or also the diffs for task descriptions and comments in Maniphest?

(Those diffs are also generated by diff, FWIW.)

T3353 has some discussion of implementing human-text (vs source code) diffing, which would improve the diffs for things like Phriction pages and description edits. This is likely not particularly difficult, but complex and hard to prioritize.

For what is worth, a similar task was created in https://phabricator.wikimedia.org/T78824 a couple of days ago -- contains screenshots comparing the results with wdiff.

How do you decide how to apply light-green vs dark-green highlighting? Is that something diff gives you?

And here's a simple diff that Phabricator displays poorly and some other tools display well:

• Phabricator:
  
• BitBucket:
  
• Diffmerge:
  
• IntelliJ:
  

With more complicated diffs, the difference is more pronounced. I end up having to arc patch so I can use Diffmerge locally to actually see what's going on.

Also, Python's built-in difflib also does the right thing (at least for this small example).

Where's the code that decides how to highlight columns?

We compute intraline diffs ourselves, but it's based on the output of the underlying diff tool. All of the other examples have a "better" base diff for this example. We're basically just using the output from git diff or diff in this case:

$ diff -u old new 
--- old	2014-12-19 13:30:13.000000000 -0800
+++ new	2014-12-19 13:30:38.000000000 -0800
@@ -1 +1,6 @@
-print "main program"
+def main():
+    print "main program"
+
+if __name__ == "__main__":
+    main()
+

Assuming that screenshot is from Diffusion, note that we use diff -bwu (or similar) in Differential (at least, by default), which produces a diff more similar to what you're looking for:

We don't ignore whitespace by default in Diffusion because the default configuration of Phabricator uses Differential as the primary code review application, and Diffusion more as a reference. T3498 discusses expanding the options in Diffusion.

You can find the main intraline diff algorithm here:

https://secure.phabricator.com/diffusion/PHU/browse/master/src/utils/PhutilEditDistanceMatrix.php

Haha, noobs calling everything "Phabricator".

The screenshot is from Differential, not Diffusion, but I created the diff by copy/pasting the output from diff -u. How did you create yours?

diff -bwu shows something similar to BitBucket's output, but I actually prefer Diffmerge and Intellij -- show whitespace changes but only highlight the parts of the line that have changed.

I don't think the fact that the input comes from diff is a big barrier.

1. You could still apply a smarter diff algorithm to each individual hunk. That would handle this example better, and probably many other common ones, too.
2. If the backing repo is available, you could apply the patch and then run a better diff tool. The cost of doing this is probably similar to the cost of doing syntax highlighting, maybe?

Yours is called .py so it gets special rules:

https://secure.phabricator.com/book/phabricator/article/differential_faq/#what-do-the-whitespace-o

If you select "Ignore All", or name it something other than .py, it will diff like mine.

Long ago we did not apply special rules to Python, but there was a great deal of grumbling and complaining and assertion that .py files must always be diffed with all whitespace preserved exactly and any other approach was wrong, so now we do it that way.

This is configurable with differential.whitespace-matters.

Been poking around at the code. The following function looks promising: https://secure.phabricator.com/diffusion/P/browse/master/src/applications/differential/parser/DifferentialHunkParser.php;a366f85c118bf6b857155792446c397dac03e0a3$229

It looks like this function could be modified to align the lines of a hunk to match up better, maybe by running a better diff algorithm first. Does that seem like the right approach? Is there a better place to hook into the diff pipeline?

I've identified that, at least for the diffs used for Maniphest task description changes, a significant part of the noise is caused by the 80 character line wrapping. Which seems to be applied to both old and new content *before* feeding it to the difference engine.

Example: https://phabricator.wikimedia.org/T94149

Note how the word "tests" is broken into tes ts in the before view, and test s in the after view. It's seeing changes to content that didn't at all change. If anything, it should perform such line breaking afterwards. Though ideally it would just leave it to the client instead with natural wrapping through the browser.

See T3353 for discussion of code vs text diffs.