Index: src/__phutil_library_map__.php =================================================================== --- src/__phutil_library_map__.php +++ src/__phutil_library_map__.php @@ -428,6 +428,7 @@ 'DifferentialLandingToHostedGit' => 'applications/differential/landing/DifferentialLandingToHostedGit.php', 'DifferentialLandingToHostedMercurial' => 'applications/differential/landing/DifferentialLandingToHostedMercurial.php', 'DifferentialLinesFieldSpecification' => 'applications/differential/field/specification/DifferentialLinesFieldSpecification.php', + 'DifferentialLintField' => 'applications/differential/customfield/DifferentialLintField.php', 'DifferentialLintFieldSpecification' => 'applications/differential/field/specification/DifferentialLintFieldSpecification.php', 'DifferentialLintStatus' => 'applications/differential/constants/DifferentialLintStatus.php', 'DifferentialLocalCommitsView' => 'applications/differential/view/DifferentialLocalCommitsView.php', @@ -464,7 +465,6 @@ 'DifferentialReviewersView' => 'applications/differential/view/DifferentialReviewersView.php', 'DifferentialRevision' => 'applications/differential/storage/DifferentialRevision.php', 'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php', - 'DifferentialRevisionDetailRenderer' => 'applications/differential/controller/DifferentialRevisionDetailRenderer.php', 'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php', 'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php', 'DifferentialRevisionEditControllerPro' => 'applications/differential/controller/DifferentialRevisionEditControllerPro.php', @@ -496,6 +496,7 @@ 'DifferentialTransactionEditor' => 'applications/differential/editor/DifferentialTransactionEditor.php', 'DifferentialTransactionQuery' => 'applications/differential/query/DifferentialTransactionQuery.php', 'DifferentialTransactionView' => 'applications/differential/view/DifferentialTransactionView.php', + 'DifferentialUnitField' => 'applications/differential/customfield/DifferentialUnitField.php', 'DifferentialUnitFieldSpecification' => 'applications/differential/field/specification/DifferentialUnitFieldSpecification.php', 'DifferentialUnitStatus' => 'applications/differential/constants/DifferentialUnitStatus.php', 'DifferentialUnitTestResult' => 'applications/differential/constants/DifferentialUnitTestResult.php', @@ -2507,7 +2508,6 @@ 'ReleephDiffChurnFieldSpecification' => 'applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php', 'ReleephDiffMessageFieldSpecification' => 'applications/releeph/field/specification/ReleephDiffMessageFieldSpecification.php', 'ReleephDiffSizeFieldSpecification' => 'applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php', - 'ReleephDifferentialRevisionDetailRenderer' => 'applications/releeph/differential/ReleephDifferentialRevisionDetailRenderer.php', 'ReleephFieldParseException' => 'applications/releeph/field/exception/ReleephFieldParseException.php', 'ReleephFieldSelector' => 'applications/releeph/field/selector/ReleephFieldSelector.php', 'ReleephFieldSpecification' => 'applications/releeph/field/specification/ReleephFieldSpecification.php', @@ -2993,6 +2993,7 @@ 'DifferentialLandingToHostedGit' => 'DifferentialLandingStrategy', 'DifferentialLandingToHostedMercurial' => 'DifferentialLandingStrategy', 'DifferentialLinesFieldSpecification' => 'DifferentialFieldSpecification', + 'DifferentialLintField' => 'DifferentialCustomField', 'DifferentialLintFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialLocalCommitsView' => 'AphrontView', 'DifferentialMail' => 'PhabricatorMail', @@ -3066,6 +3067,7 @@ 'DifferentialTransactionEditor' => 'PhabricatorApplicationTransactionEditor', 'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView', + 'DifferentialUnitField' => 'DifferentialCustomField', 'DifferentialUnitFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialViewPolicyField' => 'DifferentialCoreCustomField', 'DifferentialViewPolicyFieldSpecification' => 'DifferentialFieldSpecification', Index: src/applications/differential/controller/DifferentialRevisionViewController.php =================================================================== --- src/applications/differential/controller/DifferentialRevisionViewController.php +++ src/applications/differential/controller/DifferentialRevisionViewController.php @@ -174,6 +174,28 @@ $field_list->setViewer($user); $field_list->readFieldsFromStorage($revision); + + // TODO: This should be in a DiffQuery or similar. + $need_props = array(); + foreach ($field_list->getFields() as $field) { + foreach ($field->getRequiredDiffPropertiesForRevisionView() as $prop) { + $need_props[$prop] = $prop; + } + } + + if ($need_props) { + $prop_diff = $revision->getActiveDiff(); + $load_props = id(new DifferentialDiffProperty())->loadAllWhere( + 'diffID = %d AND name IN (%Ls)', + $prop_diff->getID(), + $need_props); + $load_props = mpull($load_props, 'getData', 'getName'); + foreach ($need_props as $need) { + $prop_diff->attachProperty($need, idx($load_props, $need)); + } + } + + $revision_detail = id(new DifferentialRevisionDetailView()) ->setUser($user) ->setRevision($revision) Index: src/applications/differential/customfield/DifferentialCustomField.php =================================================================== --- src/applications/differential/customfield/DifferentialCustomField.php +++ src/applications/differential/customfield/DifferentialCustomField.php @@ -3,6 +3,10 @@ abstract class DifferentialCustomField extends PhabricatorCustomField { + public function getRequiredDiffPropertiesForRevisionView() { + return array(); + } + protected function renderHandleList(array $handles) { if (!$handles) { return null; Index: src/applications/differential/customfield/DifferentialLintField.php =================================================================== --- /dev/null +++ src/applications/differential/customfield/DifferentialLintField.php @@ -0,0 +1,236 @@ +getFieldName(); + } + + public function getRequiredDiffPropertiesForRevisionView() { + return array( + 'arc:lint', + 'arc:lint-excuse', + 'arc:lint-postponed', + ); + } + + public function renderPropertyViewValue(array $handles) { + $diff = $this->getObject()->getActiveDiff(); + + $path_changesets = mpull($diff->loadChangesets(), 'getID', 'getFilename'); + + $lstar = DifferentialRevisionUpdateHistoryView::renderDiffLintStar($diff); + $lmsg = DifferentialRevisionUpdateHistoryView::getDiffLintMessage($diff); + $ldata = $diff->getProperty('arc:lint'); + $ltail = null; + + $rows = array(); + + $rows[] = array( + 'style' => 'star', + 'name' => $lstar, + 'value' => $lmsg, + 'show' => true, + ); + + $excuse = $diff->getProperty('arc:lint-excuse'); + if ($excuse) { + $rows[] = array( + 'style' => 'excuse', + 'name' => 'Excuse', + 'value' => phutil_escape_html_newlines($excuse), + 'show' => true, + ); + } + + $show_limit = 10; + $hidden = array(); + + if ($ldata) { + $ldata = igroup($ldata, 'path'); + foreach ($ldata as $path => $messages) { + + $rows[] = array( + 'style' => 'section', + 'name' => $path, + 'show' => $show_limit, + ); + + foreach ($messages as $message) { + $path = idx($message, 'path'); + $line = idx($message, 'line'); + + $code = idx($message, 'code'); + $severity = idx($message, 'severity'); + + $name = idx($message, 'name'); + $description = idx($message, 'description'); + + $line_link = 'line '.intval($line); + if (isset($path_changesets[$path])) { + $href = '#C'.$path_changesets[$path].'NL'.max(1, $line); + + // TODO: We are always showing the active diff + // if ($diff->getID() != $this->getDiff()->getID()) { + // $href = '/D'.$diff->getRevisionID().'?id='.$diff->getID().$href; + // } + + $line_link = phutil_tag( + 'a', + array( + 'href' => $href, + ), + $line_link); + } + + if ($show_limit) { + --$show_limit; + $show = true; + } else { + $show = false; + if (empty($hidden[$severity])) { + $hidden[$severity] = 0; + } + $hidden[$severity]++; + } + + $rows[] = array( + 'style' => $this->getSeverityStyle($severity), + 'name' => ucwords($severity), + 'value' => hsprintf( + '(%s) %s at %s', + $code, + $name, + $line_link), + 'show' => $show, + ); + + if (!empty($message['locations'])) { + $locations = array(); + foreach ($message['locations'] as $location) { + $other_line = idx($location, 'line'); + $locations[] = + idx($location, 'path', $path). + ($other_line ? ":{$other_line}" : ""); + } + $description .= "\nOther locations: ".implode(", ", $locations); + } + + if (strlen($description)) { + $rows[] = array( + 'style' => 'details', + 'value' => phutil_escape_html_newlines($description), + 'show' => false, + ); + if (empty($hidden['details'])) { + $hidden['details'] = 0; + } + $hidden['details']++; + } + } + } + } + + $postponed = $diff->getProperty('arc:lint-postponed'); + if ($postponed) { + foreach ($postponed as $linter) { + $rows[] = array( + 'style' => $this->getPostponedStyle(), + 'name' => 'Postponed', + 'value' => $linter, + 'show' => false, + ); + if (empty($hidden['postponed'])) { + $hidden['postponed'] = 0; + } + $hidden['postponed']++; + } + } + + $show_string = $this->renderShowString($hidden); + + $view = new DifferentialResultsTableView(); + $view->setRows($rows); + $view->setShowMoreString($show_string); + + return $view->render(); + } + + private function getSeverityStyle($severity) { + $map = array( + ArcanistLintSeverity::SEVERITY_ERROR => 'red', + ArcanistLintSeverity::SEVERITY_WARNING => 'yellow', + ArcanistLintSeverity::SEVERITY_AUTOFIX => 'yellow', + ArcanistLintSeverity::SEVERITY_ADVICE => 'yellow', + ); + return idx($map, $severity); + } + + private function getPostponedStyle() { + return 'blue'; + } + + private function renderShowString(array $hidden) { + if (!$hidden) { + return null; + } + + // Reorder hidden things by severity. + $hidden = array_select_keys( + $hidden, + array( + ArcanistLintSeverity::SEVERITY_ERROR, + ArcanistLintSeverity::SEVERITY_WARNING, + ArcanistLintSeverity::SEVERITY_AUTOFIX, + ArcanistLintSeverity::SEVERITY_ADVICE, + 'details', + 'postponed', + )) + $hidden; + + $show = array(); + foreach ($hidden as $key => $value) { + switch ($key) { + case ArcanistLintSeverity::SEVERITY_ERROR: + $show[] = pht('%d Error(s)', $value); + break; + case ArcanistLintSeverity::SEVERITY_WARNING: + $show[] = pht('%d Warning(s)', $value); + break; + case ArcanistLintSeverity::SEVERITY_AUTOFIX: + $show[] = pht('%d Auto-Fix(es)', $value); + break; + case ArcanistLintSeverity::SEVERITY_ADVICE: + $show[] = pht('%d Advice(s)', $value); + break; + case 'details': + $show[] = pht('%d Detail(s)', $value); + break; + case 'postponed': + $show[] = pht('%d Postponed', $value); + break; + default: + $show[] = $value; + break; + } + } + + return "Show Full Lint Results (".implode(', ', $show).")"; + } +} Index: src/applications/differential/customfield/DifferentialUnitField.php =================================================================== --- /dev/null +++ src/applications/differential/customfield/DifferentialUnitField.php @@ -0,0 +1,206 @@ +getFieldName(); + } + + public function getRequiredDiffPropertiesForRevisionView() { + return array( + 'arc:unit', + 'arc:unit-excuse', + ); + } + + public function renderPropertyViewValue(array $handles) { + $diff = $this->getObject()->getActiveDiff(); + + $ustar = DifferentialRevisionUpdateHistoryView::renderDiffUnitStar($diff); + $umsg = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff); + + $rows = array(); + + $rows[] = array( + 'style' => 'star', + 'name' => $ustar, + 'value' => $umsg, + 'show' => true, + ); + + $excuse = $diff->getProperty('arc:unit-excuse'); + if ($excuse) { + $rows[] = array( + 'style' => 'excuse', + 'name' => 'Excuse', + 'value' => phutil_escape_html_newlines($excuse), + 'show' => true, + ); + } + + $show_limit = 10; + $hidden = array(); + + $udata = $diff->getProperty('arc:unit'); + if ($udata) { + $sort_map = array( + ArcanistUnitTestResult::RESULT_BROKEN => 0, + ArcanistUnitTestResult::RESULT_FAIL => 1, + ArcanistUnitTestResult::RESULT_UNSOUND => 2, + ArcanistUnitTestResult::RESULT_SKIP => 3, + ArcanistUnitTestResult::RESULT_POSTPONED => 4, + ArcanistUnitTestResult::RESULT_PASS => 5, + ); + + foreach ($udata as $key => $test) { + $udata[$key]['sort'] = idx($sort_map, idx($test, 'result')); + } + $udata = isort($udata, 'sort'); + $engine = new PhabricatorMarkupEngine(); + $engine->setViewer($this->getViewer()); + $markup_objects = array(); + foreach ($udata as $key => $test) { + $userdata = idx($test, 'userdata'); + if ($userdata) { + if ($userdata !== false) { + $userdata = str_replace("\000", '', $userdata); + } + $markup_object = id(new PhabricatorMarkupOneOff()) + ->setContent($userdata) + ->setPreserveLinebreaks(true); + $engine->addObject($markup_object, 'default'); + $markup_objects[$key] = $markup_object; + } + } + $engine->process(); + foreach ($udata as $key => $test) { + $result = idx($test, 'result'); + + $default_hide = false; + switch ($result) { + case ArcanistUnitTestResult::RESULT_POSTPONED: + case ArcanistUnitTestResult::RESULT_PASS: + $default_hide = true; + break; + } + + if ($show_limit && !$default_hide) { + --$show_limit; + $show = true; + } else { + $show = false; + if (empty($hidden[$result])) { + $hidden[$result] = 0; + } + $hidden[$result]++; + } + + $value = idx($test, 'name'); + if (!empty($test['link'])) { + $value = phutil_tag( + 'a', + array( + 'href' => $test['link'], + 'target' => '_blank', + ), + $value); + } + $rows[] = array( + 'style' => $this->getResultStyle($result), + 'name' => ucwords($result), + 'value' => $value, + 'show' => $show, + ); + + if (isset($markup_objects[$key])) { + $rows[] = array( + 'style' => 'details', + 'value' => $engine->getOutput($markup_objects[$key], 'default'), + 'show' => false, + ); + if (empty($hidden['details'])) { + $hidden['details'] = 0; + } + $hidden['details']++; + } + } + } + + $show_string = $this->renderShowString($hidden); + + $view = new DifferentialResultsTableView(); + $view->setRows($rows); + $view->setShowMoreString($show_string); + + return $view->render(); + } + + private function getResultStyle($result) { + $map = array( + ArcanistUnitTestResult::RESULT_PASS => 'green', + ArcanistUnitTestResult::RESULT_FAIL => 'red', + ArcanistUnitTestResult::RESULT_SKIP => 'blue', + ArcanistUnitTestResult::RESULT_BROKEN => 'red', + ArcanistUnitTestResult::RESULT_UNSOUND => 'yellow', + ArcanistUnitTestResult::RESULT_POSTPONED => 'blue', + ); + return idx($map, $result); + } + + private function renderShowString(array $hidden) { + if (!$hidden) { + return null; + } + + // Reorder hidden things by severity. + $hidden = array_select_keys( + $hidden, + array( + ArcanistUnitTestResult::RESULT_BROKEN, + ArcanistUnitTestResult::RESULT_FAIL, + ArcanistUnitTestResult::RESULT_UNSOUND, + ArcanistUnitTestResult::RESULT_SKIP, + ArcanistUnitTestResult::RESULT_POSTPONED, + ArcanistUnitTestResult::RESULT_PASS, + 'details', + )) + $hidden; + + $noun = array( + ArcanistUnitTestResult::RESULT_BROKEN => 'Broken', + ArcanistUnitTestResult::RESULT_FAIL => 'Failed', + ArcanistUnitTestResult::RESULT_UNSOUND => 'Unsound', + ArcanistUnitTestResult::RESULT_SKIP => 'Skipped', + ArcanistUnitTestResult::RESULT_POSTPONED => 'Postponed', + ArcanistUnitTestResult::RESULT_PASS => 'Passed', + ); + + $show = array(); + foreach ($hidden as $key => $value) { + if ($key == 'details') { + $show[] = pht('%d Detail(s)', $value); + } else { + $show[] = $value.' '.idx($noun, $key); + } + } + + return "Show Full Unit Results (".implode(', ', $show).")"; + } + +} Index: src/applications/differential/storage/DifferentialDiff.php =================================================================== --- src/applications/differential/storage/DifferentialDiff.php +++ src/applications/differential/storage/DifferentialDiff.php @@ -35,6 +35,7 @@ private $changesets = self::ATTACHABLE; private $arcanistProject = self::ATTACHABLE; private $revision = self::ATTACHABLE; + private $properties = array(); public function getConfiguration() { return array( @@ -306,6 +307,15 @@ return $this; } + public function attachProperty($key, $value) { + $this->properties[$key] = $value; + return $this; + } + + public function getProperty($key) { + return $this->assertAttachedKey($this->properties, $key); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */