Unhighlight lines modified in rebase

Summary:
Diff of diffs display changes between new versions of file.
This is bad after rebase because there can be many unrelated changes so it is hard to spot the real change.

This diff unhighlights the lines that were added or removed in rebase.
The changes are still visible (they can be sometimes relevant) but very subtle.

Test Plan:
# Add, change and delete line. Display diff.
# Add and change some lines in parent. Rebase. Display diff. Display diff of diff.
# Change and add some lines. Display diff. Display diff to first diff. Display diff to second diff.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: jungejason, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2761

src/__celerity_resource_map__.php

@@ -566,7 +566,7 @@ celerity_register_resource_map(array(
   ),
   'differential-changeset-view-css' =>
   array(
-    'uri' => '/res/8e2ace51/rsrc/css/application/differential/changeset-view.css',
+    'uri' => '/res/c9706669/rsrc/css/application/differential/changeset-view.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -2628,7 +2628,7 @@ celerity_register_resource_map(array(
       'uri' => '/res/pkg/f363b322/core.pkg.js',
       'type' => 'js',
     ),
-    '32f461a4' =>
+    '96bc37d6' =>
     array(
       'name' => 'differential.pkg.css',
       'symbols' =>
@@ -2647,7 +2647,7 @@ celerity_register_resource_map(array(
         11 => 'differential-local-commits-view-css',
         12 => 'inline-comment-summary-css',
       ),
-      'uri' => '/res/pkg/32f461a4/differential.pkg.css',
+      'uri' => '/res/pkg/96bc37d6/differential.pkg.css',
       'type' => 'css',
     ),
     'f4bbbd84' =>
@@ -2770,7 +2770,7 @@ celerity_register_resource_map(array(
     'aphront-dialog-view-css' => 'a05e3ec6',
     'aphront-error-view-css' => 'a05e3ec6',
     'aphront-form-view-css' => 'a05e3ec6',
-    'aphront-headsup-action-list-view-css' => '32f461a4',
+    'aphront-headsup-action-list-view-css' => '96bc37d6',
     'aphront-headsup-view-css' => 'a05e3ec6',
     'aphront-list-filter-view-css' => 'a05e3ec6',
     'aphront-pager-view-css' => 'a05e3ec6',
@@ -2780,19 +2780,19 @@ celerity_register_resource_map(array(
     'aphront-tokenizer-control-css' => 'a05e3ec6',
     'aphront-tooltip-css' => 'a05e3ec6',
     'aphront-typeahead-control-css' => 'a05e3ec6',
-    'differential-changeset-view-css' => '32f461a4',
-    'differential-core-view-css' => '32f461a4',
+    'differential-changeset-view-css' => '96bc37d6',
+    'differential-core-view-css' => '96bc37d6',
     'differential-inline-comment-editor' => 'f4bbbd84',
-    'differential-local-commits-view-css' => '32f461a4',
-    'differential-results-table-css' => '32f461a4',
-    'differential-revision-add-comment-css' => '32f461a4',
-    'differential-revision-comment-css' => '32f461a4',
-    'differential-revision-comment-list-css' => '32f461a4',
-    'differential-revision-history-css' => '32f461a4',
-    'differential-table-of-contents-css' => '32f461a4',
+    'differential-local-commits-view-css' => '96bc37d6',
+    'differential-results-table-css' => '96bc37d6',
+    'differential-revision-add-comment-css' => '96bc37d6',
+    'differential-revision-comment-css' => '96bc37d6',
+    'differential-revision-comment-list-css' => '96bc37d6',
+    'differential-revision-history-css' => '96bc37d6',
+    'differential-table-of-contents-css' => '96bc37d6',
     'diffusion-commit-view-css' => 'c8ce2d88',
     'diffusion-icons-css' => 'c8ce2d88',
-    'inline-comment-summary-css' => '32f461a4',
+    'inline-comment-summary-css' => '96bc37d6',
     'javelin-behavior' => '6fb20113',
     'javelin-behavior-aphront-basic-tokenizer' => '97f65640',
     'javelin-behavior-aphront-drag-and-drop' => 'f4bbbd84',
@@ -2846,7 +2846,7 @@ celerity_register_resource_map(array(
     'maniphest-task-summary-css' => '7839ae2d',
     'maniphest-transaction-detail-css' => '7839ae2d',
     'phabricator-app-buttons-css' => 'a05e3ec6',
-    'phabricator-content-source-view-css' => '32f461a4',
+    'phabricator-content-source-view-css' => '96bc37d6',
     'phabricator-core-buttons-css' => 'a05e3ec6',
     'phabricator-core-css' => 'a05e3ec6',
     'phabricator-directory-css' => 'a05e3ec6',
@@ -2857,7 +2857,7 @@ celerity_register_resource_map(array(
     'phabricator-keyboard-shortcut' => 'f363b322',
     'phabricator-keyboard-shortcut-manager' => 'f363b322',
     'phabricator-menu-item' => 'f363b322',
-    'phabricator-object-selector-css' => '32f461a4',
+    'phabricator-object-selector-css' => '96bc37d6',
     'phabricator-paste-file-upload' => 'f363b322',
     'phabricator-prefab' => 'f363b322',
     'phabricator-project-tag-css' => '7839ae2d',

src/__phutil_library_map__.php

@@ -235,6 +235,7 @@ phutil_register_library_map(array(
     'DifferentialChangesetDetailView' => 'applications/differential/view/DifferentialChangesetDetailView.php',
     'DifferentialChangesetListView' => 'applications/differential/view/DifferentialChangesetListView.php',
     'DifferentialChangesetParser' => 'applications/differential/parser/DifferentialChangesetParser.php',
+    'DifferentialChangesetParserTestCase' => 'applications/differential/parser/__tests__/DifferentialChangesetParserTestCase.php',
     'DifferentialChangesetViewController' => 'applications/differential/controller/DifferentialChangesetViewController.php',
     'DifferentialComment' => 'applications/differential/storage/DifferentialComment.php',
     'DifferentialCommentEditor' => 'applications/differential/editor/DifferentialCommentEditor.php',
@@ -1304,6 +1305,7 @@ phutil_register_library_map(array(
     'DifferentialChangeset' => 'DifferentialDAO',
     'DifferentialChangesetDetailView' => 'AphrontView',
     'DifferentialChangesetListView' => 'AphrontView',
+    'DifferentialChangesetParserTestCase' => 'ArcanistPhutilTestCase',
     'DifferentialChangesetViewController' => 'DifferentialController',
     'DifferentialComment' => 'DifferentialDAO',
     'DifferentialCommentMail' => 'DifferentialMail',

src/applications/differential/controller/DifferentialChangesetViewController.php

@@ -122,7 +122,7 @@ final class DifferentialChangesetViewController extends DifferentialController {
         $left_data,
         $right_data);
 
-      $choice = nonempty($left, $right);
+      $choice = clone nonempty($left, $right);
       $choice->attachHunks($synthetic->getHunks());
 
       $changeset = $choice;
@@ -166,6 +166,10 @@ final class DifferentialChangesetViewController extends DifferentialController {
     $parser->setLeftSideCommentMapping($left_source, $left_new);
     $parser->setWhitespaceMode($request->getStr('whitespace'));
 
+    if ($left && $right) {
+      $parser->setOriginals($left, $right);
+    }
+
     // Load both left-side and right-side inline comments.
     $inlines = $this->loadInlineComments(
       array($left_source, $right_source),

src/applications/differential/parser/DifferentialChangesetParser.php

@@ -48,6 +48,9 @@ final class DifferentialChangesetParser {
   private $rightSideChangesetID;
   private $rightSideAttachesToNewFile;
 
+  private $originalLeft;
+  private $originalRight;
+
   private $renderingReference;
   private $isSubparser;
 
@@ -108,6 +111,73 @@ final class DifferentialChangesetParser {
     return $this;
   }
 
+  public function setOriginals(
+    DifferentialChangeset $left,
+    DifferentialChangeset $right) {
+
+    $this->originalLeft = $left;
+    $this->originalRight = $right;
+  }
+
+  public function diffOriginals() {
+    $engine = new PhabricatorDifferenceEngine();
+    $changeset = $engine->generateChangesetFromFileContent(
+      implode('', mpull($this->originalLeft->getHunks(), 'getChanges')),
+      implode('', mpull($this->originalRight->getHunks(), 'getChanges')));
+
+    // Put changes side by side.
+    $olds = array();
+    $news = array();
+    foreach ($changeset->getHunks() as $hunk) {
+      $n_old = $hunk->getOldOffset();
+      $n_new = $hunk->getNewOffset();
+      $changes = rtrim($hunk->getChanges(), "\n");
+      foreach (explode("\n", $changes) as $line) {
+        $type = $line[1]; // Change type in the original diff.
+        if ($line[0] == ' ') {
+          // Use the same key for lines that are next to each other.
+          $key = max(last_key($olds), last_key($news)) + 1;
+          $olds[$key] = null;
+          $news[$key] = null;
+          $n_old++;
+          $n_new++;
+        } else if ($line[0] == '-') {
+          $olds[] = array($n_old, $type);
+          $n_old++;
+        } else if ($line[0] == '+') {
+          $news[] = array($n_new, $type);
+          $n_new++;
+        }
+      }
+    }
+
+    $offsets_old = $this->originalLeft->computeOffsets();
+    $offsets_new = $this->originalRight->computeOffsets();
+
+    // Highlight lines that were add on each side or removed on the other side.
+    $highlight_old = array();
+    $highlight_new = array();
+    $last = max(last_key($olds), last_key($news));
+    for ($i = 0; $i <= $last; $i++) {
+      if (isset($olds[$i])) {
+        list($n, $type) = $olds[$i];
+        if ($type == '+' ||
+            ($type == ' ' && isset($news[$i]) && $news[$i][1] == '-')) {
+          $highlight_old[] = $offsets_old[$n];
+        }
+      }
+      if (isset($news[$i])) {
+        list($n, $type) = $news[$i];
+        if ($type == '+' ||
+            ($type == ' ' && isset($olds[$i]) && $olds[$i][1] == '-')) {
+          $highlight_new[] = $offsets_new[$n];
+        }
+      }
+    }
+
+    return array($highlight_old, $highlight_new);
+  }
+
   /**
    * Set a key for identifying this changeset in the render cache. If set, the
    * parser will attempt to use the changeset render cache, which can improve
@@ -270,8 +340,6 @@ final class DifferentialChangesetParser {
     $old = array();
     $new = array();
 
-    $n = 0;
-
     $this->old = array_reverse($this->old);
     $this->new = array_reverse($this->new);
 
@@ -1290,6 +1358,12 @@ final class DifferentialChangesetParser {
 
     $copy_lines = idx($this->changeset->getMetadata(), 'copy:lines', array());
 
+    if ($this->originalLeft && $this->originalRight) {
+      list($highlight_old, $highlight_new) = $this->diffOriginals();
+      $highlight_old = array_flip($highlight_old);
+      $highlight_new = array_flip($highlight_new);
+    }
+
     for ($ii = $range_start; $ii < $range_start + $range_len; $ii++) {
       if (empty($mask[$ii])) {
         // If we aren't going to show this line, we've just entered a gap.
@@ -1386,6 +1460,8 @@ final class DifferentialChangesetParser {
           if ($this->old[$ii]['type'] == '\\') {
             $o_text = $this->old[$ii]['text'];
             $o_attr = ' class="comment"';
+          } else if ($this->originalLeft && !isset($highlight_old[$o_num])) {
+            $o_attr = ' class="old-rebase"';
           } else if (empty($this->new[$ii])) {
             $o_attr = ' class="old old-full"';
           } else {
@@ -1421,6 +1497,8 @@ final class DifferentialChangesetParser {
           if ($this->new[$ii]['type'] == '\\') {
             $n_text = $this->new[$ii]['text'];
             $n_class = 'comment';
+          } else if ($this->originalRight && !isset($highlight_new[$n_num])) {
+            $n_class = 'new-rebase';
           } else if (empty($this->old[$ii])) {
             $n_class = 'new new-full';
           } else {

src/applications/differential/parser/__tests__/DifferentialChangesetParserTestCase.php

@@ -0,0 +1,53 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DifferentialChangesetParserTestCase extends ArcanistPhutilTestCase {
+
+  public function testDiffChangesets() {
+    $hunk = new DifferentialHunk();
+    $hunk->setChanges("+a\n b\n-c");
+    $hunk->setNewOffset(1);
+    $hunk->setNewLen(2);
+    $left = new DifferentialChangeset();
+    $left->attachHunks(array($hunk));
+
+    $tests = array(
+      "+a\n b\n-c" => array(array(), array()),
+      "+a\n x\n-c" => array(array(), array()),
+      "+aa\n b\n-c" => array(array(1), array(11)),
+      " b\n-c" => array(array(1), array()),
+      " x\n-c" => array(array(1), array()),
+      "+a\n b\n c" => array(array(), array(13)),
+      "+a\n x\n c" => array(array(), array(13)),
+    );
+
+    foreach ($tests as $changes => $expected) {
+      $hunk = new DifferentialHunk();
+      $hunk->setChanges($changes);
+      $hunk->setNewOffset(11);
+      $hunk->setNewLen(3);
+      $right = new DifferentialChangeset();
+      $right->attachHunks(array($hunk));
+
+      $parser = new DifferentialChangesetParser();
+      $parser->setOriginals($left, $right);
+      $this->assertEqual($expected, $parser->diffOriginals(), $changes);
+    }
+  }
+
+}

src/applications/differential/storage/DifferentialChangeset.php

@@ -126,19 +126,25 @@ final class DifferentialChangeset extends DifferentialDAO {
   }
 
   public function makeNewFile() {
-    $file = array();
-    foreach ($this->getHunks() as $hunk) {
-      $file[] = $hunk->makeNewFile();
-    }
-    return implode("\n", $file);
+    $file = mpull($this->getHunks(), 'makeNewFile');
+    return implode('', $file);
   }
 
   public function makeOldFile() {
-    $file = array();
+    $file = mpull($this->getHunks(), 'makeOldFile');
+    return implode('', $file);
+  }
+
+  public function computeOffsets() {
+    $offsets = array();
+    $n = 1;
     foreach ($this->getHunks() as $hunk) {
-      $file[] = $hunk->makeOldFile();
+      for ($i = 0; $i < $hunk->getNewLen(); $i++) {
+        $offsets[$n] = $hunk->getNewOffset() + $i;
+        $n++;
+      }
     }
-    return implode("\n", $file);
+    return $offsets;
   }
 
   public function makeChangesWithContext($num_lines = 3) {

webroot/rsrc/css/application/differential/changeset-view.css

@@ -75,6 +75,14 @@
   background:   #d0ffd0;
 }
 
+.differential-diff td.old-rebase {
+  background: #ffeeee;
+}
+
+.differential-diff td.new-rebase {
+  background: #eeffee;
+}
+
 .differential-diff td.old-full,
 .differential-diff td.old span.bright {
   background: #ffaaaa;