From c5c31d7eb9ef9489edb1d0a92ea7c90ac2cfbc18 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 19 Feb 2011 15:06:22 -0800 Subject: [PATCH] Make auto-ccs work, delete some dead code, make inlines show up in email, and resolve a display caching issue. Summary: Test Plan: Reviewers: CC: --- .../comment/DifferentialCommentEditor.php | 49 ++++++---------- .../differential/editor/comment/__init__.php | 1 + .../revision/DifferentialRevisionEditor.php | 57 ------------------- .../mail/comment/DifferentialCommentMail.php | 10 +++- .../DifferentialRevisionCommentView.php | 19 ++++--- 5 files changed, 38 insertions(+), 98 deletions(-) diff --git a/src/applications/differential/editor/comment/DifferentialCommentEditor.php b/src/applications/differential/editor/comment/DifferentialCommentEditor.php index 38826563f3..8f741d577a 100755 --- a/src/applications/differential/editor/comment/DifferentialCommentEditor.php +++ b/src/applications/differential/editor/comment/DifferentialCommentEditor.php @@ -238,7 +238,14 @@ class DifferentialCommentEditor { throw new Exception('Unsupported action.'); } - // Reload relationships to pick up any reviewer changes. + if ($this->addCC) { + DifferentialRevisionEditor::addCC( + $revision, + $this->actorPHID, + $this->actorPHID); + } + + // Reload relationships to pick up any reviewer/CC changes. $revision->loadRelationships(); $inline_comments = array(); @@ -256,24 +263,15 @@ class DifferentialCommentEditor { ->setContent((string)$this->message) ->save(); -// $diff = id(new Diff())->loadActiveWithRevision($revision); -// $changesets = id(new DifferentialChangeset())->loadAllWithDiff($diff); - + $changesets = array(); 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'); - $load = array(); - foreach ($inline_changeset_ids as $id) { - if (empty($changesets[$id])) { - $load[] = $id; - } + $load_ids = mpull($inline_comments, 'getChangesetID'); + if ($load_ids) { + $load_ids = array_unique($load_ids); + $changesets = id(new DifferentialChangeset())->loadAllWhere( + 'id in (%Ld)', + $load_ids); } - if ($load) { - $changesets += id(new DifferentialChangeset())->loadAllWithIDs($load); - } -*/ foreach ($inline_comments as $inline) { $inline->setCommentID($comment->getID()); $inline->save(); @@ -289,8 +287,8 @@ class DifferentialCommentEditor { $revision, $actor_handle, $comment, - /* $changesets TODO */ array(), - /* $inline_comments TODO */ array())) + $changesets, + $inline_comments)) ->setToPHIDs( array_merge( $revision->getReviewers(), @@ -299,19 +297,6 @@ class DifferentialCommentEditor { ->setChangedByCommit($this->getChangedByCommit()) ->send(); -/* - - tODO - - if ($this->addCC) { - require_module_lazy('site/tools/differential/lib/editor/revision'); - DifferentialRevisionEditor::addCCFBID( - $revision, - $this->actorPHID, - $this->actorPHID); - } -*/ - /* TODO diff --git a/src/applications/differential/editor/comment/__init__.php b/src/applications/differential/editor/comment/__init__.php index 11fbcdc6ba..2412349c94 100644 --- a/src/applications/differential/editor/comment/__init__.php +++ b/src/applications/differential/editor/comment/__init__.php @@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/action phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/editor/revision'); phutil_require_module('phabricator', 'applications/differential/mail/comment'); +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/phid/handle/data'); diff --git a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php index a68fc3ae4d..73bf1999ac 100644 --- a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php +++ b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php @@ -72,63 +72,6 @@ class DifferentialRevisionEditor { $this->setCCPHIDs($fields['ccPHIDs']); } - -/* - public static function newRevisionFromRawMessageWithDiff( - DifferentialRawMessage $message, - Diff $diff, - $user) { - - if ($message->getRevisionID()) { - throw new Exception( - "The provided commit message is already associated with a ". - "Differential revision."); - } - - if ($message->getReviewedByNames()) { - throw new Exception( - "The provided commit message contains a 'Reviewed By:' field."); - } - - $revision = new DifferentialRevision(); - $revision->setPHID($revision->generatePHID()); - - $revision->setOwnerID($user); - $revision->setStatus(DifferentialRevisionStatus::NEEDS_REVIEW); - $revision->attachReviewers(array()); - $revision->attachCCPHIDs(array()); - - $editor = new DifferentialRevisionEditor($revision, $user); - - self::copyFields($editor, $revision, $message, $user); - - $editor->addDiff($diff, null); - $editor->save(); - - return $revision; - } - - - public static function copyFields( - DifferentialRevisionEditor $editor, - DifferentialRevision $revision, - DifferentialRawMessage $message, - $user) { - - $revision->setName($message->getTitle()); - $revision->setSummary($message->getSummary()); - $revision->setTestPlan($message->getTestPlan()); - $revision->setSVNBlameRevision($message->getBlameRevision()); - $revision->setRevert($message->getRevertPlan()); - $revision->setPlatformImpact($message->getPlatformImpact()); - $revision->setBugzillaID($message->getBugzillaID()); - - $editor->setReviewers($message->getReviewerPHIDs()); - $editor->setCCPHIDs($message->getCCPHIDs()); - } - -*/ - public function getRevision() { return $this->revision; } diff --git a/src/applications/differential/mail/comment/DifferentialCommentMail.php b/src/applications/differential/mail/comment/DifferentialCommentMail.php index a206a7aab4..6df4468719 100755 --- a/src/applications/differential/mail/comment/DifferentialCommentMail.php +++ b/src/applications/differential/mail/comment/DifferentialCommentMail.php @@ -92,9 +92,15 @@ class DifferentialCommentMail extends DifferentialMail { throw new Exception('Changeset missing!'); } $file = $changeset->getFilename(); - $line = $inline->renderLineRange(); + $start = $inline->getLineNumber(); + $len = $inline->getLineLength(); + if ($len) { + $range = $start.'-'.($start + $len); + } else { + $range = $start; + } $content = $inline->getContent(); - $body[] = $this->formatText("{$file}:{$line} {$content}"); + $body[] = $this->formatText("{$file}:{$range} {$content}"); } $body[] = null; } diff --git a/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php b/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php index 58a4460065..1aeebbaac8 100644 --- a/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php +++ b/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php @@ -138,19 +138,24 @@ final class DifferentialRevisionCommentView extends AphrontView { ), $lines); - $content = $inline->getCache(); - if (!strlen($content)) { - $content = $this->markupEngine->markupText($content); - if ($inline->getID()) { - $inline->setCache($content); - $inline->save(); + $inline_content = $inline->getContent(); + if (strlen($inline_content)) { + $inline_cache = $inline->getCache(); + if ($inline_cache) { + $inline_content = $inline_cache; + } else { + $inline_content = $this->markupEngine->markupText($content); + if ($inline->getID()) { + $inline->setCache($inline_content); + $inline->save(); + } } } $inline_render[] = ''. ''.$lines.''. - ''.$content.''. + ''.$inline_content.''. ''; } }