diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index db9604a779..2de2ee9140 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -690,22 +690,6 @@ final class DifferentialChangesetParser { return false; } - $old = $changeset->getOldProperties(); - $new = $changeset->getNewProperties(); - - if ($old === $new) { - return false; - } - - if ($changeset->getChangeType() == DifferentialChangeType::TYPE_ADD && - $new == array('unix:filemode' => '100644')) { - return false; - } - - if ($changeset->getChangeType() == DifferentialChangeType::TYPE_DELETE && - $old == array('unix:filemode' => '100644')) { - return false; - } return true; } @@ -927,6 +911,9 @@ final class DifferentialChangesetParser { } } + $renderer->attachOldFile($old); + $renderer->attachNewFile($new); + return $renderer->renderFileChange($old, $new, $id, $vs); case DifferentialChangeType::FILE_DIRECTORY: case DifferentialChangeType::FILE_BINARY: diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php index 35e1cc4e69..9d00d37c89 100644 --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php @@ -289,13 +289,23 @@ abstract class DifferentialChangesetHTMLRenderer protected function renderPropertyChangeHeader() { $changeset = $this->getChangeset(); + list($old, $new) = $this->getChangesetProperties($changeset); - $old = $changeset->getOldProperties(); - $new = $changeset->getNewProperties(); + // If we don't have any property changes, don't render this table. + if ($old === $new) { + return null; + } $keys = array_keys($old + $new); sort($keys); + $key_map = array( + 'unix:filemode' => pht('File Mode'), + 'file:dimensions' => pht('Image Dimensions'), + 'file:mimetype' => pht('MIME Type'), + 'file:size' => pht('File Size'), + ); + $rows = array(); foreach ($keys as $key) { $oval = idx($old, $key); @@ -313,8 +323,10 @@ abstract class DifferentialChangesetHTMLRenderer $nval = phutil_escape_html_newlines($nval); } + $readable_key = idx($key_map, $key, $key); + $rows[] = phutil_tag('tr', array(), array( - phutil_tag('th', array(), $key), + phutil_tag('th', array(), $readable_key), phutil_tag('td', array('class' => 'oval'), $oval), phutil_tag('td', array('class' => 'nval'), $nval), )); diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php index a7da1f80db..8327a4103e 100644 --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -30,6 +30,9 @@ abstract class DifferentialChangesetRenderer { private $depths; private $originalCharacterEncoding; + private $oldFile = false; + private $newFile = false; + public function setOriginalCharacterEncoding($original_character_encoding) { $this->originalCharacterEncoding = $original_character_encoding; return $this; @@ -63,6 +66,38 @@ abstract class DifferentialChangesetRenderer { return $this->gaps; } + public function attachOldFile(PhabricatorFile $old = null) { + $this->oldFile = $old; + return $this; + } + + public function getOldFile() { + if ($this->oldFile === false) { + throw new PhabricatorDataNotAttachedException($this); + } + return $this->oldFile; + } + + public function hasOldFile() { + return (bool)$this->oldFile; + } + + public function attachNewFile(PhabricatorFile $new = null) { + $this->newFile = $new; + return $this; + } + + public function getNewFile() { + if ($this->newFile === false) { + throw new PhabricatorDataNotAttachedException($this); + } + return $this->newFile; + } + + public function hasNewFile() { + return (bool)$this->newFile; + } + public function setOriginalNew($original_new) { $this->originalNew = $original_new; return $this; @@ -498,4 +533,43 @@ abstract class DifferentialChangesetRenderer { return $out; } + protected function getChangesetProperties($changeset) { + $old = $changeset->getOldProperties(); + $new = $changeset->getNewProperties(); + + // When adding files, don't show the uninteresting 644 filemode change. + if ($changeset->getChangeType() == DifferentialChangeType::TYPE_ADD && + $new == array('unix:filemode' => '100644')) { + unset($new['unix:filemode']); + } + + // Likewise when removing files. + if ($changeset->getChangeType() == DifferentialChangeType::TYPE_DELETE && + $old == array('unix:filemode' => '100644')) { + unset($old['unix:filemode']); + } + + if ($this->hasOldFile()) { + $file = $this->getOldFile(); + if ($file->getImageWidth()) { + $dimensions = $file->getImageWidth().'x'.$file->getImageHeight(); + $old['file:dimensions'] = $dimensions; + } + $old['file:mimetype'] = $file->getMimeType(); + $old['file:size'] = phutil_format_bytes($file->getByteSize()); + } + + if ($this->hasNewFile()) { + $file = $this->getNewFile(); + if ($file->getImageWidth()) { + $dimensions = $file->getImageWidth().'x'.$file->getImageHeight(); + $new['file:dimensions'] = $dimensions; + } + $new['file:mimetype'] = $file->getMimeType(); + $new['file:size'] = phutil_format_bytes($file->getByteSize()); + } + + return array($old, $new); + } + } diff --git a/src/applications/differential/render/DifferentialChangesetTestRenderer.php b/src/applications/differential/render/DifferentialChangesetTestRenderer.php index 9941c8a6db..05408e6b4e 100644 --- a/src/applications/differential/render/DifferentialChangesetTestRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTestRenderer.php @@ -23,8 +23,7 @@ abstract class DifferentialChangesetTestRenderer protected function renderPropertyChangeHeader() { $changeset = $this->getChangeset(); - $old = $changeset->getOldProperties(); - $new = $changeset->getNewProperties(); + list($old, $new) = $this->getChangesetProperties($changeset); if (!$old && !$new) { return null; diff --git a/src/infrastructure/storage/lisk/PhabricatorDataNotAttachedException.php b/src/infrastructure/storage/lisk/PhabricatorDataNotAttachedException.php index 7c1453b6c0..7e32428b5e 100644 --- a/src/infrastructure/storage/lisk/PhabricatorDataNotAttachedException.php +++ b/src/infrastructure/storage/lisk/PhabricatorDataNotAttachedException.php @@ -2,7 +2,7 @@ final class PhabricatorDataNotAttachedException extends Exception { - public function __construct(PhabricatorLiskDAO $dao) { + public function __construct($object) { $stack = debug_backtrace(); // Shift off `PhabricatorDataNotAttachedException::__construct()`. @@ -19,7 +19,7 @@ final class PhabricatorDataNotAttachedException extends Exception { } } - $class = get_class($dao); + $class = get_class($object); $message = "Attempting to access attached data on {$class}{$via}, but the data is ".