From 5afdc620db596e5f7e41e79192a08fdd87a2df6c Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 27 Sep 2019 14:02:53 -0700 Subject: [PATCH] Make basic Juypter notebook rendering improvements and roughly support folding unchanged context Summary: Depends on D20843. Ref T13425. Add very basic support for "Show Hidden Context", in the form of folding it behind an unclickable shield. This isn't ideal, but should be better than nothing. Prepare for "intraline" diffs on content blocks. Fix newline handling in Markdown sections in Jupyter notebooks. Remove the word "visibile" from the codebase. Test Plan: {F6898192} Maniphest Tasks: T13425 Differential Revision: https://secure.phabricator.com/D20844 --- resources/celerity/map.php | 6 +- .../parser/DifferentialChangesetParser.php | 6 +- .../parser/DifferentialHunkParser.php | 4 +- .../DifferentialChangesetTwoUpRenderer.php | 65 +++++++++++++++++-- .../diff/PhabricatorDocumentEngineBlock.php | 10 +++ .../diff/PhabricatorDocumentEngineBlocks.php | 20 +++++- .../PhabricatorJupyterDocumentEngine.php | 27 +++++--- .../paste/query/PhabricatorPasteQuery.php | 18 ++--- .../rsrc/css/phui/phui-property-list-view.css | 14 ++++ 9 files changed, 137 insertions(+), 33 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index ad7675abff..7116134d1a 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' => '19ec9519', + 'core.pkg.css' => '7ce5a944', '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', @@ -865,7 +865,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/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 3154a8a668..0408257201 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()); 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/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php index 8285baa0ad..127e81edf7 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -372,12 +372,25 @@ final class DifferentialChangesetTwoUpRenderer $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(); @@ -390,14 +403,14 @@ final class DifferentialChangesetTwoUpRenderer $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(); @@ -409,12 +422,56 @@ final class DifferentialChangesetTwoUpRenderer $new_classes[] = 'diff-flush'; $new_classes = implode(' ', $new_classes); + + $is_visible = $new->getIsVisible(); } else { - $new_content = null; $new_key = null; $new_classes = null; } + 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) { + list($old_content, $new_content) = array( + $old->newContentView(), + $new->newContentView(), + ); + } else { + if ($old) { + $old_content = $old->newContentView(); + } else { + $old_content = null; + } + + if ($new) { + $new_content = $new->newContentView(); + } else { + $new_content = null; + } + } + $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..f35a17cb0b 100644 --- a/src/applications/files/diff/PhabricatorDocumentEngineBlock.php +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlock.php @@ -8,6 +8,7 @@ final class PhabricatorDocumentEngineBlock private $classes = array(); private $differenceHash; private $differenceType; + private $isVisible; public function setContent($content) { $this->content = $content; @@ -58,4 +59,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/PhabricatorDocumentEngineBlocks.php b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php index 0dc033d74c..3013d0968e 100644 --- a/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php @@ -52,6 +52,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(); @@ -62,6 +65,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)) { @@ -69,7 +81,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; @@ -81,7 +95,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/PhabricatorJupyterDocumentEngine.php b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php index 17e32d0cd0..cfe66bf331 100644 --- a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php +++ b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php @@ -307,7 +307,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) { @@ -328,8 +329,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, @@ -514,15 +516,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/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; +}