Highlight inline diff context in HTML mail

Summary:
Ref T10694. Ref T9790. When generating inline diff context, highlight it and then mangle the highlighted output into `style="..."` so it works in HTML.

Also try to tighten up some spacing/formatting stuff.

Test Plan:
Got some output in this vein:

{F1259937}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9790, T10694

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

src/__phutil_library_map__.php

@@ -370,6 +370,7 @@ phutil_register_library_map(array(
     'DifferentialChangesetFileTreeSideNavBuilder' => 'applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php',
     'DifferentialChangesetHTMLRenderer' => 'applications/differential/render/DifferentialChangesetHTMLRenderer.php',
     'DifferentialChangesetListView' => 'applications/differential/view/DifferentialChangesetListView.php',
+    'DifferentialChangesetOneUpMailRenderer' => 'applications/differential/render/DifferentialChangesetOneUpMailRenderer.php',
     'DifferentialChangesetOneUpRenderer' => 'applications/differential/render/DifferentialChangesetOneUpRenderer.php',
     'DifferentialChangesetOneUpTestRenderer' => 'applications/differential/render/DifferentialChangesetOneUpTestRenderer.php',
     'DifferentialChangesetParser' => 'applications/differential/parser/DifferentialChangesetParser.php',
@@ -4559,6 +4560,7 @@ phutil_register_library_map(array(
     'DifferentialChangesetFileTreeSideNavBuilder' => 'Phobject',
     'DifferentialChangesetHTMLRenderer' => 'DifferentialChangesetRenderer',
     'DifferentialChangesetListView' => 'AphrontView',
+    'DifferentialChangesetOneUpMailRenderer' => 'DifferentialChangesetRenderer',
     'DifferentialChangesetOneUpRenderer' => 'DifferentialChangesetHTMLRenderer',
     'DifferentialChangesetOneUpTestRenderer' => 'DifferentialChangesetTestRenderer',
     'DifferentialChangesetParser' => 'Phobject',

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -1212,9 +1212,7 @@ final class DifferentialTransactionEditor
     }
 
     if ($inlines) {
-      $body->addTextSection(
-        pht('INLINE COMMENTS'),
-        $this->renderInlineCommentsForMail($object, $inlines));
+      $this->appendInlineCommentsForMail($object, $inlines, $body);
     }
 
     $changed_uri = $this->getChangedPriorToCommitURI();
@@ -1374,13 +1372,23 @@ final class DifferentialTransactionEditor
     return $result;
   }
 
-  private function renderInlineCommentsForMail(
+  private function appendInlineCommentsForMail(
     PhabricatorLiskDAO $object,
-    array $inlines) {
-    return id(new DifferentialInlineCommentMailView())
+    array $inlines,
+    PhabricatorMetaMTAMailBody $body) {
+
+    $section = id(new DifferentialInlineCommentMailView())
       ->setViewer($this->getActor())
       ->setInlines($inlines)
       ->buildMailSection();
+
+    $header = pht('INLINE COMMENTS');
+
+    $section_text = "\n".$section->getPlaintext();
+    $section_html = $section->getHTML();
+
+    $body->addPlaintextSection($header, $section_text, false);
+    $body->addHTMLSection($header, $section_html);
   }
 
   private function loadDiff($phid, $need_changesets = false) {

src/applications/differential/mail/DifferentialInlineCommentMailView.php

@@ -71,9 +71,11 @@ final class DifferentialInlineCommentMailView
             $context_html = $this->renderInline($parent, true, true);
           }
         } else {
-          $patch = $this->getPatch($hunk_parser, $comment);
-          $context_text = $this->renderPatch($comment, $patch, false);
-          $context_html = $this->renderPatch($comment, $patch, true);
+          $patch_text = $this->getPatch($hunk_parser, $comment, false);
+          $context_text = $this->renderPatch($comment, $patch_text, false);
+
+          $patch_html = $this->getPatch($hunk_parser, $comment, true);
+          $context_html = $this->renderPatch($comment, $patch_html, true);
         }
 
         $render_text = $this->renderInline($comment, false, false);
@@ -83,7 +85,6 @@ final class DifferentialInlineCommentMailView
         $section->addHTMLFragment($context_html);
 
         $section->addPlaintextFragment($spacer_text);
-        $section->addHTMLFragment($spacer_html);
 
         $section->addPlaintextFragment($render_text);
         $section->addHTMLFragment($render_html);
@@ -180,6 +181,18 @@ final class DifferentialInlineCommentMailView
     $content = $this->renderRemarkupContent($content, $is_html);
 
     if ($is_quote) {
+      if ($is_html) {
+        $style = array(
+          'padding: 4px 0;',
+        );
+
+        $content = phutil_tag(
+          'div',
+          array(
+            'style' => implode(' ', $style),
+          ),
+          $content);
+      }
       $header = $this->renderHeader($comment, $is_html, true);
     } else {
       $header = null;
@@ -254,6 +267,7 @@ final class DifferentialInlineCommentMailView
       'padding: 4px 8px;',
       'background: #F8F9FC;',
       'border-left: 3px solid #a7b5bf;',
+      'margin: 4px 0 0;',
     );
 
     $styles = implode(' ', $styles);
@@ -268,18 +282,43 @@ final class DifferentialInlineCommentMailView
 
   private function getPatch(
     DifferentialHunkParser $parser,
-    DifferentialTransactionComment $comment) {
+    DifferentialTransactionComment $comment,
+    $is_html) {
 
     $changeset = $this->getChangeset($comment->getChangesetID());
-    $hunks = $changeset->getHunks();
-
     $is_new = $comment->getIsNewFile();
     $start = $comment->getLineNumber();
     $length = $comment->getLineLength();
 
-    $diff = $parser->makeContextDiff($hunks, $is_new, $start, $length, 1);
+    if (!$is_html) {
+      $hunks = $changeset->getHunks();
+      $patch = $parser->makeContextDiff($hunks, $is_new, $start, $length, 1);
+      $patch = phutil_split_lines($patch);
 
-    return $diff;
+      // Remove the "@@ -x,y +u,v @@" line.
+      array_shift($patch);
+
+      return implode('', $patch);
+    }
+
+    $viewer = $this->getViewer();
+    $engine = new PhabricatorMarkupEngine();
+
+    if ($is_new) {
+      $offset_mode = 'new';
+    } else {
+      $offset_mode = 'old';
+    }
+
+    $parser = id(new DifferentialChangesetParser())
+      ->setUser($viewer)
+      ->setChangeset($changeset)
+      ->setOffsetMode($offset_mode)
+      ->setMarkupEngine($engine);
+
+    $parser->setRenderer(new DifferentialChangesetOneUpMailRenderer());
+
+    return $parser->render($start - 1, $length + 3, array());
   }
 
   private function renderPatch(
@@ -287,17 +326,10 @@ final class DifferentialInlineCommentMailView
     $patch,
     $is_html) {
 
-    $patch = phutil_split_lines($patch);
-
-    // Remove the "@@ -x,y +u,v @@" line.
-    array_shift($patch);
-
-    $patch = implode('', $patch);
-
     if ($is_html) {
       $style = array(
         'font: 11px/15px "Menlo", "Consolas", "Monaco", monospace;',
-        'padding: 0',
+        'padding: 4px 0;',
         'margin: 0;',
       );
 
@@ -346,7 +378,12 @@ final class DifferentialInlineCommentMailView
 
     $header = "{$path}:{$range}";
     if ($is_html) {
-      $header = phutil_tag('strong', array(), $header);
+      $header = phutil_tag(
+        'span',
+        array(
+          'style' => 'color: #000000',
+        ),
+        $header);
     }
 
     if ($with_author) {
@@ -359,7 +396,12 @@ final class DifferentialInlineCommentMailView
       $byline = '@'.$author->getName();
 
       if ($is_html) {
-        $byline = phutil_tag('strong', array(), $byline);
+        $byline = phutil_tag(
+          'span',
+          array(
+            'style' => 'color: #000000',
+          ),
+          $byline);
       }
 
       $header = pht('%s wrote in %s', $byline, $header);
@@ -371,7 +413,7 @@ final class DifferentialInlineCommentMailView
       $header = phutil_tag(
         'div',
         array(
-          'style' => 'font-style: italic',
+          'style' => 'font-style: italic; color: #74777d',
         ),
         $header);
     }

src/applications/differential/parser/DifferentialChangesetParser.php

@@ -50,6 +50,7 @@ final class DifferentialChangesetParser extends Phobject {
   private $showEditAndReplyLinks = true;
   private $canMarkDone;
   private $objectOwnerPHID;
+  private $offsetMode;
 
   private $rangeStart;
   private $rangeEnd;
@@ -138,6 +139,15 @@ final class DifferentialChangesetParser extends Phobject {
     return $this->objectOwnerPHID;
   }
 
+  public function setOffsetMode($offset_mode) {
+    $this->offsetMode = $offset_mode;
+    return $this;
+  }
+
+  public function getOffsetMode() {
+    return $this->offsetMode;
+  }
+
   public static function getDefaultRendererForViewer(PhabricatorUser $viewer) {
     $prefs = $viewer->loadPreferences();
     $pref_unified = PhabricatorUserPreferences::PREFERENCE_DIFF_UNIFIED;
@@ -829,6 +839,22 @@ final class DifferentialChangesetParser extends Phobject {
     }
 
     $this->tryCacheStuff();
+
+    // If we're rendering in an offset mode, treat the range numbers as line
+    // numbers instead of rendering offsets.
+    $offset_mode = $this->getOffsetMode();
+    if ($offset_mode) {
+      if ($offset_mode == 'new') {
+        $offset_map = $this->new;
+      } else {
+        $offset_map = $this->old;
+      }
+
+      $range_end = $this->getOffset($offset_map, $range_start + $range_len);
+      $range_start = $this->getOffset($offset_map, $range_start);
+      $range_len = $range_end - $range_start;
+    }
+
     $render_pch = $this->shouldRenderPropertyChangeHeader($this->changeset);
 
     $rows = max(
@@ -1632,5 +1658,21 @@ final class DifferentialChangesetParser extends Phobject {
     return $results;
   }
 
+  private function getOffset(array $map, $line) {
+    if (!$map) {
+      return null;
+    }
+
+    $line = (int)$line;
+    foreach ($map as $key => $spec) {
+      if ($spec && isset($spec['line'])) {
+        if ((int)$spec['line'] >= $line) {
+          return $key;
+        }
+      }
+    }
+
+    return $key;
+  }
 
 }

src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php

@@ -0,0 +1,98 @@
+<?php
+
+final class DifferentialChangesetOneUpMailRenderer
+  extends DifferentialChangesetRenderer {
+
+  public function isOneUpRenderer() {
+    return true;
+  }
+
+  protected function getRendererTableClass() {
+    return 'diff-1up-mail';
+  }
+
+  public function getRendererKey() {
+    return '1up-mail';
+  }
+
+  protected function renderChangeTypeHeader($force) {
+    return null;
+  }
+
+  protected function renderUndershieldHeader() {
+    return null;
+  }
+
+  public function renderShield($message, $force = 'default') {
+    return null;
+  }
+
+  protected function renderPropertyChangeHeader() {
+    return null;
+  }
+
+  public function renderFileChange(
+    $old_file = null,
+    $new_file = null,
+    $id = 0,
+    $vs = 0) {
+    return null;
+  }
+
+  public function renderTextChange(
+    $range_start,
+    $range_len,
+    $rows) {
+
+    $primitives = $this->buildPrimitives($range_start, $range_len);
+    return $this->renderPrimitives($primitives, $rows);
+  }
+
+  protected function renderPrimitives(array $primitives, $rows) {
+    $out = array();
+    foreach ($primitives as $k => $p) {
+      $type = $p['type'];
+      switch ($type) {
+        case 'old':
+        case 'new':
+        case 'old-file':
+        case 'new-file':
+          $is_old = ($type == 'old' || $type == 'old-file');
+
+          if ($is_old) {
+            if ($p['htype']) {
+              $style = 'background: #ffd0d0;';
+            } else {
+              $style = null;
+            }
+          } else {
+            if ($p['htype']) {
+              $style = 'background: #d0ffd0;';
+            } else {
+              $style =  null;
+            }
+          }
+
+          $out[] = phutil_tag(
+            'div',
+            array(
+              'style' => $style,
+            ),
+            $p['render']);
+          break;
+        default:
+          break;
+      }
+    }
+
+    $style_map = id(new PhabricatorDefaultSyntaxStyle())
+      ->getRemarkupStyleMap();
+
+    $styled_body = id(new PhutilPygmentizeParser())
+      ->setMap($style_map)
+      ->parse((string)hsprintf('%s', $out));
+
+    return phutil_safe_html($styled_body);
+  }
+
+}

src/applications/metamta/view/PhabricatorMetaMTAMailBody.php

@@ -111,8 +111,11 @@ final class PhabricatorMetaMTAMailBody extends Phobject {
     return $this;
   }
 
-  public function addPlaintextSection($header, $text) {
-    $this->sections[] = $header."\n".$this->indent($text);
+  public function addPlaintextSection($header, $text, $indent = true) {
+    if ($indent) {
+      $text = $this->indent($text);
+    }
+    $this->sections[] = $header."\n".$text;
     return $this;
   }