Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Commit 10f2413

Browse files
author
epriestley
committed
Render inline comment suggestions as real diffs
Summary: Ref T13513. When rendering an inline suggestion for display, use highlighting and diffing. Test Plan: {F7495053} Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21277
1 parent 8465621 commit 10f2413

File tree

7 files changed

+98
-18
lines changed

7 files changed

+98
-18
lines changed

resources/celerity/map.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
'core.pkg.css' => 'ba768cdb',
1313
'core.pkg.js' => '845355f4',
1414
'dark-console.pkg.js' => '187792c2',
15-
'differential.pkg.css' => 'f924dbcf',
15+
'differential.pkg.css' => '5c459f92',
1616
'differential.pkg.js' => '256a327a',
1717
'diffusion.pkg.css' => '42c75c37',
1818
'diffusion.pkg.js' => 'a98c0bf7',
@@ -65,7 +65,7 @@
6565
'rsrc/css/application/differential/add-comment.css' => '7e5900d9',
6666
'rsrc/css/application/differential/changeset-view.css' => '60c3d405',
6767
'rsrc/css/application/differential/core.css' => '7300a73e',
68-
'rsrc/css/application/differential/phui-inline-comment.css' => '4107254a',
68+
'rsrc/css/application/differential/phui-inline-comment.css' => '9863a85e',
6969
'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d',
7070
'rsrc/css/application/differential/revision-history.css' => '8aa3eac5',
7171
'rsrc/css/application/differential/revision-list.css' => '93d2df7d',
@@ -854,7 +854,7 @@
854854
'phui-icon-view-css' => '4cbc684a',
855855
'phui-image-mask-css' => '62c7f4d2',
856856
'phui-info-view-css' => 'a10a909b',
857-
'phui-inline-comment-view-css' => '4107254a',
857+
'phui-inline-comment-view-css' => '9863a85e',
858858
'phui-invisible-character-view-css' => 'c694c4a4',
859859
'phui-left-right-css' => '68513c34',
860860
'phui-lightbox-css' => '4ebf22da',

src/applications/differential/parser/DifferentialChangesetParser.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -834,7 +834,6 @@ public function render(
834834
->setNewAttachesToNewFile($this->rightSideAttachesToNewFile)
835835
->setCodeCoverage($this->getCoverage())
836836
->setRenderingReference($this->getRenderingReference())
837-
->setMarkupEngine($this->markupEngine)
838837
->setHandles($this->handles)
839838
->setOldLines($this->old)
840839
->setNewLines($this->new)
@@ -845,6 +844,10 @@ public function render(
845844
->setHighlightingDisabled($this->highlightingDisabled)
846845
->setDepthOnlyLines($this->getDepthOnlyLines());
847846

847+
if ($this->markupEngine) {
848+
$renderer->setMarkupEngine($this->markupEngine);
849+
}
850+
848851
list($engine, $old_ref, $new_ref) = $this->newDocumentEngine();
849852
if ($engine) {
850853
$engine_blocks = $engine->newEngineBlocks(

src/applications/differential/render/DifferentialChangesetOneUpRenderer.php

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,17 @@
33
final class DifferentialChangesetOneUpRenderer
44
extends DifferentialChangesetHTMLRenderer {
55

6+
private $simpleMode;
7+
8+
public function setSimpleMode($simple_mode) {
9+
$this->simpleMode = $simple_mode;
10+
return $this;
11+
}
12+
13+
public function getSimpleMode() {
14+
return $this->simpleMode;
15+
}
16+
617
public function isOneUpRenderer() {
718
return true;
819
}
@@ -36,6 +47,8 @@ public function renderTextChange(
3647
protected function renderPrimitives(array $primitives, $rows) {
3748
list($left_prefix, $right_prefix) = $this->getLineIDPrefixes();
3849

50+
$is_simple = $this->getSimpleMode();
51+
3952
$no_copy = phutil_tag('td', array('class' => 'copy'));
4053
$no_coverage = null;
4154

@@ -185,6 +198,12 @@ protected function renderPrimitives(array $primitives, $rows) {
185198
$cells[] = $no_coverage;
186199
}
187200

201+
// In simple mode, only render the text. This is used to render
202+
// "Edit Suggestions" in inline comments.
203+
if ($is_simple) {
204+
$cells = array($cells[3]);
205+
}
206+
188207
$out[] = phutil_tag('tr', array(), $cells);
189208

190209
break;
@@ -231,11 +250,17 @@ protected function renderPrimitives(array $primitives, $rows) {
231250
}
232251
}
233252

253+
$result = null;
254+
234255
if ($out) {
235-
return $this->wrapChangeInTable(phutil_implode_html('', $out));
256+
if ($is_simple) {
257+
$result = $this->newSimpleTable($out);
258+
} else {
259+
$result = $this->wrapChangeInTable(phutil_implode_html('', $out));
260+
}
236261
}
237262

238-
return null;
263+
return $result;
239264
}
240265

241266
public function renderDocumentEngineBlocks(
@@ -488,4 +513,14 @@ public function getRowScaffoldForInline(PHUIDiffInlineCommentView $view) {
488513
->addInlineView($view);
489514
}
490515

516+
517+
private function newSimpleTable($content) {
518+
return phutil_tag(
519+
'table',
520+
array(
521+
'class' => 'diff-1up-simple-table',
522+
),
523+
$content);
524+
}
525+
491526
}

src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContext.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,20 @@
33
final class PhabricatorDiffInlineCommentContext
44
extends PhabricatorInlineCommentContext {
55

6+
private $filename;
67
private $headLines;
78
private $bodyLines;
89
private $tailLines;
910

11+
public function setFilename($filename) {
12+
$this->filename = $filename;
13+
return $this;
14+
}
15+
16+
public function getFilename() {
17+
return $this->filename;
18+
}
19+
1020
public function setHeadLines(array $head_lines) {
1121
$this->headLines = $head_lines;
1222
return $this;

src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,12 @@ protected function didFilterPage(array $inlines) {
289289
}
290290

291291
if ($inline->getIsNewFile()) {
292+
$vector = $changeset->getNewStatePathVector();
293+
$filename = last($vector);
292294
$corpus = $changeset->makeNewFile();
293295
} else {
296+
$vector = $changeset->getOldStatePathVector();
297+
$filename = last($vector);
294298
$corpus = $changeset->makeOldFile();
295299
}
296300

@@ -321,6 +325,7 @@ protected function didFilterPage(array $inlines) {
321325
$tail = $this->simplifyContext($tail, false);
322326

323327
$context = id(new PhabricatorDiffInlineCommentContext())
328+
->setFilename($filename)
324329
->setHeadLines($head)
325330
->setBodyLines($body)
326331
->setTailLines($tail);

src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -540,20 +540,35 @@ private function newSuggestionView(PhabricatorInlineComment $inline) {
540540
return null;
541541
}
542542

543+
$viewer = $this->getViewer();
543544

544-
$raw_diff = id(new PhabricatorDifferenceEngine())
545-
->generateRawDiffFromFileContent($old_lines, $new_lines);
545+
$changeset = id(new PhabricatorDifferenceEngine())
546+
->generateChangesetFromFileContent($old_lines, $new_lines);
546547

547-
$raw_diff = phutil_split_lines($raw_diff);
548-
$raw_diff = array_slice($raw_diff, 3);
549-
$raw_diff = implode('', $raw_diff);
548+
$changeset->setFilename($context->getFilename());
549+
550+
// TODO: This isn't cached!
551+
552+
$viewstate = new PhabricatorChangesetViewState();
553+
554+
$parser = id(new DifferentialChangesetParser())
555+
->setViewer($viewer)
556+
->setViewstate($viewstate)
557+
->setChangeset($changeset);
558+
559+
$renderer = new DifferentialChangesetOneUpRenderer();
560+
$renderer->setSimpleMode(true);
561+
562+
$parser->setRenderer($renderer);
563+
564+
$diff_view = $parser->render(0, 0xFFFF, array());
550565

551566
$view = phutil_tag(
552567
'div',
553568
array(
554569
'class' => 'inline-suggestion-view PhabricatorMonospaced',
555570
),
556-
$raw_diff);
571+
$diff_view);
557572

558573
return $view;
559574
}

webroot/rsrc/css/application/differential/phui-inline-comment.css

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -476,20 +476,32 @@ textarea.inline-suggestion-input {
476476
border-right: 1px solid {$lightgreyborder};
477477
}
478478

479-
.inline-suggestion-input-cell {
480-
padding: 8px;
479+
.inline-suggestion-table td.inline-suggestion-input-cell {
480+
padding: 8px 4px;
481481
}
482482

483-
.inline-suggestion-text-cell {
484-
padding: 0 8px;
483+
.inline-suggestion-table td.inline-suggestion-text-cell {
484+
/* This is attempting to align the text in the textarea with the text on
485+
the surrounding context lines. */
486+
padding: 0 8px 0 11px;
485487
}
486488

487489
.inline-suggestion-view {
488-
padding: 8px 12px;
490+
padding: 4px 0;
489491
white-space: pre-wrap;
490-
background: {$greybackground};
492+
background: {$lightgreybackground};
491493
margin: 0 -12px 8px;
492494
border-width: 1px 0;
493495
border-style: solid;
494496
border-color: {$lightgreyborder};
495497
}
498+
499+
.diff-1up-simple-table {
500+
width: 100%;
501+
table-layout: fixed;
502+
}
503+
504+
.diff-1up-simple-table > tbody > tr > td {
505+
padding-left: 12px;
506+
padding-right: 12px;
507+
}

0 commit comments

Comments
 (0)