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

Commit 1656a2f

Browse files
author
epriestley
committed
Allow inline comment storage objects to generate their own runtime objects
Summary: Ref T13513. Currently, inline storage objects ("TransactionComment") can't directly generate a runtime object ("InlineComment"). Allow this transformation to be performed in a genric way so clunky code which does it per-object-type can be removed, lifted, or simplified. Simplify an especially gross callsite in preview code. Test Plan: Previewed inline comments. Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21226
1 parent 3976488 commit 1656a2f

File tree

7 files changed

+41
-23
lines changed

7 files changed

+41
-23
lines changed

resources/celerity/map.php

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
'core.pkg.js' => '1e667bcb',
1414
'dark-console.pkg.js' => '187792c2',
1515
'differential.pkg.css' => '2d70b7b9',
16-
'differential.pkg.js' => '22ec6f26',
16+
'differential.pkg.js' => 'faa6e8ab',
1717
'diffusion.pkg.css' => '42c75c37',
1818
'diffusion.pkg.js' => 'a98c0bf7',
1919
'maniphest.pkg.css' => '35995d6d',
@@ -433,7 +433,7 @@
433433
'rsrc/js/application/releeph/releeph-preview-branch.js' => '75184d68',
434434
'rsrc/js/application/releeph/releeph-request-state-change.js' => '9f081f05',
435435
'rsrc/js/application/releeph/releeph-request-typeahead.js' => 'aa3a100c',
436-
'rsrc/js/application/repository/repository-crossreference.js' => '1c95ea63',
436+
'rsrc/js/application/repository/repository-crossreference.js' => '6337cf26',
437437
'rsrc/js/application/search/behavior-reorder-profile-menu-items.js' => 'e5bdb730',
438438
'rsrc/js/application/search/behavior-reorder-queries.js' => 'b86f297f',
439439
'rsrc/js/application/transactions/behavior-comment-actions.js' => '4dffaeb2',
@@ -685,7 +685,7 @@
685685
'javelin-behavior-reorder-applications' => 'aa371860',
686686
'javelin-behavior-reorder-columns' => '8ac32fd9',
687687
'javelin-behavior-reorder-profile-menu-items' => 'e5bdb730',
688-
'javelin-behavior-repository-crossreference' => '1c95ea63',
688+
'javelin-behavior-repository-crossreference' => '6337cf26',
689689
'javelin-behavior-scrollbar' => '92388bae',
690690
'javelin-behavior-search-reorder-queries' => 'b86f297f',
691691
'javelin-behavior-select-content' => 'e8240b50',
@@ -1049,12 +1049,6 @@
10491049
'javelin-install',
10501050
'javelin-util',
10511051
),
1052-
'1c95ea63' => array(
1053-
'javelin-behavior',
1054-
'javelin-dom',
1055-
'javelin-stratcom',
1056-
'javelin-uri',
1057-
),
10581052
'1cab0e9a' => array(
10591053
'javelin-behavior',
10601054
'javelin-dom',
@@ -1501,6 +1495,12 @@
15011495
'60cd9241' => array(
15021496
'javelin-behavior',
15031497
),
1498+
'6337cf26' => array(
1499+
'javelin-behavior',
1500+
'javelin-dom',
1501+
'javelin-stratcom',
1502+
'javelin-uri',
1503+
),
15041504
'65bb0011' => array(
15051505
'javelin-behavior',
15061506
'javelin-dom',

src/__phutil_library_map__.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3593,6 +3593,7 @@
35933593
'PhabricatorInfrastructureTestCase' => '__tests__/PhabricatorInfrastructureTestCase.php',
35943594
'PhabricatorInlineComment' => 'infrastructure/diff/interface/PhabricatorInlineComment.php',
35953595
'PhabricatorInlineCommentController' => 'infrastructure/diff/PhabricatorInlineCommentController.php',
3596+
'PhabricatorInlineCommentInterface' => 'applications/transactions/interface/PhabricatorInlineCommentInterface.php',
35963597
'PhabricatorInlineSummaryView' => 'infrastructure/diff/view/PhabricatorInlineSummaryView.php',
35973598
'PhabricatorInstructionsEditField' => 'applications/transactions/editfield/PhabricatorInstructionsEditField.php',
35983599
'PhabricatorIntConfigType' => 'applications/config/type/PhabricatorIntConfigType.php',
@@ -6789,7 +6790,10 @@
67896790
'DifferentialTestPlanField' => 'DifferentialCoreCustomField',
67906791
'DifferentialTitleCommitMessageField' => 'DifferentialCommitMessageField',
67916792
'DifferentialTransaction' => 'PhabricatorModularTransaction',
6792-
'DifferentialTransactionComment' => 'PhabricatorApplicationTransactionComment',
6793+
'DifferentialTransactionComment' => array(
6794+
'PhabricatorApplicationTransactionComment',
6795+
'PhabricatorInlineCommentInterface',
6796+
),
67936797
'DifferentialTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
67946798
'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
67956799
'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView',
@@ -8599,7 +8603,10 @@
85998603
'PhabricatorAuditStatusConstants' => 'Phobject',
86008604
'PhabricatorAuditSynchronizeManagementWorkflow' => 'PhabricatorAuditManagementWorkflow',
86018605
'PhabricatorAuditTransaction' => 'PhabricatorModularTransaction',
8602-
'PhabricatorAuditTransactionComment' => 'PhabricatorApplicationTransactionComment',
8606+
'PhabricatorAuditTransactionComment' => array(
8607+
'PhabricatorApplicationTransactionComment',
8608+
'PhabricatorInlineCommentInterface',
8609+
),
86038610
'PhabricatorAuditTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
86048611
'PhabricatorAuditTransactionView' => 'PhabricatorApplicationTransactionView',
86058612
'PhabricatorAuditUpdateOwnersManagementWorkflow' => 'PhabricatorAuditManagementWorkflow',

src/applications/audit/storage/PhabricatorAuditTransactionComment.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
<?php
22

33
final class PhabricatorAuditTransactionComment
4-
extends PhabricatorApplicationTransactionComment {
4+
extends PhabricatorApplicationTransactionComment
5+
implements
6+
PhabricatorInlineCommentInterface {
57

68
protected $commitPHID;
79
protected $pathID;
@@ -85,4 +87,8 @@ public function isEmptyInlineComment() {
8587
return !strlen($this->getContent());
8688
}
8789

90+
public function newInlineCommentObject() {
91+
return PhabricatorAuditInlineComment::newFromModernComment($this);
92+
}
93+
8894
}

src/applications/differential/storage/DifferentialTransactionComment.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
<?php
22

33
final class DifferentialTransactionComment
4-
extends PhabricatorApplicationTransactionComment {
4+
extends PhabricatorApplicationTransactionComment
5+
implements
6+
PhabricatorInlineCommentInterface {
57

68
protected $revisionPHID;
79
protected $changesetID;
@@ -131,4 +133,8 @@ public function isEmptyInlineComment() {
131133
return !strlen($this->getContent());
132134
}
133135

136+
public function newInlineCommentObject() {
137+
return DifferentialInlineComment::newFromModernComment($this);
138+
}
139+
134140
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
interface PhabricatorInlineCommentInterface {
4+
5+
public function newInlineCommentObject();
6+
7+
}

src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,7 @@ public function render() {
3838

3939
$inlines = $this->getInlineComments();
4040
foreach ($inlines as $key => $inline) {
41-
// TODO: This is real, real gross.
42-
43-
if ($inline instanceof DifferentialTransactionComment) {
44-
$inlines[$key] = DifferentialInlineComment::newFromModernComment(
45-
$inline);
46-
} else {
47-
$inlines[$key] = PhabricatorAuditInlineComment::newFromModernComment(
48-
$inline);
49-
}
41+
$inlines[$key] = $inline->newInlineCommentObject();
5042
}
5143

5244
$engine = new PhabricatorMarkupEngine();

webroot/rsrc/js/application/repository/repository-crossreference.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ JX.behavior('repository-crossreference', function(config, statics) {
219219
}
220220

221221
function getPath(target) {
222-
// This method works in Differential, when browsing a changset.
222+
// This method works in Differential, when browsing a changeset.
223223
var changeset;
224224
try {
225225
changeset = JX.DOM.findAbove(target, 'div', 'differential-changeset');

0 commit comments

Comments
 (0)