Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14090180
D20840.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
20 KB
Referenced Files
None
Subscribers
None
D20840.diff
View Options
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('<Invalid Output>');
@@ -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
- // <td /> 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 <td /> 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));
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mon, Nov 25, 4:07 PM (19 h, 21 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6785612
Default Alt Text
D20840.diff (20 KB)
Attached To
Mode
D20840: When rendering Jupyter notebook diffs, split code inputs into individual blocks
Attached
Detach File
Event Timeline
Log In to Comment