From 4aa72aa5ff826853b403961861dcebaf6b82080f Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Feb 2011 19:38:43 -0800 Subject: [PATCH] Inline comment-related fixes. --- src/__celerity_resource_map__.php | 28 +++---- src/__phutil_library_map__.php | 2 + ...AphrontDefaultApplicationConfiguration.php | 3 +- ...ifferentialInlineCommentEditController.php | 1 + ...erentialInlineCommentPreviewController.php | 65 +++++++++++++++ .../inlinecommentpreview/__init__.php | 19 +++++ .../DifferentialRevisionEditController.php | 10 +-- .../controller/revisionedit/__init__.php | 1 + .../DifferentialRevisionListController.php | 1 - .../DifferentialRevisionViewController.php | 45 +++++++++++ .../controller/revisionview/__init__.php | 2 + .../DifferentialRevisionListData.php | 4 - .../comment/DifferentialCommentEditor.php | 20 +++-- .../differential/editor/comment/__init__.php | 1 + .../revision/DifferentialRevisionEditor.php | 6 +- .../storage/revision/DifferentialRevision.php | 2 +- .../storage/revision/__init__.php | 1 - .../addcomment/DifferentialAddCommentView.php | 15 +++- .../DifferentialInlineCommentView.php | 11 ++- .../DifferentialRevisionCommentView.php | 79 ++++++++++++++++++- .../view/revisioncomment/__init__.php | 1 + .../DifferentialRevisionCommentListView.php | 19 ++++- .../view/revisioncommentlist/__init__.php | 2 + .../phid/handle/PhabricatorObjectHandle.php | 2 +- .../application/differential/add-comment.css | 7 ++ .../differential/revision-comment.css | 56 ++++++++++++- .../differential/behavior-comment-preview.js | 16 ++++ 27 files changed, 368 insertions(+), 51 deletions(-) create mode 100644 src/applications/differential/controller/inlinecommentpreview/DifferentialInlineCommentPreviewController.php create mode 100644 src/applications/differential/controller/inlinecommentpreview/__init__.php diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 30e05c36b6..041ae521fc 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -100,7 +100,7 @@ celerity_register_resource_map(array( ), 'differential-revision-add-comment-css' => array( - 'uri' => '/res/623fef21/rsrc/css/application/differential/add-comment.css', + 'uri' => '/res/d7f8719e/rsrc/css/application/differential/add-comment.css', 'type' => 'css', 'requires' => array( @@ -109,7 +109,7 @@ celerity_register_resource_map(array( ), 'differential-changeset-view-css' => array( - 'uri' => '/res/11e7232a/rsrc/css/application/differential/changeset-view.css', + 'uri' => '/res/4e0295a9/rsrc/css/application/differential/changeset-view.css', 'type' => 'css', 'requires' => array( @@ -136,7 +136,7 @@ celerity_register_resource_map(array( ), 'differential-revision-comment-css' => array( - 'uri' => '/res/bf6369c6/rsrc/css/application/differential/revision-comment.css', + 'uri' => '/res/368bd612/rsrc/css/application/differential/revision-comment.css', 'type' => 'css', 'requires' => array( @@ -245,7 +245,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-feedback-preview' => array( - 'uri' => '/res/34fbb670/rsrc/js/application/differential/behavior-comment-preview.js', + 'uri' => '/res/8695d8b8/rsrc/js/application/differential/behavior-comment-preview.js', 'type' => 'js', 'requires' => array( @@ -255,7 +255,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-edit-inline-comments' => array( - 'uri' => '/res/f5b54891/rsrc/js/application/differential/behavior-edit-inline-comments.js', + 'uri' => '/res/74747b2e/rsrc/js/application/differential/behavior-edit-inline-comments.js', 'type' => 'js', 'requires' => array( @@ -275,7 +275,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-show-more' => array( - 'uri' => '/res/d26ebcae/rsrc/js/application/differential/behavior-show-more.js', + 'uri' => '/res/ea998002/rsrc/js/application/differential/behavior-show-more.js', 'type' => 'js', 'requires' => array( @@ -303,7 +303,7 @@ celerity_register_resource_map(array( ), 'javelin-lib-dev' => array( - 'uri' => '/res/53784c9a/rsrc/js/javelin/javelin.dev.js', + 'uri' => '/res/a0e7a5e9/rsrc/js/javelin/javelin.dev.js', 'type' => 'js', 'requires' => array( @@ -378,7 +378,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/c5efa388/core.pkg.css', 'type' => 'css', ), - 'f399aad7' => + '9d9c881c' => array ( 'name' => 'differential.pkg.css', 'symbols' => @@ -389,7 +389,7 @@ celerity_register_resource_map(array( 3 => 'differential-revision-history-css', 4 => 'differential-table-of-contents-css', ), - 'uri' => '/res/pkg/f399aad7/differential.pkg.css', + 'uri' => '/res/pkg/9d9c881c/differential.pkg.css', 'type' => 'css', ), ), @@ -406,10 +406,10 @@ celerity_register_resource_map(array( 'aphront-tokenizer-control-css' => 'c5efa388', 'aphront-typeahead-control-css' => 'c5efa388', 'phabricator-directory-css' => 'c5efa388', - 'differential-core-view-css' => 'f399aad7', - 'differential-changeset-view-css' => 'f399aad7', - 'differential-revision-detail-css' => 'f399aad7', - 'differential-revision-history-css' => 'f399aad7', - 'differential-table-of-contents-css' => 'f399aad7', + 'differential-core-view-css' => '9d9c881c', + 'differential-changeset-view-css' => '9d9c881c', + 'differential-revision-detail-css' => '9d9c881c', + 'differential-revision-history-css' => '9d9c881c', + 'differential-table-of-contents-css' => '9d9c881c', ), )); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3ef9c0e99b..4d7055953b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -101,6 +101,7 @@ phutil_register_library_map(array( 'DifferentialHunk' => 'applications/differential/storage/hunk', 'DifferentialInlineComment' => 'applications/differential/storage/inlinecomment', 'DifferentialInlineCommentEditController' => 'applications/differential/controller/inlinecommentedit', + 'DifferentialInlineCommentPreviewController' => 'applications/differential/controller/inlinecommentpreview', 'DifferentialInlineCommentView' => 'applications/differential/view/inlinecomment', 'DifferentialLintStatus' => 'applications/differential/constants/lintstatus', 'DifferentialMail' => 'applications/differential/mail/base', @@ -287,6 +288,7 @@ phutil_register_library_map(array( 'DifferentialHunk' => 'DifferentialDAO', 'DifferentialInlineComment' => 'DifferentialDAO', 'DifferentialInlineCommentEditController' => 'DifferentialController', + 'DifferentialInlineCommentPreviewController' => 'DifferentialController', 'DifferentialInlineCommentView' => 'AphrontView', 'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail', 'DifferentialReviewRequestMail' => 'DifferentialMail', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index 09474c10ad..ebee81637b 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -90,7 +90,8 @@ class AphrontDefaultApplicationConfiguration 'preview/(?\d+)/$' => 'DifferentialCommentPreviewController', 'save/$' => 'DifferentialCommentSaveController', 'inline/' => array( - 'preview/$' => 'DifferentialInlineCommentPreviewController', + 'preview/(?\d+)/$' => + 'DifferentialInlineCommentPreviewController', 'edit/(?\d+)/$' => 'DifferentialInlineCommentEditController', ), ), diff --git a/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php index 1eb44e9c07..7134a83220 100644 --- a/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php +++ b/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php @@ -88,6 +88,7 @@ class DifferentialInlineCommentEditController extends DifferentialController { if ($request->isFormPost()) { if (strlen($text)) { $inline->setContent($text); + $inline->setCache(null); $inline->save(); return $this->buildRenderedCommentResponse( $inline, diff --git a/src/applications/differential/controller/inlinecommentpreview/DifferentialInlineCommentPreviewController.php b/src/applications/differential/controller/inlinecommentpreview/DifferentialInlineCommentPreviewController.php new file mode 100644 index 0000000000..b3a3e168b9 --- /dev/null +++ b/src/applications/differential/controller/inlinecommentpreview/DifferentialInlineCommentPreviewController.php @@ -0,0 +1,65 @@ +revisionID = $data['id']; + } + + public function processRequest() { + + $request = $this->getRequest(); + $user = $request->getUser(); + + // TODO: This is a reasonable approximation of the feature as it exists + // in Facebook trunk but we should probably pull filename data, sort these, + // figure out next/prev/edit/delete, deal with out-of-date inlines, etc. + + $inlines = id(new DifferentialInlineComment())->loadAllWhere( + 'authorPHID = %s AND revisionID = %d AND commentID IS NULL', + $user->getPHID(), + $this->revisionID); + + $factory = new DifferentialMarkupEngineFactory(); + $engine = $factory->newDifferentialCommentMarkupEngine(); + + $phids = array($user->getPHID()); + $handles = id(new PhabricatorObjectHandleData($phids)) + ->loadHandles(); + + $views = array(); + foreach ($inlines as $inline) { + $view = new DifferentialInlineCommentView(); + $view->setInlineComment($inline); + $view->setMarkupEngine($engine); + $view->setHandles($handles); + $view->setEditable(false); + $views[] = $view->render(); + } + $views = implode("\n", $views); + + return id(new AphrontAjaxResponse()) + ->setContent($views); + } + + +} diff --git a/src/applications/differential/controller/inlinecommentpreview/__init__.php b/src/applications/differential/controller/inlinecommentpreview/__init__.php new file mode 100644 index 0000000000..900156b3d4 --- /dev/null +++ b/src/applications/differential/controller/inlinecommentpreview/__init__.php @@ -0,0 +1,19 @@ +loadRelationships(); - if ($request->isFormPost() && !$request->getStr('viaDiffView')) { + if ($request->isFormPost() && !$request->getStr('viaDiffView')) { $revision->setTitle($request->getStr('title')); $revision->setSummary($request->getStr('summary')); $revision->setTestPlan($request->getStr('testplan')); @@ -98,14 +98,14 @@ class DifferentialRevisionEditController extends DifferentialController { $reviewer_phids = $revision->getReviewers(); $cc_phids = $revision->getCCPHIDs(); } - + $phids = array_merge($reviewer_phids, $cc_phids); $phids = array_unique($phids); - + $handles = id(new PhabricatorObjectHandleData($phids)) ->loadHandles(); $handles = mpull($handles, 'getFullName', 'getPHID'); - + $reviewer_map = array_select_keys($handles, $reviewer_phids); $cc_map = array_select_keys($handles, $cc_phids); diff --git a/src/applications/differential/controller/revisionedit/__init__.php b/src/applications/differential/controller/revisionedit/__init__.php index 872135cb60..805b2c0977 100644 --- a/src/applications/differential/controller/revisionedit/__init__.php +++ b/src/applications/differential/controller/revisionedit/__init__.php @@ -12,6 +12,7 @@ phutil_require_module('phabricator', 'applications/differential/controller/base' phutil_require_module('phabricator', 'applications/differential/editor/revision'); phutil_require_module('phabricator', 'applications/differential/storage/diff'); phutil_require_module('phabricator', 'applications/differential/storage/revision'); +phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/textarea'); diff --git a/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php b/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php index c7f5e3c309..2ef2e1e276 100644 --- a/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php +++ b/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php @@ -133,7 +133,6 @@ class DifferentialRevisionListController extends DifferentialController { ON revision.id = relationship.revisionID WHERE revision.id IN (%Ld) AND relationship.relation = %s - AND relationship.forbidden = 0 ORDER BY sequence', $rev->getTableName(), DifferentialRevision::RELATIONSHIP_TABLE, diff --git a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php index 75bac79d0e..78cf947987 100644 --- a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php @@ -46,6 +46,8 @@ class DifferentialRevisionViewController extends DifferentialController { $this->getImplicitComments($revision), $comments); + $inlines = $this->loadInlineComments($comments, $changesets); + $object_phids = array_merge( $revision->getReviewers(), $revision->getCCPHIDs(), @@ -54,6 +56,7 @@ class DifferentialRevisionViewController extends DifferentialController { $request->getUser()->getPHID(), ), mpull($comments, 'getAuthorPHID')); + $object_phids = array_unique($object_phids); $handles = id(new PhabricatorObjectHandleData($object_phids)) ->loadHandles(); @@ -70,6 +73,8 @@ class DifferentialRevisionViewController extends DifferentialController { $comment_view = new DifferentialRevisionCommentListView(); $comment_view->setComments($comments); $comment_view->setHandles($handles); + $comment_view->setInlineComments($inlines); + $comment_view->setChangesets($changesets); $diff_history = new DifferentialRevisionUpdateHistoryView(); $diff_history->setDiffs($diffs); @@ -267,6 +272,46 @@ class DifferentialRevisionViewController extends DifferentialController { return array_keys($actions); } + private function loadInlineComments(array $comments, array &$changesets) { + + $inline_comments = array(); + + $comment_ids = array_filter(mpull($comments, 'getID')); + if (!$comment_ids) { + return $inline_comments; + } + + $inline_comments = id(new DifferentialInlineComment()) + ->loadAllWhere( + 'commentID in (%Ld)', + $comment_ids); + + $load_changesets = array(); + foreach ($inline_comments as $inline) { + $changeset_id = $inline->getChangesetID(); + if (isset($changesets[$changeset_id])) { + continue; + } + $load_changesets[$changeset_id] = true; + } + + $more_changesets = array(); + if ($load_changesets) { + $changeset_ids = array_keys($load_changesets); + $more_changesets += id(new DifferentialChangeset()) + ->loadAllWhere( + 'id IN (%Ld)', + $changeset_ids); + } + + if ($more_changesets) { + $changesets += $more_changesets; + $changesets = msort($changesets, 'getSortKey'); + } + + return $inline_comments; + } + } /* diff --git a/src/applications/differential/controller/revisionview/__init__.php b/src/applications/differential/controller/revisionview/__init__.php index 04ea6dd3ac..d5e441eefd 100644 --- a/src/applications/differential/controller/revisionview/__init__.php +++ b/src/applications/differential/controller/revisionview/__init__.php @@ -10,7 +10,9 @@ phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'applications/differential/constants/action'); phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/controller/base'); +phutil_require_module('phabricator', 'applications/differential/storage/changeset'); phutil_require_module('phabricator', 'applications/differential/storage/comment'); +phutil_require_module('phabricator', 'applications/differential/storage/inlinecomment'); phutil_require_module('phabricator', 'applications/differential/storage/revision'); phutil_require_module('phabricator', 'applications/differential/view/addcomment'); phutil_require_module('phabricator', 'applications/differential/view/changesetlistview'); diff --git a/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php b/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php index 1fe22bb4c3..d4dbb6eb31 100755 --- a/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php +++ b/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php @@ -107,7 +107,6 @@ class DifferentialRevisionListData { SELECT revision.* FROM %T revision JOIN %T relationship ON relationship.revisionID = revision.id AND relationship.relation = %s - AND relationship.forbidden = 0 WHERE relationship.objectPHID IN (%Ls) AND revision.status in (%Ld) @@ -143,7 +142,6 @@ class DifferentialRevisionListData { SELECT revision.* FROM %T revision JOIN %T relationship ON relationship.revisionID = revision.id AND relationship.relation = %s - AND relationship.forbidden = 0 WHERE relationship.objectPHID IN (%Ls) AND revision.status in (%Ld) @@ -202,7 +200,6 @@ class DifferentialRevisionListData { FROM %T revision LEFT JOIN %T relationship ON revision.id = relationship.revisionID AND relationship.relation = %s - AND relationship.forbidden = 0 WHERE '.$pattern.' GROUP BY revision.id '.$this->getOrderClause(); @@ -241,7 +238,6 @@ class DifferentialRevisionListData { 'SELECT revision.* FROM %T revision JOIN %T relationship ON relationship.revisionID = revision.id AND relationship.relation = %s - AND relationship.forbidden = 0 AND relationship.objectPHID in (%Ls) WHERE revision.status in (%Ld) %Q', $revision->getTableName(), diff --git a/src/applications/differential/editor/comment/DifferentialCommentEditor.php b/src/applications/differential/editor/comment/DifferentialCommentEditor.php index b9e23528ad..6a3e22960c 100755 --- a/src/applications/differential/editor/comment/DifferentialCommentEditor.php +++ b/src/applications/differential/editor/comment/DifferentialCommentEditor.php @@ -241,15 +241,13 @@ class DifferentialCommentEditor { // Reload relationships to pick up any reviewer changes. $revision->loadRelationships(); -/* - TODO - $inline_comments = array(); if ($this->attachInlineComments) { - $inline_comments = id(new DifferentialInlineComment()) - ->loadAllUnsaved($revision, $this->actorPHID); + $inline_comments = id(new DifferentialInlineComment())->loadAllWhere( + 'authorPHID = %s AND revisionID = %d AND commentID IS NULL', + $this->actorPHID, + $revision->getID()); } -*/ $comment = id(new DifferentialComment()) ->setAuthorPHID($this->actorPHID) @@ -258,11 +256,11 @@ class DifferentialCommentEditor { ->setContent((string)$this->message) ->save(); -/* - $diff = id(new Diff())->loadActiveWithRevision($revision); - $changesets = id(new DifferentialChangeset())->loadAllWithDiff($diff); +// $diff = id(new Diff())->loadActiveWithRevision($revision); +// $changesets = id(new DifferentialChangeset())->loadAllWithDiff($diff); if ($inline_comments) { +/* // We may have feedback on non-current changesets. Rather than orphaning // it, just submit it. This is non-ideal but not horrible. $inline_changeset_ids = array_pull($inline_comments, 'getChangesetID'); @@ -275,12 +273,12 @@ class DifferentialCommentEditor { if ($load) { $changesets += id(new DifferentialChangeset())->loadAllWithIDs($load); } +*/ foreach ($inline_comments as $inline) { - $inline->setFeedbackID($feedback->getID()); + $inline->setCommentID($comment->getID()); $inline->save(); } } -*/ id(new DifferentialCommentMail( $revision, diff --git a/src/applications/differential/editor/comment/__init__.php b/src/applications/differential/editor/comment/__init__.php index 5eb988d3ae..1d14ca7092 100644 --- a/src/applications/differential/editor/comment/__init__.php +++ b/src/applications/differential/editor/comment/__init__.php @@ -11,6 +11,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/revisi phutil_require_module('phabricator', 'applications/differential/editor/revision'); phutil_require_module('phabricator', 'applications/differential/mail/comment'); phutil_require_module('phabricator', 'applications/differential/storage/comment'); +phutil_require_module('phabricator', 'applications/differential/storage/inlinecomment'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php index e140229a69..0bdc121d37 100644 --- a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php +++ b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php @@ -497,7 +497,7 @@ class DifferentialRevisionEditor { array $rem_phids, array $add_phids, $reason_phid) { - + $rem_map = array_fill_keys($rem_phids, true); $add_map = array_fill_keys($add_phids, true); @@ -516,7 +516,7 @@ class DifferentialRevisionEditor { $raw = $revision->getRawRelations(DifferentialRevision::RELATION_REVIEWER); $raw = ipull($raw, null, 'objectPHID'); - + $sequence = count($seq_map); foreach ($raw as $phid => $relation) { if (isset($seq_map[$phid])) { @@ -540,7 +540,7 @@ class DifferentialRevisionEditor { 'reasonPHID' => $reason_phid, ); } - + $conn_w = $revision->establishConnection('w'); $sql = array(); diff --git a/src/applications/differential/storage/revision/DifferentialRevision.php b/src/applications/differential/storage/revision/DifferentialRevision.php index 0c0a04d512..82834b2402 100755 --- a/src/applications/differential/storage/revision/DifferentialRevision.php +++ b/src/applications/differential/storage/revision/DifferentialRevision.php @@ -104,7 +104,7 @@ class DifferentialRevision extends DifferentialDAO { if ($this->relationships === null) { throw new Exception("Must load relationships!"); } - + return ipull($this->getRawRelations($relation), 'objectPHID'); } diff --git a/src/applications/differential/storage/revision/__init__.php b/src/applications/differential/storage/revision/__init__.php index 9a227dd210..3720822856 100644 --- a/src/applications/differential/storage/revision/__init__.php +++ b/src/applications/differential/storage/revision/__init__.php @@ -10,7 +10,6 @@ phutil_require_module('phabricator', 'applications/differential/storage/base'); phutil_require_module('phabricator', 'applications/differential/storage/comment'); phutil_require_module('phabricator', 'applications/differential/storage/diff'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); -phutil_require_module('phabricator', 'storage/qsprintf'); phutil_require_module('phabricator', 'storage/queryfx'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/differential/view/addcomment/DifferentialAddCommentView.php b/src/applications/differential/view/addcomment/DifferentialAddCommentView.php index 1fc3def9ee..a84e27ae51 100644 --- a/src/applications/differential/view/addcomment/DifferentialAddCommentView.php +++ b/src/applications/differential/view/addcomment/DifferentialAddCommentView.php @@ -72,13 +72,18 @@ final class DifferentialAddCommentView extends AphrontView { id(new AphrontFormSubmitControl()) ->setValue('Comment')); + $rev_id = $revision->getID(); + Javelin::initBehavior( 'differential-feedback-preview', array( - 'uri' => '/differential/comment/preview/'.$revision->getID().'/', - 'preview' => 'comment-preview', - 'action' => 'comment-action', - 'content' => 'comment-content', + 'uri' => '/differential/comment/preview/'.$rev_id.'/', + 'preview' => 'comment-preview', + 'action' => 'comment-action', + 'content' => 'comment-content', + + 'inlineuri' => '/differential/comment/inline/preview/'.$rev_id.'/', + 'inline' => 'inline-comment-preview', )); return @@ -93,6 +98,8 @@ final class DifferentialAddCommentView extends AphrontView { 'Loading comment preview...'. ''. ''. + '
'. + '
'. ''. ''; } diff --git a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php b/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php index a51dd7663a..ca5b6ad5dd 100644 --- a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php +++ b/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php @@ -109,7 +109,16 @@ final class DifferentialInlineCommentView extends AphrontView { $links = null; } - $content = $this->markupEngine->markupText($content); + $cache = $inline->getCache(); + if (strlen($cache)) { + $content = $cache; + } else { + $content = $this->markupEngine->markupText($content); + if ($inline->getID()) { + $inline->setCache($content); + $inline->save(); + } + } $markup = javelin_render_tag( 'div', diff --git a/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php b/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php index 9c174583fd..6149632244 100644 --- a/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php +++ b/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php @@ -22,6 +22,8 @@ final class DifferentialRevisionCommentView extends AphrontView { private $handles; private $markupEngine; private $preview; + private $inlines; + private $changesets; public function setComment($comment) { $this->comment = $comment; @@ -43,6 +45,17 @@ final class DifferentialRevisionCommentView extends AphrontView { return $this; } + public function setInlineComments(array $inline_comments) { + $this->inlines = $inline_comments; + return $this; + } + + public function setChangesets(array $changesets) { + // Ship these in sorted by getSortKey() and keyed by ID... or else! + $this->changesets = $changesets; + return $this; + } + public function render() { require_celerity_resource('phabricator-remarkup-css'); @@ -91,6 +104,67 @@ final class DifferentialRevisionCommentView extends AphrontView { ''; } + if ($this->inlines) { + $inline_render = array(); + $inlines = $this->inlines; + $changesets = $this->changesets; + $inlines_by_changeset = mgroup($inlines, 'getChangesetID'); + $inlines_by_changeset = array_select_keys( + $inlines_by_changeset, + array_keys($this->changesets)); + $inline_render[] = ''; + foreach ($inlines_by_changeset as $changeset_id => $inlines) { + $changeset = $changesets[$changeset_id]; + $inlines = msort($inlines, 'getLineNumber'); + $inline_render[] = + ''. + ''. + ''; + foreach ($inlines as $inline) { + if (!$inline->getLineLength()) { + $lines = $inline->getLineNumber(); + } else { + $lines = $inline->getLineNumber()."\xE2\x80\x93". + ($inline->getLineNumber() + $inline->getLineLength()); + } + + $lines = phutil_render_tag( + 'a', + array( + 'href' => '#', + 'class' => 'num', + ), + $lines); + + $content = $inline->getCache(); + if (!strlen($content)) { + $content = $this->markupEngine->markupText($content); + if ($inline->getID()) { + $inline->setCache($content); + $inline->save(); + } + } + + $inline_render[] = + ''. + ''. + ''. + ''; + } + } + $inline_render[] = '
'. + $changeset->getFileName(). + '
'.$lines.''.$content.'
'; + $inline_render = implode("\n", $inline_render); + $inline_render = + '
'. + 'Inline Comments'. + '
'. + $inline_render; + } else { + $inline_render = null; + } + $background = null; $uri = $author->getImageURI(); if ($uri) { @@ -104,10 +178,11 @@ final class DifferentialRevisionCommentView extends AphrontView { '
'.$title.'
'. ''. '
'. - '
'. - '
'. + '
'. + '
'. $content. '
'. + $inline_render. '
'. '
'. '
'; diff --git a/src/applications/differential/view/revisioncomment/__init__.php b/src/applications/differential/view/revisioncomment/__init__.php index 2f19fad814..e6fd906027 100644 --- a/src/applications/differential/view/revisioncomment/__init__.php +++ b/src/applications/differential/view/revisioncomment/__init__.php @@ -11,6 +11,7 @@ phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'view/base'); phutil_require_module('phutil', 'markup'); +phutil_require_module('phutil', 'utils'); phutil_require_source('DifferentialRevisionCommentView.php'); diff --git a/src/applications/differential/view/revisioncommentlist/DifferentialRevisionCommentListView.php b/src/applications/differential/view/revisioncommentlist/DifferentialRevisionCommentListView.php index dbcfc50b77..76edc7e89a 100644 --- a/src/applications/differential/view/revisioncommentlist/DifferentialRevisionCommentListView.php +++ b/src/applications/differential/view/revisioncommentlist/DifferentialRevisionCommentListView.php @@ -20,17 +20,29 @@ final class DifferentialRevisionCommentListView extends AphrontView { private $comments; private $handles; + private $inlines; + private $changesets; - public function setComments($comments) { + public function setComments(array $comments) { $this->comments = $comments; return $this; } + public function setInlineComments(array $inline_comments) { + $this->inlines = $inline_comments; + return $this; + } + public function setHandles(array $handles) { $this->handles = $handles; return $this; } + public function setChangesets(array $changesets) { + $this->changesets = $changesets; + return $this; + } + public function render() { require_celerity_resource('differential-revision-comment-list-css'); @@ -38,12 +50,17 @@ final class DifferentialRevisionCommentListView extends AphrontView { $factory = new DifferentialMarkupEngineFactory(); $engine = $factory->newDifferentialCommentMarkupEngine(); + $inlines = mgroup($this->inlines, 'getCommentID'); + + $comments = array(); foreach ($this->comments as $comment) { $view = new DifferentialRevisionCommentView(); $view->setComment($comment); $view->setHandles($this->handles); $view->setMarkupEngine($engine); + $view->setInlineComments(idx($inlines, $comment->getID(), array())); + $view->setChangesets($this->changesets); $comments[] = $view->render(); } diff --git a/src/applications/differential/view/revisioncommentlist/__init__.php b/src/applications/differential/view/revisioncommentlist/__init__.php index 446ab1fa36..1fe4c1067a 100644 --- a/src/applications/differential/view/revisioncommentlist/__init__.php +++ b/src/applications/differential/view/revisioncommentlist/__init__.php @@ -11,5 +11,7 @@ phutil_require_module('phabricator', 'applications/differential/view/revisioncom phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'view/base'); +phutil_require_module('phutil', 'utils'); + phutil_require_source('DifferentialRevisionCommentListView.php'); diff --git a/src/applications/phid/handle/PhabricatorObjectHandle.php b/src/applications/phid/handle/PhabricatorObjectHandle.php index eb448214c6..aee61e6ef9 100644 --- a/src/applications/phid/handle/PhabricatorObjectHandle.php +++ b/src/applications/phid/handle/PhabricatorObjectHandle.php @@ -57,7 +57,7 @@ class PhabricatorObjectHandle { $this->fullName = $full_name; return $this; } - + public function getFullName() { return $this->fullName; } diff --git a/webroot/rsrc/css/application/differential/add-comment.css b/webroot/rsrc/css/application/differential/add-comment.css index 99be1e9f7f..8ee1dce3d2 100644 --- a/webroot/rsrc/css/application/differential/add-comment.css +++ b/webroot/rsrc/css/application/differential/add-comment.css @@ -21,3 +21,10 @@ .differential-comment-preview .differential-loading-text { color: #aaaaaa; } + +#inline-comment-preview { + margin-left: 60px; + + width: 88ex; + width: 81ch; +} diff --git a/webroot/rsrc/css/application/differential/revision-comment.css b/webroot/rsrc/css/application/differential/revision-comment.css index 9a3190b425..10277bb499 100644 --- a/webroot/rsrc/css/application/differential/revision-comment.css +++ b/webroot/rsrc/css/application/differential/revision-comment.css @@ -38,7 +38,11 @@ padding: .3em 5px .4em 1.25em; } -.differential-comment-content code { +.differential-comment-core p { + margin: 0.35em 0; +} + +.differential-comment-core code { width: 88ex; width: 81ch; } @@ -92,3 +96,53 @@ border-color: #cc9966; background: #fff9f3; } + + + +.differential-inline-summary th, +.differential-inline-summary td { + vertical-align: top; + padding: 0 0 6px; +} + +.differential-inline-summary th { + padding-top: 16px; + color: #666666; + font-weight: bold; +} + +.differential-inline-summary tr > th:first-child { + padding-top: 2px; +} + + +.differential-inline-summary td.inline-line-number { + color: #444444; + white-space: nowrap; + text-align: left; + font-weight: bold; + padding: 0 4px; + width: 90px; +} + +.differential-inline-summary td.inline-line-number .num { + display: block; + position: relative; + padding-left: 15px; + padding-right: 5px; + width: 70px; /* Need lots of width for 23,950-23,951 */ +} + +.differential-inline-summary td.inline-line-number a:hover { + background: #3b5998; + color: white; + text-decoration: none; +} + + +.differential-inline-summary-section { + margin: 1em 0 .5em; + font-size: 11px; + border-bottom: 1px solid #dddddd; + color: #666666; +} diff --git a/webroot/rsrc/js/application/differential/behavior-comment-preview.js b/webroot/rsrc/js/application/differential/behavior-comment-preview.js index 41bdec070b..3644bf6727 100644 --- a/webroot/rsrc/js/application/differential/behavior-comment-preview.js +++ b/webroot/rsrc/js/application/differential/behavior-comment-preview.js @@ -47,4 +47,20 @@ JX.behavior('differential-feedback-preview', function(config) { JX.DOM.listen(action, 'change', null, check); check(); + + + function refreshInlinePreview() { + new JX.Request(config.inlineuri, function(r) { + JX.DOM.setContent(JX.$(config.inline), JX.HTML(r)); + }) + .setTimeout(5000) + .send(); + } + + JX.Stratcom.listen( + 'differential-inline-comment-update', + null, + refreshInlinePreview); + + refreshInlinePreview(); });