diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 755fd5a906..bebb1d3802 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '4451f9fd', + 'core.pkg.css' => 'f8c53b6f', 'core.pkg.js' => '6e5c894f', 'differential.pkg.css' => '607c84be', 'differential.pkg.js' => 'a0212a0b', @@ -169,7 +169,7 @@ return array( 'rsrc/css/phui/phui-pager.css' => 'd022c7ad', 'rsrc/css/phui/phui-pinboard-view.css' => '1f08f5d8', 'rsrc/css/phui/phui-policy-section-view.css' => '139fdc64', - 'rsrc/css/phui/phui-property-list-view.css' => 'ad841f1c', + 'rsrc/css/phui/phui-property-list-view.css' => '807b1632', 'rsrc/css/phui/phui-remarkup-preview.css' => '91767007', 'rsrc/css/phui/phui-segment-bar-view.css' => '5166b370', 'rsrc/css/phui/phui-spacing.css' => 'b05cadc3', @@ -868,7 +868,7 @@ return array( 'phui-pager-css' => 'd022c7ad', 'phui-pinboard-view-css' => '1f08f5d8', 'phui-policy-section-view-css' => '139fdc64', - 'phui-property-list-view-css' => 'ad841f1c', + 'phui-property-list-view-css' => '807b1632', 'phui-remarkup-preview-css' => '91767007', 'phui-segment-bar-view-css' => '5166b370', 'phui-spacing-css' => 'b05cadc3', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5ac174701c..c35ac91e1d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3140,6 +3140,7 @@ phutil_register_library_map(array( 'PhabricatorDivinerApplication' => 'applications/diviner/application/PhabricatorDivinerApplication.php', 'PhabricatorDocumentEngine' => 'applications/files/document/PhabricatorDocumentEngine.php', 'PhabricatorDocumentEngineBlock' => 'applications/files/diff/PhabricatorDocumentEngineBlock.php', + 'PhabricatorDocumentEngineBlockDiff' => 'applications/files/diff/PhabricatorDocumentEngineBlockDiff.php', 'PhabricatorDocumentEngineBlocks' => 'applications/files/diff/PhabricatorDocumentEngineBlocks.php', 'PhabricatorDocumentRef' => 'applications/files/document/PhabricatorDocumentRef.php', 'PhabricatorDocumentRenderingEngine' => 'applications/files/document/render/PhabricatorDocumentRenderingEngine.php', @@ -9481,6 +9482,7 @@ phutil_register_library_map(array( 'PhabricatorDivinerApplication' => 'PhabricatorApplication', 'PhabricatorDocumentEngine' => 'Phobject', 'PhabricatorDocumentEngineBlock' => 'Phobject', + 'PhabricatorDocumentEngineBlockDiff' => 'Phobject', 'PhabricatorDocumentEngineBlocks' => 'Phobject', 'PhabricatorDocumentRef' => 'Phobject', 'PhabricatorDocumentRenderingEngine' => 'Phobject', diff --git a/src/aphront/response/Aphront403Response.php b/src/aphront/response/Aphront403Response.php index 3f7f6d73ca..3d3d5c9457 100644 --- a/src/aphront/response/Aphront403Response.php +++ b/src/aphront/response/Aphront403Response.php @@ -28,7 +28,7 @@ final class Aphront403Response extends AphrontHTMLResponse { $dialog = id(new AphrontDialogView()) ->setUser($user) ->setTitle(pht('403 Forbidden')) - ->addCancelButton('/', pht('Peace Out')) + ->addCancelButton('/', pht('Yikes!')) ->appendParagraph($forbidden_text); $view = id(new PhabricatorStandardPageView()) diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 408988ed1f..a485a787cb 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -247,7 +247,7 @@ final class DifferentialChangesetParser extends Phobject { return $this->depthOnlyLines; } - public function setVisibileLinesMask(array $mask) { + public function setVisibleLinesMask(array $mask) { $this->visible = $mask; return $this; } @@ -699,13 +699,13 @@ final class DifferentialChangesetParser extends Phobject { $lines_context = $this->getLinesOfContext(); $hunk_parser->generateIntraLineDiffs(); - $hunk_parser->generateVisibileLinesMask($lines_context); + $hunk_parser->generateVisibleLinesMask($lines_context); $this->setOldLines($hunk_parser->getOldLines()); $this->setNewLines($hunk_parser->getNewLines()); $this->setIntraLineDiffs($hunk_parser->getIntraLineDiffs()); $this->setDepthOnlyLines($hunk_parser->getDepthOnlyLines()); - $this->setVisibileLinesMask($hunk_parser->getVisibleLinesMask()); + $this->setVisibleLinesMask($hunk_parser->getVisibleLinesMask()); $this->hunkStartLines = $hunk_parser->getHunkStartLines( $changeset->getHunks()); @@ -868,8 +868,19 @@ final class DifferentialChangesetParser extends Phobject { ->setHighlightingDisabled($this->highlightingDisabled) ->setDepthOnlyLines($this->getDepthOnlyLines()); + list($engine, $old_ref, $new_ref) = $this->newDocumentEngine(); + if ($engine) { + $engine_blocks = $engine->newEngineBlocks( + $old_ref, + $new_ref); + } else { + $engine_blocks = null; + } + + $has_document_engine = ($engine_blocks !== null); + $shield = null; - if ($this->isTopLevel && !$this->comments) { + if ($this->isTopLevel && !$this->comments && !$has_document_engine) { if ($this->isGenerated()) { $shield = $renderer->renderShield( pht( @@ -1024,7 +1035,6 @@ final class DifferentialChangesetParser extends Phobject { ->setOldComments($old_comments) ->setNewComments($new_comments); - $engine_blocks = $this->newDocumentEngineChangesetView(); if ($engine_blocks !== null) { $reference = $this->getRenderingReference(); $parts = explode('/', $reference); @@ -1041,6 +1051,10 @@ final class DifferentialChangesetParser extends Phobject { $vs = $id; } + $renderer + ->setDocumentEngine($engine) + ->setDocumentEngineBlocks($engine_blocks); + return $renderer->renderDocumentEngineBlocks( $engine_blocks, (string)$id, @@ -1653,7 +1667,7 @@ final class DifferentialChangesetParser extends Phobject { return $prefix.$line; } - private function newDocumentEngineChangesetView() { + private function newDocumentEngine() { $changeset = $this->changeset; $viewer = $this->getViewer(); @@ -1724,7 +1738,8 @@ final class DifferentialChangesetParser extends Phobject { } if ($document_engine) { - return $document_engine->newDiffView( + return array( + $document_engine, $old_ref, $new_ref); } diff --git a/src/applications/differential/parser/DifferentialHunkParser.php b/src/applications/differential/parser/DifferentialHunkParser.php index 84f466adcd..6cdadf69e7 100644 --- a/src/applications/differential/parser/DifferentialHunkParser.php +++ b/src/applications/differential/parser/DifferentialHunkParser.php @@ -36,7 +36,7 @@ final class DifferentialHunkParser extends Phobject { } public function getVisibleLinesMask() { if ($this->visibleLinesMask === null) { - throw new PhutilInvalidStateException('generateVisibileLinesMask'); + throw new PhutilInvalidStateException('generateVisibleLinesMask'); } return $this->visibleLinesMask; } @@ -354,7 +354,7 @@ final class DifferentialHunkParser extends Phobject { return $this; } - public function generateVisibileLinesMask($lines_context) { + public function generateVisibleLinesMask($lines_context) { $old = $this->getOldLines(); $new = $this->getNewLines(); $max_length = max(count($old), count($new)); diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php index ae8db861b6..c856b9b14d 100644 --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php @@ -270,11 +270,20 @@ abstract class DifferentialChangesetHTMLRenderer } } - if ($this->getHighlightingDisabled()) { - $messages[] = pht( - 'This file is larger than %s, so syntax highlighting is '. - 'disabled by default.', - phutil_format_bytes(DifferentialChangesetParser::HIGHLIGHT_BYTE_LIMIT)); + $blocks = $this->getDocumentEngineBlocks(); + if ($blocks) { + foreach ($blocks->getMessages() as $message) { + $messages[] = $message; + } + } else { + if ($this->getHighlightingDisabled()) { + $byte_limit = DifferentialChangesetParser::HIGHLIGHT_BYTE_LIMIT; + $byte_limit = phutil_format_bytes($byte_limit); + $messages[] = pht( + 'This file is larger than %s, so syntax highlighting is '. + 'disabled by default.', + $byte_limit); + } } return $this->formatHeaderMessages($messages); diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php index e628a5a2f8..fc50b0d605 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php @@ -233,42 +233,233 @@ final class DifferentialChangesetOneUpRenderer $old_changeset_key, $new_changeset_key) { - // TODO: This should eventually merge into the normal primitives pathway, - // but fake it for now and just share as much code as possible. + $engine = $this->getDocumentEngine(); + $layout = $block_list->newTwoUpLayout(); - $primitives = array(); - foreach ($block_list->newOneUpLayout() as $block) { - $primitives[] = array( - 'type' => 'old-file', - 'htype' => '', - 'line' => $block->getBlockKey(), - 'render' => $block->newContentView(), - ); + $old_comments = $this->getOldComments(); + $new_comments = $this->getNewComments(); + + $unchanged = array(); + foreach ($layout as $key => $row) { + list($old, $new) = $row; + + if (!$old) { + continue; + } + + if (!$new) { + continue; + } + + if ($old->getDifferenceType() !== null) { + continue; + } + + if ($new->getDifferenceType() !== null) { + continue; + } + + $unchanged[$key] = true; } - // TODO: We'd like to share primitive code here, but buildPrimitives() - // currently chokes on changesets with no textual data. - foreach ($this->getOldComments() as $line => $group) { - foreach ($group as $comment) { - $primitives[] = array( - 'type' => 'inline', - 'comment' => $comment, - 'right' => false, + $rows = array(); + $count = count($layout); + for ($ii = 0; $ii < $count;) { + $start = $ii; + + for ($jj = $ii; $jj < $count; $jj++) { + list($old, $new) = $layout[$jj]; + + if (empty($unchanged[$jj])) { + break; + } + + $rows[] = array( + 'type' => 'unchanged', + 'layoutKey' => $jj, ); } + $ii = $jj; + + for ($jj = $ii; $jj < $count; $jj++) { + list($old, $new) = $layout[$jj]; + + if (!empty($unchanged[$jj])) { + break; + } + + $rows[] = array( + 'type' => 'old', + 'layoutKey' => $jj, + ); + } + + for ($jj = $ii; $jj < $count; $jj++) { + list($old, $new) = $layout[$jj]; + + if (!empty($unchanged[$jj])) { + break; + } + + $rows[] = array( + 'type' => 'new', + 'layoutKey' => $jj, + ); + } + $ii = $jj; + + // We always expect to consume at least one row when iterating through + // the loop and make progress. If we don't, bail out to avoid spinning + // to death. + if ($ii === $start) { + throw new Exception( + pht( + 'Failed to make progress during 1up diff layout.')); + } } - foreach ($this->getNewComments() as $line => $group) { - foreach ($group as $comment) { - $primitives[] = array( - 'type' => 'inline', - 'comment' => $comment, - 'right' => true, - ); + $old_ref = null; + $new_ref = null; + $refs = $block_list->getDocumentRefs(); + if ($refs) { + list($old_ref, $new_ref) = $refs; + } + + $view = array(); + foreach ($rows as $row) { + $row_type = $row['type']; + $layout_key = $row['layoutKey']; + $row_layout = $layout[$layout_key]; + list($old, $new) = $row_layout; + + if ($old) { + $old_key = $old->getBlockKey(); + } else { + $old_key = null; + } + + if ($new) { + $new_key = $new->getBlockKey(); + } else { + $new_key = null; + } + + $cells = array(); + $cell_classes = array(); + + if ($row_type === 'unchanged') { + $cell_content = $engine->newBlockContentView( + $old_ref, + $old); + } else if ($old && $new) { + $block_diff = $engine->newBlockDiffViews( + $old_ref, + $old, + $new_ref, + $new); + + // TODO: We're currently double-rendering this: once when building + // the old row, and once when building the new one. In both cases, + // we throw away the other half of the output. We could cache this + // to improve performance. + + if ($row_type === 'old') { + $cell_content = $block_diff->getOldContent(); + $cell_classes = $block_diff->getOldClasses(); + } else { + $cell_content = $block_diff->getNewContent(); + $cell_classes = $block_diff->getNewClasses(); + } + } else if ($row_type === 'old') { + $cell_content = $engine->newBlockContentView( + $old_ref, + $old); + $cell_classes[] = 'old'; + $cell_classes[] = 'old-full'; + + $new_key = null; + } else if ($row_type === 'new') { + $cell_content = $engine->newBlockContentView( + $new_ref, + $new); + $cell_classes[] = 'new'; + $cell_classes[] = 'new-full'; + + $old_key = null; + } + + if ($old_key === null) { + $old_id = null; + } else { + $old_id = "C{$old_changeset_key}OL{$old_key}"; + } + + if ($new_key === null) { + $new_id = null; + } else { + $new_id = "C{$new_changeset_key}NL{$new_key}"; + } + + $cells[] = phutil_tag( + 'td', + array( + 'id' => $old_id, + 'data-n' => $old_key, + 'class' => 'n', + )); + + $cells[] = phutil_tag( + 'td', + array( + 'id' => $new_id, + 'data-n' => $new_key, + 'class' => 'n', + )); + + $cells[] = phutil_tag( + 'td', + array( + 'class' => 'copy', + )); + + $cell_classes[] = 'diff-flush'; + $cell_classes = implode(' ', $cell_classes); + + $cells[] = phutil_tag( + 'td', + array( + 'class' => $cell_classes, + 'data-copy-mode' => 'copy-unified', + ), + $cell_content); + + $view[] = phutil_tag( + 'tr', + array(), + $cells); + + if ($old_key !== null) { + $old_inlines = idx($old_comments, $old_key, array()); + foreach ($old_inlines as $inline) { + $inline = $this->buildInlineComment( + $inline, + $on_right = false); + $view[] = $this->getRowScaffoldForInline($inline); + } + } + + if ($new_key !== null) { + $new_inlines = idx($new_comments, $new_key, array()); + foreach ($new_inlines as $inline) { + $inline = $this->buildInlineComment( + $inline, + $on_right = true); + $view[] = $this->getRowScaffoldForInline($inline); + } } } - $output = $this->renderPrimitives($primitives, 1); + $output = $this->wrapChangeInTable($view); return $this->renderChangesetTable($output); } diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php index e6c6f01fb7..6b1ed8fa27 100644 --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -36,6 +36,9 @@ abstract class DifferentialChangesetRenderer extends Phobject { private $scopeEngine = false; private $depthOnlyLines; + private $documentEngine; + private $documentEngineBlocks; + private $oldFile = false; private $newFile = false; @@ -239,6 +242,25 @@ abstract class DifferentialChangesetRenderer extends Phobject { return $this->oldChangesetID; } + public function setDocumentEngine(PhabricatorDocumentEngine $engine) { + $this->documentEngine = $engine; + return $this; + } + + public function getDocumentEngine() { + return $this->documentEngine; + } + + public function setDocumentEngineBlocks( + PhabricatorDocumentEngineBlocks $blocks) { + $this->documentEngineBlocks = $blocks; + return $this; + } + + public function getDocumentEngineBlocks() { + return $this->documentEngineBlocks; + } + public function setNewComments(array $new_comments) { foreach ($new_comments as $line_number => $comments) { assert_instances_of($comments, 'PhabricatorInlineCommentInterface'); @@ -355,6 +377,16 @@ abstract class DifferentialChangesetRenderer extends Phobject { $notice = null; if ($this->getIsTopLevel()) { $force = (!$content && !$props); + + // If we have DocumentEngine messages about the blocks, assume they + // explain why there's no content. + $blocks = $this->getDocumentEngineBlocks(); + if ($blocks) { + if ($blocks->getMessages()) { + $force = false; + } + } + $notice = $this->renderChangeTypeHeader($force); } diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php index 8285baa0ad..a7af9333ff 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -369,52 +369,123 @@ final class DifferentialChangesetTwoUpRenderer $old_changeset_key, $new_changeset_key) { + $engine = $this->getDocumentEngine(); + + $old_ref = null; + $new_ref = null; + $refs = $block_list->getDocumentRefs(); + if ($refs) { + list($old_ref, $new_ref) = $refs; + } + $old_comments = $this->getOldComments(); $new_comments = $this->getNewComments(); + $gap_view = javelin_tag( + 'tr', + array( + 'sigil' => 'context-target', + ), + phutil_tag( + 'td', + array( + 'colspan' => 6, + 'class' => 'show-more', + ), + pht("\xE2\x80\xA2 \xE2\x80\xA2 \xE2\x80\xA2"))); + $rows = array(); + $in_gap = false; foreach ($block_list->newTwoUpLayout() as $row) { list($old, $new) = $row; if ($old) { - $old_content = $old->newContentView(); $old_key = $old->getBlockKey(); - - $old_classes = $old->getClasses(); - - if ($old->getDifferenceType() === '-') { - $old_classes[] = 'old'; - $old_classes[] = 'old-full'; - } - - $old_classes[] = 'diff-flush'; - - $old_classes = implode(' ', $old_classes); + $is_visible = $old->getIsVisible(); } else { - $old_content = null; $old_key = null; - $old_classes = null; } if ($new) { - $new_content = $new->newContentView(); $new_key = $new->getBlockKey(); - $new_classes = $new->getClasses(); + $is_visible = $new->getIsVisible(); + } else { + $new_key = null; + } - if ($new->getDifferenceType() === '+') { - $new_classes[] = 'new'; - $new_classes[] = 'new-full'; + if (!$is_visible) { + if (!$in_gap) { + $in_gap = true; + $rows[] = $gap_view; + } + continue; + } + + if ($in_gap) { + $in_gap = false; + } + + if ($old) { + $is_rem = ($old->getDifferenceType() === '-'); + } else { + $is_rem = false; + } + + if ($new) { + $is_add = ($new->getDifferenceType() === '+'); + } else { + $is_add = false; + } + + if ($is_rem && $is_add) { + $block_diff = $engine->newBlockDiffViews( + $old_ref, + $old, + $new_ref, + $new); + + $old_content = $block_diff->getOldContent(); + $new_content = $block_diff->getNewContent(); + + $old_classes = $block_diff->getOldClasses(); + $new_classes = $block_diff->getNewClasses(); + } else { + $old_classes = array(); + $new_classes = array(); + + if ($old) { + $old_content = $engine->newBlockContentView( + $old_ref, + $old); + + if ($is_rem) { + $old_classes[] = 'old'; + $old_classes[] = 'old-full'; + } + } else { + $old_content = null; } - $new_classes[] = 'diff-flush'; + if ($new) { + $new_content = $engine->newBlockContentView( + $new_ref, + $new); - $new_classes = implode(' ', $new_classes); - } else { - $new_content = null; - $new_key = null; - $new_classes = null; + if ($is_add) { + $new_classes[] = 'new'; + $new_classes[] = 'new-full'; + } + } else { + $new_content = null; + } } + $old_classes[] = 'diff-flush'; + $old_classes = implode(' ', $old_classes); + + $new_classes[] = 'diff-flush'; + $new_classes = implode(' ', $new_classes); + $old_inline_rows = array(); if ($old_key !== null) { $old_inlines = idx($old_comments, $old_key, array()); diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlock.php b/src/applications/files/diff/PhabricatorDocumentEngineBlock.php index 38e9b56454..e2d5af23b2 100644 --- a/src/applications/files/diff/PhabricatorDocumentEngineBlock.php +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlock.php @@ -5,9 +5,9 @@ final class PhabricatorDocumentEngineBlock private $blockKey; private $content; - private $classes = array(); private $differenceHash; private $differenceType; + private $isVisible; public function setContent($content) { $this->content = $content; @@ -18,10 +18,6 @@ final class PhabricatorDocumentEngineBlock return $this->content; } - public function newContentView() { - return $this->getContent(); - } - public function setBlockKey($block_key) { $this->blockKey = $block_key; return $this; @@ -31,15 +27,6 @@ final class PhabricatorDocumentEngineBlock return $this->blockKey; } - public function addClass($class) { - $this->classes[] = $class; - return $this; - } - - public function getClasses() { - return $this->classes; - } - public function setDifferenceHash($difference_hash) { $this->differenceHash = $difference_hash; return $this; @@ -58,4 +45,13 @@ final class PhabricatorDocumentEngineBlock return $this->differenceType; } + public function setIsVisible($is_visible) { + $this->isVisible = $is_visible; + return $this; + } + + public function getIsVisible() { + return $this->isVisible; + } + } diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlockDiff.php b/src/applications/files/diff/PhabricatorDocumentEngineBlockDiff.php new file mode 100644 index 0000000000..b582d523c6 --- /dev/null +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlockDiff.php @@ -0,0 +1,47 @@ +oldContent = $old_content; + return $this; + } + + public function getOldContent() { + return $this->oldContent; + } + + public function setNewContent($new_content) { + $this->newContent = $new_content; + return $this; + } + + public function getNewContent() { + return $this->newContent; + } + + public function addOldClass($class) { + $this->oldClasses[] = $class; + return $this; + } + + public function getOldClasses() { + return $this->oldClasses; + } + + public function addNewClass($class) { + $this->newClasses[] = $class; + return $this; + } + + public function getNewClasses() { + return $this->newClasses; + } + +} diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php index fc45c204a8..f8f3a414b1 100644 --- a/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php @@ -4,6 +4,16 @@ final class PhabricatorDocumentEngineBlocks extends Phobject { private $lists = array(); + private $messages = array(); + + public function addMessage($message) { + $this->messages[] = $message; + return $this; + } + + public function getMessages() { + return $this->messages; + } public function addBlockList(PhabricatorDocumentRef $ref, array $blocks) { assert_instances_of($blocks, 'PhabricatorDocumentEngineBlock'); @@ -16,10 +26,18 @@ final class PhabricatorDocumentEngineBlocks return $this; } + public function getDocumentRefs() { + return ipull($this->lists, 'ref'); + } + public function newTwoUpLayout() { $rows = array(); $lists = $this->lists; + if (count($lists) != 2) { + return array(); + } + $specs = array(); foreach ($this->lists as $list) { $specs[] = $this->newDiffSpec($list['blocks']); @@ -38,6 +56,9 @@ final class PhabricatorDocumentEngineBlocks ->parseHunksForLineData($changeset->getHunks()) ->reparseHunksForSpecialAttributes(); + $hunk_parser->generateVisibleLinesMask(2); + $mask = $hunk_parser->getVisibleLinesMask(); + $old_lines = $hunk_parser->getOldLines(); $new_lines = $hunk_parser->getNewLines(); @@ -48,6 +69,15 @@ final class PhabricatorDocumentEngineBlocks $old_line = idx($old_lines, $ii); $new_line = idx($new_lines, $ii); + $is_visible = !empty($mask[$ii + 1]); + + // TODO: There's currently a bug where one-line files get incorrectly + // masked. This causes images to completely fail to render. Just ignore + // the mask if it came back empty. + if (!$mask) { + $is_visible = true; + } + if ($old_line) { $old_hash = rtrim($old_line['text'], "\n"); if (!strlen($old_hash)) { @@ -55,7 +85,9 @@ final class PhabricatorDocumentEngineBlocks $old_block = null; } else { $old_block = array_shift($old_map[$old_hash]); - $old_block->setDifferenceType($old_line['type']); + $old_block + ->setDifferenceType($old_line['type']) + ->setIsVisible($is_visible); } } else { $old_block = null; @@ -67,7 +99,9 @@ final class PhabricatorDocumentEngineBlocks $new_block = null; } else { $new_block = array_shift($new_map[$new_hash]); - $new_block->setDifferenceType($new_line['type']); + $new_block + ->setDifferenceType($new_line['type']) + ->setIsVisible($is_visible); } } else { $new_block = null; diff --git a/src/applications/files/document/PhabricatorDocumentEngine.php b/src/applications/files/document/PhabricatorDocumentEngine.php index 7e2a0d3b34..bc7960b4ad 100644 --- a/src/applications/files/document/PhabricatorDocumentEngine.php +++ b/src/applications/files/document/PhabricatorDocumentEngine.php @@ -37,7 +37,31 @@ abstract class PhabricatorDocumentEngine return false; } - public function newDiffView( + public function newBlockDiffViews( + PhabricatorDocumentRef $uref, + PhabricatorDocumentEngineBlock $ublock, + PhabricatorDocumentRef $vref, + PhabricatorDocumentEngineBlock $vblock) { + + $u_content = $this->newBlockContentView($uref, $ublock); + $v_content = $this->newBlockContentView($vref, $vblock); + + return id(new PhabricatorDocumentEngineBlockDiff()) + ->setOldContent($u_content) + ->addOldClass('old') + ->addOldClass('old-full') + ->setNewContent($v_content) + ->addNewClass('new') + ->addNewClass('new-full'); + } + + public function newBlockContentView( + PhabricatorDocumentRef $ref, + PhabricatorDocumentEngineBlock $block) { + return $block->getContent(); + } + + public function newEngineBlocks( PhabricatorDocumentRef $uref, PhabricatorDocumentRef $vref) { throw new PhutilMethodNotImplementedException(); diff --git a/src/applications/files/document/PhabricatorImageDocumentEngine.php b/src/applications/files/document/PhabricatorImageDocumentEngine.php index 53dcb83965..fa678cc034 100644 --- a/src/applications/files/document/PhabricatorImageDocumentEngine.php +++ b/src/applications/files/document/PhabricatorImageDocumentEngine.php @@ -27,7 +27,7 @@ final class PhabricatorImageDocumentEngine return ($uref->getFile() && $vref->getFile()); } - public function newDiffView( + public function newEngineBlocks( PhabricatorDocumentRef $uref, PhabricatorDocumentRef $vref) { @@ -39,6 +39,23 @@ final class PhabricatorImageDocumentEngine ->addBlockList($vref, $v_blocks); } + public function newBlockDiffViews( + PhabricatorDocumentRef $uref, + PhabricatorDocumentEngineBlock $ublock, + PhabricatorDocumentRef $vref, + PhabricatorDocumentEngineBlock $vblock) { + + $u_content = $this->newBlockContentView($uref, $ublock); + $v_content = $this->newBlockContentView($vref, $vblock); + + return id(new PhabricatorDocumentEngineBlockDiff()) + ->setOldContent($u_content) + ->addOldClass('diff-image-cell') + ->setNewContent($v_content) + ->addNewClass('diff-image-cell'); + } + + private function newDiffBlocks(PhabricatorDocumentRef $ref) { $blocks = array(); @@ -59,7 +76,6 @@ final class PhabricatorImageDocumentEngine $blocks[] = id(new PhabricatorDocumentEngineBlock()) ->setBlockKey('1') - ->addClass('diff-image-cell') ->setDifferenceHash($hash) ->setContent($image_view); diff --git a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php index 9836797f84..b5c8b04b6d 100644 --- a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php +++ b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php @@ -41,16 +41,143 @@ final class PhabricatorJupyterDocumentEngine return true; } - public function newDiffView( + public function newEngineBlocks( PhabricatorDocumentRef $uref, PhabricatorDocumentRef $vref) { - $u_blocks = $this->newDiffBlocks($uref); - $v_blocks = $this->newDiffBlocks($vref); + $blocks = new PhabricatorDocumentEngineBlocks(); - return id(new PhabricatorDocumentEngineBlocks()) - ->addBlockList($uref, $u_blocks) - ->addBlockList($vref, $v_blocks); + try { + $u_blocks = $this->newDiffBlocks($uref); + $v_blocks = $this->newDiffBlocks($vref); + + $blocks->addBlockList($uref, $u_blocks); + $blocks->addBlockList($vref, $v_blocks); + } catch (Exception $ex) { + $blocks->addMessage($ex->getMessage()); + } + + return $blocks; + } + + public function newBlockDiffViews( + PhabricatorDocumentRef $uref, + PhabricatorDocumentEngineBlock $ublock, + PhabricatorDocumentRef $vref, + PhabricatorDocumentEngineBlock $vblock) { + + $ucell = $ublock->getContent(); + $vcell = $vblock->getContent(); + + $utype = idx($ucell, 'cell_type'); + $vtype = idx($vcell, 'cell_type'); + + if ($utype === $vtype) { + switch ($utype) { + case 'markdown': + $usource = idx($ucell, 'source'); + $usource = implode('', $usource); + + $vsource = idx($vcell, 'source'); + $vsource = implode('', $vsource); + + $diff = id(new PhutilProseDifferenceEngine()) + ->getDiff($usource, $vsource); + + $u_content = $this->newProseDiffCell($diff, array('=', '-')); + $v_content = $this->newProseDiffCell($diff, array('=', '+')); + + $u_content = $this->newJupyterCell(null, $u_content, null); + $v_content = $this->newJupyterCell(null, $v_content, null); + + $u_content = $this->newCellContainer($u_content); + $v_content = $this->newCellContainer($v_content); + + return id(new PhabricatorDocumentEngineBlockDiff()) + ->setOldContent($u_content) + ->addOldClass('old') + ->setNewContent($v_content) + ->addNewClass('new'); + } + } + + return parent::newBlockDiffViews($uref, $ublock, $vref, $vblock); + } + + public function newBlockContentView( + PhabricatorDocumentRef $ref, + PhabricatorDocumentEngineBlock $block) { + + $viewer = $this->getViewer(); + $cell = $block->getContent(); + + $cell_content = $this->renderJupyterCell($viewer, $cell); + + return $this->newCellContainer($cell_content); + } + + private function newCellContainer($cell_content) { + $notebook_table = phutil_tag( + 'table', + array( + 'class' => 'jupyter-notebook', + ), + $cell_content); + + $container = phutil_tag( + 'div', + array( + 'class' => 'document-engine-jupyter document-engine-diff', + ), + $notebook_table); + + return $container; + } + + private function newProseDiffCell(PhutilProseDiff $diff, array $mask) { + $mask = array_fuse($mask); + + $result = array(); + foreach ($diff->getParts() as $part) { + $type = $part['type']; + $text = $part['text']; + + if (!isset($mask[$type])) { + continue; + } + + switch ($type) { + case '-': + $result[] = phutil_tag( + 'span', + array( + 'class' => 'bright', + ), + $text); + break; + case '+': + $result[] = phutil_tag( + 'span', + array( + 'class' => 'bright', + ), + $text); + break; + case '=': + $result[] = $text; + break; + } + } + + return array( + null, + phutil_tag( + 'div', + array( + 'class' => 'jupyter-cell-markdown', + ), + $result), + ); } private function newDiffBlocks(PhabricatorDocumentRef $ref) { @@ -62,22 +189,6 @@ final class PhabricatorJupyterDocumentEngine $idx = 1; $blocks = array(); foreach ($cells as $cell) { - $cell_content = $this->renderJupyterCell($viewer, $cell); - - $notebook_table = phutil_tag( - 'table', - array( - 'class' => 'jupyter-notebook', - ), - $cell_content); - - $container = phutil_tag( - 'div', - array( - 'class' => 'document-engine-jupyter document-engine-diff', - ), - $notebook_table); - // When the cell is a source code line, we can hash just the raw // input rather than all the cell metadata. @@ -85,6 +196,9 @@ final class PhabricatorJupyterDocumentEngine case 'code/line': $hash_input = $cell['raw']; break; + case 'markdown': + $hash_input = implode('', $cell['source']); + break; default: $hash_input = serialize($cell); break; @@ -97,7 +211,7 @@ final class PhabricatorJupyterDocumentEngine $blocks[] = id(new PhabricatorDocumentEngineBlock()) ->setBlockKey($idx) ->setDifferenceHash($hash) - ->setContent($container); + ->setContent($cell); $idx++; } @@ -197,6 +311,26 @@ final class PhabricatorJupyterDocumentEngine foreach ($cells as $cell) { $cell_type = idx($cell, 'cell_type'); + if ($cell_type === 'markdown') { + $source = $cell['source']; + $source = implode('', $source); + + // Attempt to split contiguous blocks of markdown into smaller + // pieces. + + $chunks = preg_split( + '/\n\n+/', + $source); + + foreach ($chunks as $chunk) { + $result = $cell; + $result['source'] = array($chunk); + $results[] = $result; + } + + continue; + } + if ($cell_type !== 'code') { $results[] = $cell; continue; @@ -254,13 +388,6 @@ final class PhabricatorJupyterDocumentEngine list($label, $content) = $this->renderJupyterCellContent($viewer, $cell); - $label_cell = phutil_tag( - 'td', - array( - 'class' => 'jupyter-label', - ), - $label); - $classes = null; switch (idx($cell, 'cell_type')) { case 'code/line': @@ -268,6 +395,20 @@ final class PhabricatorJupyterDocumentEngine break; } + return $this->newJupyterCell( + $label, + $content, + $classes); + } + + private function newJupyterCell($label, $content, $classes) { + $label_cell = phutil_tag( + 'td', + array( + 'class' => 'jupyter-label', + ), + $label); + $content_cell = phutil_tag( 'td', array( @@ -300,7 +441,8 @@ final class PhabricatorJupyterDocumentEngine return $this->newCodeOutputCell($cell); } - return $this->newRawCell(id(new PhutilJSON())->encodeFormatted($cell)); + return $this->newRawCell(id(new PhutilJSON()) + ->encodeFormatted($cell)); } private function newRawCell($content) { @@ -321,8 +463,9 @@ final class PhabricatorJupyterDocumentEngine $content = array(); } - $content = implode('', $content); - $content = phutil_escape_html_newlines($content); + // TODO: This should ideally highlight as Markdown, but the "md" + // highlighter in Pygments is painfully slow and not terribly useful. + $content = $this->highlightLines($content, 'txt'); return array( null, @@ -507,15 +650,20 @@ final class PhabricatorJupyterDocumentEngine return $label; } - private function highlightLines(array $lines) { - $head = head($lines); - $matches = null; - if (preg_match('/^%%(.*)$/', $head, $matches)) { - $restore = array_shift($lines); - $lang = $matches[1]; + private function highlightLines(array $lines, $force_language = null) { + if ($force_language === null) { + $head = head($lines); + $matches = null; + if (preg_match('/^%%(.*)$/', $head, $matches)) { + $restore = array_shift($lines); + $lang = $matches[1]; + } else { + $restore = null; + $lang = 'py'; + } } else { $restore = null; - $lang = 'py'; + $lang = $force_language; } $content = PhabricatorSyntaxHighlighter::highlightWithLanguage( diff --git a/src/applications/paste/query/PhabricatorPasteQuery.php b/src/applications/paste/query/PhabricatorPasteQuery.php index 54b53ceb4a..693c82780f 100644 --- a/src/applications/paste/query/PhabricatorPasteQuery.php +++ b/src/applications/paste/query/PhabricatorPasteQuery.php @@ -378,15 +378,15 @@ final class PhabricatorPasteQuery } private function highlightSource($source, $title, $language) { - if (empty($language)) { - return PhabricatorSyntaxHighlighter::highlightWithFilename( - $title, - $source); - } else { - return PhabricatorSyntaxHighlighter::highlightWithLanguage( - $language, - $source); - } + if (empty($language)) { + return PhabricatorSyntaxHighlighter::highlightWithFilename( + $title, + $source); + } else { + return PhabricatorSyntaxHighlighter::highlightWithLanguage( + $language, + $source); + } } public function getQueryApplicationClass() { diff --git a/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php b/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php index fe5cab8622..02477186ff 100644 --- a/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php +++ b/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php @@ -17,7 +17,7 @@ final class PHUIDiffOneUpInlineCommentRowScaffold $inline = head($inlines); $attrs = array( - 'colspan' => 3, + 'colspan' => 2, 'id' => $inline->getScaffoldCellID(), ); @@ -32,6 +32,7 @@ final class PHUIDiffOneUpInlineCommentRowScaffold $cells = array( phutil_tag('td', array('class' => 'n'), $left_hidden), phutil_tag('td', array('class' => 'n'), $right_hidden), + phutil_tag('td', array('class' => 'copy')), phutil_tag('td', $attrs, $inline), ); diff --git a/webroot/rsrc/css/phui/phui-property-list-view.css b/webroot/rsrc/css/phui/phui-property-list-view.css index 61be658feb..98cb0db820 100644 --- a/webroot/rsrc/css/phui/phui-property-list-view.css +++ b/webroot/rsrc/css/phui/phui-property-list-view.css @@ -315,6 +315,16 @@ div.phui-property-list-stacked .phui-property-list-properties border-width: 0 1px; } +td.new .jupyter-cell-code-line { + background: {$new-background}; + border-color: {$new-bright}; +} + +td.old .jupyter-cell-code-line { + background: {$old-background}; + border-color: {$old-bright}; +} + .jupyter-cell-code-head { border-top-width: 1px; margin-top: 4px; @@ -364,3 +374,7 @@ div.phui-property-list-stacked .phui-property-list-properties .jupyter-output-html { background: {$sh-indigobackground}; } + +.jupyter-cell-markdown { + white-space: pre-wrap; +}