From 7a8b4a21ab724c4e2d87a420052a9d31e155655f Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 4 Mar 2013 18:07:47 -0800 Subject: [PATCH] Use transaction diffs to show description changes in Pholio Summary: Fixes T2659. These didn't exist until recently. Test Plan: {F34556} Reviewers: chad Reviewed By: chad CC: aran Maniphest Tasks: T2659 Differential Revision: https://secure.phabricator.com/D5221 --- .../storage/PhabricatorConfigTransaction.php | 6 ++--- .../pholio/storage/PholioTransaction.php | 26 ++++++++++++++++--- ...plicationTransactionTextDiffDetailView.php | 13 +++++++--- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/applications/config/storage/PhabricatorConfigTransaction.php b/src/applications/config/storage/PhabricatorConfigTransaction.php index a0428a73d7..a8932f1ec5 100644 --- a/src/applications/config/storage/PhabricatorConfigTransaction.php +++ b/src/applications/config/storage/PhabricatorConfigTransaction.php @@ -85,15 +85,13 @@ final class PhabricatorConfigTransaction if ($old['deleted']) { $old_text = ''; } else { - // NOTE: Here and below, we're adding a synthetic "\n" to prevent the - // differ from complaining about missing trailing newlines. - $old_text = PhabricatorConfigJSON::prettyPrintJSON($old['value'])."\n"; + $old_text = PhabricatorConfigJSON::prettyPrintJSON($old['value']); } if ($new['deleted']) { $new_text = ''; } else { - $new_text = PhabricatorConfigJSON::prettyPrintJSON($new['value'])."\n"; + $new_text = PhabricatorConfigJSON::prettyPrintJSON($new['value']); } $view = id(new PhabricatorApplicationTransactionTextDiffDetailView()) diff --git a/src/applications/pholio/storage/PholioTransaction.php b/src/applications/pholio/storage/PholioTransaction.php index f61390b12f..ed2a9493e1 100644 --- a/src/applications/pholio/storage/PholioTransaction.php +++ b/src/applications/pholio/storage/PholioTransaction.php @@ -49,10 +49,8 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { break; case PholioTransactionType::TYPE_DESCRIPTION: return pht( - '%s updated the description of this mock. '. - 'The old description was: %s', - $this->renderHandleLink($author_phid), - $old); + "%s updated the mock's description.", + $this->renderHandleLink($author_phid)); break; case PholioTransactionType::TYPE_INLINE: return pht( @@ -63,5 +61,25 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { return parent::getTitle(); } + public function hasChangeDetails() { + switch ($this->getTransactionType()) { + case PholioTransactionType::TYPE_DESCRIPTION: + return true; + } + return parent::hasChangeDetails(); + } + + public function renderChangeDetails(PhabricatorUser $viewer) { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $view = id(new PhabricatorApplicationTransactionTextDiffDetailView()) + ->setUser($viewer) + ->setOldText($old) + ->setNewText($new); + + return $view->render(); + } + } diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php index ac062125df..2546d38a58 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php @@ -23,10 +23,15 @@ final class PhabricatorApplicationTransactionTextDiffDetailView // TODO: On mobile, or perhaps by default, we should switch to 1-up once // that is built. - $old = phutil_utf8_hard_wrap($old, 80); - $old = implode("\n", $old); - $new = phutil_utf8_hard_wrap($new, 80); - $new = implode("\n", $new); + if (strlen($old)) { + $old = phutil_utf8_hard_wrap($old, 80); + $old = implode("\n", $old)."\n"; + } + + if (strlen($new)) { + $new = phutil_utf8_hard_wrap($new, 80); + $new = implode("\n", $new)."\n"; + } $engine = new PhabricatorDifferenceEngine(); $changeset = $engine->generateChangesetFromFileContent($old, $new);