3

Let's say I have rewritten a C function from scratch. It often happens that some lines happen to be identical before and after, in particular blank lines and closing braces. When I create a unified diff (using git diff or plain GNU diff -u), these identical lines splits up the hunk, making the patch harder to read for the reviewer. Diff's ambition to produce the minimal diff sometimes sacrifices readability, which is not what I want. Is there a way to make diff sacrifice minimality to keep long hunks together?

Example: Consider this, produced by diff:


--- A   2018-10-01 09:37:37.606642955 +0200
+++ B   2018-10-01 09:37:40.405675295 +0200
@@ -1,6 +1,9 @@
 int fib(int n) {
-  if (n <= 1) {
-    return n;
+  int i, t1 = 0, t2 = 1;
+  for (i = 0; i < n; ++i) {
+    int next = t1 + t2;
+    t1 = t2;
+    t2 = next;
   }
-  return fib(n-1) + fib(n-2);
+  return t1;
 }
In my opinion, the following equivalent patch would have been easier to read, at the cost of just one extra line:

--- A   2018-10-01 09:37:37.606642955 +0200
+++ B   2018-10-01 09:37:40.405675295 +0200
@@ -1,6 +1,9 @@
 int fib(int n) {
-  if (n <= 1) {
-    return n;
-  }
-  return fib(n-1) + fib(n-2);
+  int i, t1 = 0, t2 = 1;
+  for (i = 0; i < n; ++i) {
+    int next = t1 + t2;
+    t1 = t2;
+    t2 = next;
+  }
+  return t1;
 }

So, one possible heuristic would be "if there is an identical line, and lines have been added and removed both before and after the line, then consider the identical line to be changed". I can of course create my own script that applies this rule on an existing patch, but is there an existing tool to solve my problem?

3

You could implement your approach by piping the diff -u output to:

perl -0777 -pe '1 while s{^-.*\n\K((?:\+.*\n)+) ((.*\n)(?:-.*\n)+)}
                         {-$2$1+$3}mg'

Now I'm not convinced that not showing lonely common lines always helps with legibility. Compare for instance:

--- A   2018-10-01 09:37:37.606642955 +0200
+++ B   2018-10-01 09:37:40.405675295 +0200
@@ -1,8 +1,11 @@
 int fib(int n) {
-  if (n <= 1) {
-    return n;
+  int i, t1 = 0, t2 = 1;
+  for (i = 0; i < n; ++i) {
+    int next = t1 + t2;
+    t1 = t2;
+    t2 = next;
   }
-  /* assinging foo */
+  /* assigning foo */
   foo = (bar ? bar : complicated_stuff(*a.asd.qwe|(FLAG1|FLAG2)));
-  return fib(n-1) + fib(n-2);
+  return t1;

with the output of the perl code on it:

--- A   2018-10-01 09:37:37.606642955 +0200
+++ B   2018-10-01 09:37:40.405675295 +0200
@@ -1,8 +1,11 @@
 int fib(int n) {
-  if (n <= 1) {
-    return n;
-  }
-  /* assinging foo */
-  foo = (bar ? bar : complicated_stuff(*a.asd.qwe|(FLAG1|FLAG2)));
-  return fib(n-1) + fib(n-2);
+  int i, t1 = 0, t2 = 1;
+  for (i = 0; i < n; ++i) {
+    int next = t1 + t2;
+    t1 = t2;
+    t2 = next;
+  }
+  /* assigning foo */
+  foo = (bar ? bar : complicated_stuff(*a.asd.qwe|(FLAG1|FLAG2)));
+  return t1;

I find it more useful that diff shows me that that complicated line is unchanged. In the expanded version, you'd be tempted to look for the difference between the two.

You could replace the ((.*\n) with ((.{0,20}\n) to only consider short lonely common lines (or (((?:\h*\H){0,10}\h*\n) to consider only the number of non-blanks).

  • Another heuristic may be to take the sizes of surrounding -/+ blocks into consideration: The more +- lines you have around a line, the more tempting it is to merge the blocks. E.g., it could be worth duplicating two adjacent short lines or one long line. Also, if there is just one +- line before and and one +- after, then merging the blocks is not so compelling. Also, I have a vague feeling that I'm asking the wrong question. In my concrete fib case, applying a filter to merge diff blocks is one obvious improvement, but maybe there is there some radically different way to approach the problem? – Erik Carstensen Oct 2 '18 at 16:10
0

The following script improves my situation, by detecting "almost completely different" sequences in a diff, and considering them to be completely different. Looking for better solutions.

#!/usr/bin/python
import sys
(mbefore, pbefore, mid, mafter) = ([], [], [], [])
def flush():
    for x in (mbefore, pbefore, mid, mafter):
        sys.stdout.write(''.join(x))
        del x[:]

for line in sys.stdin:
    if line[0] == '-':
        if mid:
            mafter.append(line)
        else:
            flush()
            mbefore.append(line)
    elif line[0] == '+':
        if mafter:
            mbefore.append('-' + mid[0][1:])
            pbefore.append('+' + mid[0][1:])
            mbefore += mafter
            pbefore.append(line)
            del mafter[:]
            del mid[:]
        elif not mid and mbefore:
            pbefore.append(line)
        else:
            flush()
            sys.stdout.write(line)
    elif line[0] == ' ':
        if pbefore and not mid:
            mid.append(line)
        else:
            flush()
            sys.stdout.write(line)
    else:
        flush()
        sys.stdout.write(line)
flush()

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.