diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,10 +9,10 @@ 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '6a8c9533', + 'core.pkg.css' => '19ec9519', 'core.pkg.js' => '6e5c894f', - 'differential.pkg.css' => 'abd2c0d8', - 'differential.pkg.js' => '68fa36fc', + 'differential.pkg.css' => '607c84be', + 'differential.pkg.js' => 'a0212a0b', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -61,7 +61,7 @@ 'rsrc/css/application/dashboard/dashboard.css' => '5a205b9d', 'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d', 'rsrc/css/application/differential/add-comment.css' => '7e5900d9', - 'rsrc/css/application/differential/changeset-view.css' => '5dda5e53', + 'rsrc/css/application/differential/changeset-view.css' => '489b6995', 'rsrc/css/application/differential/core.css' => '7300a73e', 'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b', 'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d', @@ -113,7 +113,7 @@ 'rsrc/css/application/uiexample/example.css' => 'b4795059', 'rsrc/css/core/core.css' => '1b29ed61', 'rsrc/css/core/remarkup.css' => 'f06cc20e', - 'rsrc/css/core/syntax.css' => '4234f572', + 'rsrc/css/core/syntax.css' => '220b85f9', 'rsrc/css/core/z-index.css' => '99c0f5eb', 'rsrc/css/diviner/diviner-shared.css' => '4bd263b0', 'rsrc/css/font/font-awesome.css' => '3883938a', @@ -169,7 +169,7 @@ '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' => '34180764', + 'rsrc/css/phui/phui-property-list-view.css' => 'ad841f1c', 'rsrc/css/phui/phui-remarkup-preview.css' => '91767007', 'rsrc/css/phui/phui-segment-bar-view.css' => '5166b370', 'rsrc/css/phui/phui-spacing.css' => 'b05cadc3', @@ -377,7 +377,7 @@ 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', 'rsrc/js/application/diff/DiffChangeset.js' => 'a31ffc00', - 'rsrc/js/application/diff/DiffChangesetList.js' => '22f6bb51', + 'rsrc/js/application/diff/DiffChangesetList.js' => '0f5c016d', 'rsrc/js/application/diff/DiffInline.js' => 'a4a14a94', 'rsrc/js/application/diff/behavior-preview-link.js' => 'f51e9c17', 'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd', @@ -554,7 +554,7 @@ 'conpherence-thread-manager' => 'aec8e38c', 'conpherence-transaction-css' => '3a3f5e7e', 'd3' => '9d068042', - 'differential-changeset-view-css' => '5dda5e53', + 'differential-changeset-view-css' => '489b6995', 'differential-core-view-css' => '7300a73e', 'differential-revision-add-comment-css' => '7e5900d9', 'differential-revision-comment-css' => '7dbc8d1d', @@ -774,7 +774,7 @@ 'phabricator-darkmessage' => '26cd4b73', 'phabricator-dashboard-css' => '5a205b9d', 'phabricator-diff-changeset' => 'a31ffc00', - 'phabricator-diff-changeset-list' => '22f6bb51', + 'phabricator-diff-changeset-list' => '0f5c016d', 'phabricator-diff-inline' => 'a4a14a94', 'phabricator-drag-and-drop-file-upload' => '4370900d', 'phabricator-draggable-list' => 'c9ad6f70', @@ -865,7 +865,7 @@ 'phui-pager-css' => 'd022c7ad', 'phui-pinboard-view-css' => '1f08f5d8', 'phui-policy-section-view-css' => '139fdc64', - 'phui-property-list-view-css' => '34180764', + 'phui-property-list-view-css' => 'ad841f1c', 'phui-remarkup-preview-css' => '91767007', 'phui-segment-bar-view-css' => '5166b370', 'phui-spacing-css' => 'b05cadc3', @@ -900,7 +900,7 @@ 'sprite-login-css' => '18b368a6', 'sprite-tokens-css' => 'f1896dc5', 'syntax-default-css' => '055fc231', - 'syntax-highlighting-css' => '4234f572', + 'syntax-highlighting-css' => '220b85f9', 'tokens-css' => 'ce5a50bd', 'trigger-rule' => '41b7b4f6', 'trigger-rule-control' => '5faf27b9', @@ -1005,6 +1005,10 @@ 'javelin-workflow', 'phuix-icon-view', ), + '0f5c016d' => array( + 'javelin-install', + 'phuix-button-view', + ), '111bfd2d' => array( 'javelin-install', ), @@ -1065,6 +1069,9 @@ 'javelin-behavior', 'javelin-request', ), + '220b85f9' => array( + 'syntax-default-css', + ), '225bbb98' => array( 'javelin-install', 'javelin-reactor', @@ -1075,10 +1082,6 @@ 'javelin-typeahead-source', 'javelin-util', ), - '22f6bb51' => array( - 'javelin-install', - 'phuix-button-view', - ), 23387297 => array( 'javelin-install', 'javelin-util', @@ -1260,9 +1263,6 @@ 'javelin-behavior', 'javelin-uri', ), - '4234f572' => array( - 'syntax-default-css', - ), '4370900d' => array( 'javelin-install', 'javelin-util', @@ -1303,6 +1303,9 @@ 'javelin-dom', 'phabricator-draggable-list', ), + '489b6995' => array( + 'phui-inline-comment-view-css', + ), '48fe33d0' => array( 'javelin-behavior', 'javelin-dom', @@ -1457,9 +1460,6 @@ 'javelin-dom', 'phuix-dropdown-menu', ), - '5dda5e53' => array( - 'phui-inline-comment-view-css', - ), '5faf27b9' => array( 'phuix-form-control-view', ), diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -387,6 +387,8 @@ $old_classes[] = 'old-full'; } + $old_classes[] = 'diff-flush'; + $old_classes = implode(' ', $old_classes); } else { $old_content = null; @@ -404,6 +406,8 @@ $new_classes[] = 'new-full'; } + $new_classes[] = 'diff-flush'; + $new_classes = implode(' ', $new_classes); } else { $new_content = null; @@ -451,6 +455,7 @@ 'td', array( 'class' => $old_classes, + 'data-copy-mode' => 'copy-l', ), $old_content); @@ -479,6 +484,7 @@ array( 'class' => $new_classes, 'colspan' => '2', + 'data-copy-mode' => 'copy-r', ), $new_content); diff --git a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php --- a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php +++ b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php @@ -57,11 +57,7 @@ $viewer = $this->getViewer(); $content = $ref->loadData(); - $data = phutil_json_decode($content); - $cells = idx($data, 'cells'); - if (!is_array($cells)) { - throw new Exception('Missing "cells".'); - } + $cells = $this->newCells($content, true); $idx = 1; $blocks = array(); @@ -82,8 +78,20 @@ ), $notebook_table); + // When the cell is a source code line, we can hash just the raw + // input rather than all the cell metadata. + + switch (idx($cell, 'cell_type')) { + case 'code/line': + $hash_input = $cell['raw']; + break; + default: + $hash_input = serialize($cell); + break; + } + $hash = PhabricatorHash::digestWithNamedKey( - serialize($cell), + $hash_input, 'document-engine.content-digest'); $blocks[] = id(new PhabricatorDocumentEngineBlock()) @@ -101,10 +109,39 @@ $viewer = $this->getViewer(); $content = $ref->loadData(); + try { + $cells = $this->newCells($content, false); + } catch (Exception $ex) { + return $this->newMessage($ex->getMessage()); + } + + $rows = array(); + foreach ($cells as $cell) { + $rows[] = $this->renderJupyterCell($viewer, $cell); + } + + $notebook_table = phutil_tag( + 'table', + array( + 'class' => 'jupyter-notebook', + ), + $rows); + + $container = phutil_tag( + 'div', + array( + 'class' => 'document-engine-jupyter', + ), + $notebook_table); + + return $container; + } + + private function newCells($content, $for_diff) { try { $data = phutil_json_decode($content); } catch (PhutilJSONParserException $ex) { - return $this->newMessage( + throw new Exception( pht( 'This is not a valid JSON document and can not be rendered as '. 'a Jupyter notebook: %s.', @@ -112,7 +149,7 @@ } if (!is_array($data)) { - return $this->newMessage( + throw new Exception( pht( 'This document does not encode a valid JSON object and can not '. 'be rendered as a Jupyter notebook.')); @@ -121,14 +158,14 @@ $nbformat = idx($data, 'nbformat'); if (!strlen($nbformat)) { - return $this->newMessage( + throw new Exception( pht( 'This document is missing an "nbformat" field. Jupyter notebooks '. 'must have this field.')); } if ($nbformat !== 4) { - return $this->newMessage( + throw new Exception( pht( 'This Jupyter notebook uses an unsupported version of the file '. 'format (found version %s, expected version 4).', @@ -137,39 +174,80 @@ $cells = idx($data, 'cells'); if (!is_array($cells)) { - return $this->newMessage( + throw new Exception( pht( 'This Jupyter notebook does not specify a list of "cells".')); } if (!$cells) { - return $this->newMessage( + throw new Exception( pht( 'This Jupyter notebook does not specify any notebook cells.')); } - $rows = array(); - foreach ($cells as $cell) { - $rows[] = $this->renderJupyterCell($viewer, $cell); + if (!$for_diff) { + return $cells; } - $notebook_table = phutil_tag( - 'table', - array( - 'class' => 'jupyter-notebook', - ), - $rows); + // If we're extracting cells to build a diff view, split code cells into + // individual lines and individual outputs. We want users to be able to + // add inline comments to each line and each output block. - $container = phutil_tag( - 'div', - array( - 'class' => 'document-engine-jupyter', - ), - $notebook_table); + $results = array(); + foreach ($cells as $cell) { + $cell_type = idx($cell, 'cell_type'); - return $container; + if ($cell_type !== 'code') { + $results[] = $cell; + continue; + } + + $label = $this->newCellLabel($cell); + + $lines = idx($cell, 'source'); + if (!is_array($lines)) { + $lines = array(); + } + + $content = $this->highlightLines($lines); + + $count = count($lines); + for ($ii = 0; $ii < $count; $ii++) { + $is_head = ($ii === 0); + $is_last = ($ii === ($count - 1)); + + if ($is_head) { + $line_label = $label; + } else { + $line_label = null; + } + + $results[] = array( + 'cell_type' => 'code/line', + 'label' => $line_label, + 'raw' => $lines[$ii], + 'display' => idx($content, $ii), + 'head' => $is_head, + 'last' => $is_last, + ); + } + + $outputs = array(); + $output_list = idx($cell, 'outputs'); + if (is_array($output_list)) { + foreach ($output_list as $output) { + $results[] = array( + 'cell_type' => 'code/output', + 'output' => $output, + ); + } + } + } + + return $results; } + private function renderJupyterCell( PhabricatorUser $viewer, array $cell) { @@ -177,13 +255,24 @@ list($label, $content) = $this->renderJupyterCellContent($viewer, $cell); $label_cell = phutil_tag( - 'th', - array(), + 'td', + array( + 'class' => 'jupyter-label', + ), $label); + $classes = null; + switch (idx($cell, 'cell_type')) { + case 'code/line': + $classes = 'jupyter-cell-flush'; + break; + } + $content_cell = phutil_tag( 'td', - array(), + array( + 'class' => $classes, + ), $content); return phutil_tag( @@ -204,7 +293,11 @@ case 'markdown': return $this->newMarkdownCell($cell); case 'code': - return $this->newCodeCell($cell); + return $this->newCodeCell($cell); + case 'code/line': + return $this->newCodeLineCell($cell); + case 'code/output': + return $this->newCodeOutputCell($cell); } return $this->newRawCell(id(new PhutilJSON())->encodeFormatted($cell)); @@ -243,23 +336,14 @@ } private function newCodeCell(array $cell) { - $execution_count = idx($cell, 'execution_count'); - if ($execution_count) { - $label = 'In ['.$execution_count.']:'; - } else { - $label = null; - } + $label = $this->newCellLabel($cell); $content = idx($cell, 'source'); if (!is_array($content)) { $content = array(); } - $content = implode('', $content); - - $content = PhabricatorSyntaxHighlighter::highlightWithLanguage( - 'py', - $content); + $content = $this->highlightLines($content); $outputs = array(); $output_list = idx($cell, 'outputs'); @@ -275,7 +359,9 @@ phutil_tag( 'div', array( - 'class' => 'jupyter-cell-code PhabricatorMonospaced remarkup-code', + 'class' => + 'jupyter-cell-code jupyter-cell-code-block '. + 'PhabricatorMonospaced remarkup-code', ), array( $content, @@ -285,6 +371,45 @@ ); } + private function newCodeLineCell(array $cell) { + $classes = array(); + $classes[] = 'PhabricatorMonospaced'; + $classes[] = 'remarkup-code'; + $classes[] = 'jupyter-cell-code'; + $classes[] = 'jupyter-cell-code-line'; + + if ($cell['head']) { + $classes[] = 'jupyter-cell-code-head'; + } + + if ($cell['last']) { + $classes[] = 'jupyter-cell-code-last'; + } + + $classes = implode(' ', $classes); + + return array( + $cell['label'], + array( + phutil_tag( + 'div', + array( + 'class' => $classes, + ), + array( + $cell['display'], + )), + ), + ); + } + + private function newCodeOutputCell(array $cell) { + return array( + null, + $this->newOutput($cell['output']), + ); + } + private function newOutput(array $output) { if (!is_array($output)) { return pht(''); @@ -371,4 +496,45 @@ $content); } + private function newCellLabel(array $cell) { + $execution_count = idx($cell, 'execution_count'); + if ($execution_count) { + $label = 'In ['.$execution_count.']:'; + } else { + $label = null; + } + + return $label; + } + + private function highlightLines(array $lines) { + $head = head($lines); + $matches = null; + if (preg_match('/^%%(.*)$/', $head, $matches)) { + $restore = array_shift($lines); + $lang = $matches[1]; + } else { + $restore = null; + $lang = 'py'; + } + + $content = PhabricatorSyntaxHighlighter::highlightWithLanguage( + $lang, + implode('', $lines)); + $content = phutil_split_lines($content); + + if ($restore !== null) { + $language_tag = phutil_tag( + 'span', + array( + 'class' => 'language-tag', + ), + $restore); + + array_unshift($content, $language_tag); + } + + return $content; + } + } diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -63,6 +63,11 @@ padding: 1px 8px; } +.differential-diff td.diff-flush { + padding-top: 0; + padding-bottom: 0; +} + .device .differential-diff td { padding: 1px 4px; } diff --git a/webroot/rsrc/css/core/syntax.css b/webroot/rsrc/css/core/syntax.css --- a/webroot/rsrc/css/core/syntax.css +++ b/webroot/rsrc/css/core/syntax.css @@ -6,6 +6,10 @@ color: #aa0066; } +.remarkup-code .language-tag { + color: {$lightgreytext}; +} + .remarkup-code td > span { display: inline; word-break: break-all; diff --git a/webroot/rsrc/css/phui/phui-property-list-view.css b/webroot/rsrc/css/phui/phui-property-list-view.css --- a/webroot/rsrc/css/phui/phui-property-list-view.css +++ b/webroot/rsrc/css/phui/phui-property-list-view.css @@ -298,22 +298,56 @@ .jupyter-cell-code { white-space: pre-wrap; + word-break: break-word; background: {$lightgreybackground}; - padding: 8px; - border: 1px solid {$lightgreyborder}; border-radius: 2px; + border-color: {$lightgreyborder}; + border-style: solid; +} + +.jupyter-cell-code-block { + padding: 8px; + border-width: 1px; +} + +.jupyter-cell-code-line { + padding: 2px 8px; + border-width: 0 1px; +} + +.jupyter-cell-code-head { + border-top-width: 1px; + margin-top: 4px; + padding-top: 8px; +} + +.jupyter-cell-code-last { + border-bottom-width: 1px; + margin-bottom: 4px; + padding-bottom: 8px; } -.jupyter-notebook > tbody > tr > th, .jupyter-notebook > tbody > tr > td { padding: 8px; } -.jupyter-notebook > tbody > tr > th { +.jupyter-notebook > tbody > tr > td.jupyter-cell-flush { + padding-top: 0; + padding-bottom: 0; +} + +.jupyter-notebook, +.jupyter-notebook > tbody > tr > td { + width: 100%; +} + +.jupyter-notebook > tbody > tr > td.jupyter-label { white-space: nowrap; text-align: right; - min-width: 48px; + min-width: 56px; font-weight: bold; + width: auto; + padding: 8px 8px 0; } .jupyter-output { diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -1195,41 +1195,26 @@ bot = tmp; } - // Find the leftmost cell that we're going to highlight: this is the next - // in the row that does not have a "data-n" (line number) - // attribute. In 2up views, it should be directly adjacent. In - // 1up views, we may have to skip over the other line number column. - var l = top; - while (l.nextSibling && l.getAttribute('data-n')) { - l = l.nextSibling; - } - - // Find the rightmost cell that we're going to highlight: this is the - // farthest consecutive, adjacent in the row that does not have - // a "data-n" (line number) attribute. Sometimes the left and right nodes - // are the same (left side of 2up view); sometimes we're going to - // highlight several nodes (copy + code + coverage). - var r = l; - while (true) { - // No more cells in the row, so we can't keep expanding. - if (!r.nextSibling) { - break; - } - - if (r.nextSibling.getAttribute('data-n')) { - break; - } + // Find the leftmost cell that we're going to highlight. This is the + // next sibling with a "data-copy-mode" attribute, which is a marker + // for the cell with actual content in it. + var content_cell = top; + while (content_cell && !content_cell.getAttribute('data-copy-mode')) { + content_cell = content_cell.nextSibling; + } - r = r.nextSibling; + // If we didn't find a cell to highlight, don't highlight anything. + if (!content_cell) { + return; } - var pos = JX.$V(l) - .add(JX.Vector.getAggregateScrollForNode(l)); + var pos = JX.$V(content_cell) + .add(JX.Vector.getAggregateScrollForNode(content_cell)); - var dim = JX.$V(r) - .add(JX.Vector.getAggregateScrollForNode(r)) + var dim = JX.$V(content_cell) + .add(JX.Vector.getAggregateScrollForNode(content_cell)) .add(-pos.x, -pos.y) - .add(JX.Vector.getDim(r)); + .add(JX.Vector.getDim(content_cell)); var bpos = JX.$V(bot) .add(JX.Vector.getAggregateScrollForNode(bot));