Make auto-ccs work, delete some dead code, make inlines show up in email,

and resolve a display caching issue.

Summary:

Test Plan:

Reviewers:

CC:

src/applications/differential/editor/comment/DifferentialCommentEditor.php

@@ -238,7 +238,14 @@ class DifferentialCommentEditor {
         throw new Exception('Unsupported action.');
     }
 
-    // Reload relationships to pick up any reviewer changes.
+    if ($this->addCC) {
+      DifferentialRevisionEditor::addCC(
+        $revision,
+        $this->actorPHID,
+        $this->actorPHID);
+    }
+
+    // Reload relationships to pick up any reviewer/CC changes.
     $revision->loadRelationships();
 
     $inline_comments = array();
@@ -256,24 +263,15 @@ class DifferentialCommentEditor {
       ->setContent((string)$this->message)
       ->save();
 
-//    $diff = id(new Diff())->loadActiveWithRevision($revision);
+    $changesets = array();
-//    $changesets = id(new DifferentialChangeset())->loadAllWithDiff($diff);
-
     if ($inline_comments) {
-/*
+      $load_ids = mpull($inline_comments, 'getChangesetID');
-      // We may have feedback on non-current changesets. Rather than orphaning
+      if ($load_ids) {
-      // it, just submit it. This is non-ideal but not horrible.
+        $load_ids = array_unique($load_ids);
-      $inline_changeset_ids = array_pull($inline_comments, 'getChangesetID');
+        $changesets = id(new DifferentialChangeset())->loadAllWhere(
-      $load = array();
+          'id in (%Ld)',
-      foreach ($inline_changeset_ids as $id) {
+          $load_ids);
-        if (empty($changesets[$id])) {
-          $load[] = $id;
-        }
       }
-      if ($load) {
-        $changesets += id(new DifferentialChangeset())->loadAllWithIDs($load);
-      }
-*/
       foreach ($inline_comments as $inline) {
         $inline->setCommentID($comment->getID());
         $inline->save();
@@ -289,8 +287,8 @@ class DifferentialCommentEditor {
       $revision,
       $actor_handle,
       $comment,
-      /* $changesets TODO */ array(),
+      $changesets,
-      /* $inline_comments TODO */ array()))
+      $inline_comments))
       ->setToPHIDs(
         array_merge(
           $revision->getReviewers(),
@@ -299,19 +297,6 @@ class DifferentialCommentEditor {
       ->setChangedByCommit($this->getChangedByCommit())
       ->send();
 
-/*
-
-  tODO
-
-    if ($this->addCC) {
-      require_module_lazy('site/tools/differential/lib/editor/revision');
-      DifferentialRevisionEditor::addCCFBID(
-        $revision,
-        $this->actorPHID,
-        $this->actorPHID);
-    }
-*/
-
 /*
 
   TODO

src/applications/differential/editor/comment/__init__.php

@@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/action
 phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus');
 phutil_require_module('phabricator', 'applications/differential/editor/revision');
 phutil_require_module('phabricator', 'applications/differential/mail/comment');
+phutil_require_module('phabricator', 'applications/differential/storage/changeset');
 phutil_require_module('phabricator', 'applications/differential/storage/comment');
 phutil_require_module('phabricator', 'applications/differential/storage/inlinecomment');
 phutil_require_module('phabricator', 'applications/phid/handle/data');

src/applications/differential/editor/revision/DifferentialRevisionEditor.php

@@ -72,63 +72,6 @@ class DifferentialRevisionEditor {
     $this->setCCPHIDs($fields['ccPHIDs']);
   }
 
-
-/*
-  public static function newRevisionFromRawMessageWithDiff(
-    DifferentialRawMessage $message,
-    Diff $diff,
-    $user) {
-
-    if ($message->getRevisionID()) {
-      throw new Exception(
-        "The provided commit message is already associated with a ".
-        "Differential revision.");
-    }
-
-    if ($message->getReviewedByNames()) {
-      throw new Exception(
-        "The provided commit message contains a 'Reviewed By:' field.");
-    }
-
-    $revision = new DifferentialRevision();
-    $revision->setPHID($revision->generatePHID());
-
-    $revision->setOwnerID($user);
-    $revision->setStatus(DifferentialRevisionStatus::NEEDS_REVIEW);
-    $revision->attachReviewers(array());
-    $revision->attachCCPHIDs(array());
-
-    $editor = new DifferentialRevisionEditor($revision, $user);
-
-    self::copyFields($editor, $revision, $message, $user);
-
-    $editor->addDiff($diff, null);
-    $editor->save();
-
-    return $revision;
-  }
-
-
-  public static function copyFields(
-    DifferentialRevisionEditor $editor,
-    DifferentialRevision $revision,
-    DifferentialRawMessage $message,
-    $user) {
-
-    $revision->setName($message->getTitle());
-    $revision->setSummary($message->getSummary());
-    $revision->setTestPlan($message->getTestPlan());
-    $revision->setSVNBlameRevision($message->getBlameRevision());
-    $revision->setRevert($message->getRevertPlan());
-    $revision->setPlatformImpact($message->getPlatformImpact());
-    $revision->setBugzillaID($message->getBugzillaID());
-
-    $editor->setReviewers($message->getReviewerPHIDs());
-    $editor->setCCPHIDs($message->getCCPHIDs());
-  }
-
-*/
-
   public function getRevision() {
     return $this->revision;
   }

src/applications/differential/mail/comment/DifferentialCommentMail.php

@@ -92,9 +92,15 @@ class DifferentialCommentMail extends DifferentialMail {
           throw new Exception('Changeset missing!');
         }
         $file = $changeset->getFilename();
-        $line = $inline->renderLineRange();
+        $start = $inline->getLineNumber();
+        $len = $inline->getLineLength();
+        if ($len) {
+          $range = $start.'-'.($start + $len);
+        } else {
+          $range = $start;
+        }
         $content = $inline->getContent();
-        $body[] = $this->formatText("{$file}:{$line} {$content}");
+        $body[] = $this->formatText("{$file}:{$range} {$content}");
       }
       $body[] = null;
     }

src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php

@@ -138,19 +138,24 @@ final class DifferentialRevisionCommentView extends AphrontView {
             ),
             $lines);
 
-          $content = $inline->getCache();
+          $inline_content = $inline->getContent();
-          if (!strlen($content)) {
+          if (strlen($inline_content)) {
-            $content = $this->markupEngine->markupText($content);
+            $inline_cache = $inline->getCache();
-            if ($inline->getID()) {
+            if ($inline_cache) {
-              $inline->setCache($content);
+              $inline_content = $inline_cache;
-              $inline->save();
+            } else {
+              $inline_content = $this->markupEngine->markupText($content);
+              if ($inline->getID()) {
+                $inline->setCache($inline_content);
+                $inline->save();
+              }
             }
           }
 
           $inline_render[] =
             '<tr>'.
               '<td class="inline-line-number">'.$lines.'</td>'.
-              '<td>'.$content.'</td>'.
+              '<td>'.$inline_content.'</td>'.
             '</tr>';
         }
       }