diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 3652d18a9a..ad62b2a1ef 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -1053,14 +1053,30 @@ final class DifferentialChangesetParser extends Phobject { $this->comments = id(new PHUIDiffInlineThreader()) ->reorderAndThreadCommments($this->comments); + $old_max_display = 1; + foreach ($this->old as $old) { + if (isset($old['line'])) { + $old_max_display = $old['line']; + } + } + + $new_max_display = 1; + foreach ($this->new as $new) { + if (isset($new['line'])) { + $new_max_display = $new['line']; + } + } + foreach ($this->comments as $comment) { - $final = $comment->getLineNumber() + - $comment->getLineLength(); - $final = max(1, $final); + $display_line = $comment->getLineNumber() + $comment->getLineLength(); + $display_line = max(1, $display_line); + if ($this->isCommentOnRightSideWhenDisplayed($comment)) { - $new_comments[$final][] = $comment; + $display_line = min($new_max_display, $display_line); + $new_comments[$display_line][] = $comment; } else { - $old_comments[$final][] = $comment; + $display_line = min($old_max_display, $display_line); + $old_comments[$display_line][] = $comment; } } } diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php index 186924e359..bcdbf4dd46 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php @@ -83,7 +83,9 @@ final class DifferentialChangesetOneUpRenderer $cells = array(); if ($is_old) { if ($p['htype']) { - if (empty($p['oline'])) { + if ($p['htype'] === '\\') { + $class = 'comment'; + } else if (empty($p['oline'])) { $class = 'left old old-full'; } else { $class = 'left old'; @@ -129,7 +131,9 @@ final class DifferentialChangesetOneUpRenderer $cells[] = $no_coverage; } else { if ($p['htype']) { - if (empty($p['oline'])) { + if ($p['htype'] === '\\') { + $class = 'comment'; + } else if (empty($p['oline'])) { $class = 'right new new-full'; } else { $class = 'right new'; diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php index d493c0e886..dcae3b979b 100644 --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -505,6 +505,8 @@ abstract class DifferentialChangesetRenderer extends Phobject { $ospec['htype'] = $old[$ii]['type']; if (isset($old_render[$ii])) { $ospec['render'] = $old_render[$ii]; + } else if ($ospec['htype'] === '\\') { + $ospec['render'] = $old[$ii]['text']; } } @@ -514,6 +516,8 @@ abstract class DifferentialChangesetRenderer extends Phobject { $nspec['htype'] = $new[$ii]['type']; if (isset($new_render[$ii])) { $nspec['render'] = $new_render[$ii]; + } else if ($nspec['htype'] === '\\') { + $nspec['render'] = $new[$ii]['text']; } } diff --git a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php index 753cdf3921..20474b047b 100644 --- a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php +++ b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php @@ -85,15 +85,8 @@ final class PhabricatorJupyterDocumentEngine if ($utype === $vtype) { switch ($utype) { case 'markdown': - $usource = idx($ucell, 'source'); - if (is_array($usource)) { - $usource = implode('', $usource); - } - - $vsource = idx($vcell, 'source'); - if (is_array($vsource)) { - $vsource = implode('', $vsource); - } + $usource = $this->readString($ucell, 'source'); + $vsource = $this->readString($vcell, 'source'); $diff = id(new PhutilProseDifferenceEngine()) ->getDiff($usource, $vsource); @@ -117,8 +110,6 @@ final class PhabricatorJupyterDocumentEngine $vsource = idx($vcell, 'raw'); $udisplay = idx($ucell, 'display'); $vdisplay = idx($vcell, 'display'); - $ulabel = idx($ucell, 'label'); - $vlabel = idx($vcell, 'label'); $intraline_segments = ArcanistDiffUtils::generateIntralineDiff( $usource, @@ -142,15 +133,15 @@ final class PhabricatorJupyterDocumentEngine $vdisplay, $v_segments); - $u_content = $this->newCodeLineCell($ucell, $usource); - $v_content = $this->newCodeLineCell($vcell, $vsource); + list($u_label, $u_content) = $this->newCodeLineCell($ucell, $usource); + list($v_label, $v_content) = $this->newCodeLineCell($vcell, $vsource); $classes = array( 'jupyter-cell-flush', ); - $u_content = $this->newJupyterCell($ulabel, $u_content, $classes); - $v_content = $this->newJupyterCell($vlabel, $v_content, $classes); + $u_content = $this->newJupyterCell($u_label, $u_content, $classes); + $v_content = $this->newJupyterCell($v_label, $v_content, $classes); $u_content = $this->newCellContainer($u_content); $v_content = $this->newCellContainer($v_content); @@ -259,10 +250,7 @@ final class PhabricatorJupyterDocumentEngine $hash_input = $cell['raw']; break; case 'markdown': - $hash_input = $cell['source']; - if (is_array($hash_input)) { - $hash_input = implode('', $cell['source']); - } + $hash_input = $this->readString($cell, 'source'); break; default: $hash_input = serialize($cell); @@ -334,7 +322,6 @@ final class PhabricatorJupyterDocumentEngine 'be rendered as a Jupyter notebook.')); } - $nbformat = idx($data, 'nbformat'); if (!strlen($nbformat)) { throw new Exception( @@ -376,10 +363,7 @@ final class PhabricatorJupyterDocumentEngine foreach ($cells as $cell) { $cell_type = idx($cell, 'cell_type'); if ($cell_type === 'markdown') { - $source = $cell['source']; - if (is_array($source)) { - $source = implode('', $source); - } + $source = $this->readString($cell, 'source'); // Attempt to split contiguous blocks of markdown into smaller // pieces. @@ -404,11 +388,7 @@ final class PhabricatorJupyterDocumentEngine $label = $this->newCellLabel($cell); - $lines = idx($cell, 'source'); - if (!is_array($lines)) { - $lines = array(); - } - + $lines = $this->readStringList($cell, 'source'); $content = $this->highlightLines($lines); $count = count($lines); @@ -526,10 +506,7 @@ final class PhabricatorJupyterDocumentEngine } private function newMarkdownCell(array $cell) { - $content = idx($cell, 'source'); - if (!is_array($content)) { - $content = array(); - } + $content = $this->readStringList($cell, 'source'); // TODO: This should ideally highlight as Markdown, but the "md" // highlighter in Pygments is painfully slow and not terribly useful. @@ -549,11 +526,7 @@ final class PhabricatorJupyterDocumentEngine private function newCodeCell(array $cell) { $label = $this->newCellLabel($cell); - $content = idx($cell, 'source'); - if (!is_array($content)) { - $content = array(); - } - + $content = $this->readStringList($cell, 'source'); $content = $this->highlightLines($content); $outputs = array(); @@ -660,11 +633,7 @@ final class PhabricatorJupyterDocumentEngine continue; } - $raw_data = $data[$image_format]; - if (!is_array($raw_data)) { - $raw_data = array($raw_data); - } - $raw_data = implode('', $raw_data); + $raw_data = $this->readString($data, $image_format); $content = phutil_tag( 'img', @@ -695,11 +664,7 @@ final class PhabricatorJupyterDocumentEngine break; case 'stream': default: - $content = idx($output, 'text'); - if (!is_array($content)) { - $content = array(); - } - $content = implode('', $content); + $content = $this->readString($output, 'text'); break; } @@ -761,4 +726,23 @@ final class PhabricatorJupyterDocumentEngine return true; } + private function readString(array $src, $key) { + $list = $this->readStringList($src, $key); + return implode('', $list); + } + + private function readStringList(array $src, $key) { + $list = idx($src, $key); + + if (is_array($list)) { + $list = $list; + } else if (is_string($list)) { + $list = array($list); + } else { + $list = array(); + } + + return $list; + } + } diff --git a/src/infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php b/src/infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php index 095a44c221..f1888cc363 100644 --- a/src/infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php +++ b/src/infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php @@ -358,7 +358,7 @@ final class PhabricatorInlineCommentAdjustmentEngine list($tail_deleted, $tail_offset, $tail_line) = $tail_info; if ($head_offset !== false) { - $inline->setLineNumber($head_line + 1 + $head_offset); + $inline->setLineNumber($head_line + $head_offset); } else { $inline->setLineNumber($head_line); $inline->setLineLength($tail_line - $head_line);