Add a view option to disable blame in Diffusion and fix some view transition bugs
Summary: See PHI604. Ref T13130. Ref T13105. There's currently no way to turn blame off in Diffusion. Add a "Hide Blame" option to the "View Options" dropdown so it can be toggled off. Also fix a couple of bugs around this: for example, if you loaded a Jupyter notebook and then switched to "Source" view, blame would incorrectly fail to activate because the original rendering of the "stage" used an asynchronous engine so `willRenderRef()` wasn't called to populate blame. Test Plan: - Viewed a source file, toggled blame off/on, reloaded page to see state stick in URL. - Viewed a Jupyter notebook, toggled to "Source" view, saw blame. - Viewed stuff in Files (no blame UI options). - Tried to do some invalid stuff like toggle blame on a non-blame engine (options disable properly). Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13130, T13105 Differential Revision: https://secure.phabricator.com/D19414
This commit is contained in:
@@ -69,7 +69,7 @@ final class DiffusionDocumentRenderingEngine
|
||||
return;
|
||||
}
|
||||
|
||||
protected function willRenderRef(PhabricatorDocumentRef $ref) {
|
||||
protected function willStageRef(PhabricatorDocumentRef $ref) {
|
||||
$drequest = $this->getDiffusionRequest();
|
||||
|
||||
$blame_uri = (string)$drequest->generateURI(
|
||||
@@ -78,9 +78,13 @@ final class DiffusionDocumentRenderingEngine
|
||||
'stable' => true,
|
||||
));
|
||||
|
||||
$ref
|
||||
->setSymbolMetadata($this->getSymbolMetadata())
|
||||
->setBlameURI($blame_uri);
|
||||
$ref->setBlameURI($blame_uri);
|
||||
}
|
||||
|
||||
protected function willRenderRef(PhabricatorDocumentRef $ref) {
|
||||
$drequest = $this->getDiffusionRequest();
|
||||
|
||||
$ref->setSymbolMetadata($this->getSymbolMetadata());
|
||||
|
||||
$coverage = $drequest->loadCoverage();
|
||||
if (strlen($coverage)) {
|
||||
|
||||
@@ -7,6 +7,7 @@ abstract class PhabricatorDocumentEngine
|
||||
private $highlightedLines = array();
|
||||
private $encodingConfiguration;
|
||||
private $highlightingConfiguration;
|
||||
private $blameConfiguration = true;
|
||||
|
||||
final public function setViewer(PhabricatorUser $viewer) {
|
||||
$this->viewer = $viewer;
|
||||
@@ -60,6 +61,19 @@ abstract class PhabricatorDocumentEngine
|
||||
return $this->highlightingConfiguration;
|
||||
}
|
||||
|
||||
final public function setBlameConfiguration($blame_configuration) {
|
||||
$this->blameConfiguration = $blame_configuration;
|
||||
return $this;
|
||||
}
|
||||
|
||||
final public function getBlameConfiguration() {
|
||||
return $this->blameConfiguration;
|
||||
}
|
||||
|
||||
final protected function getBlameEnabled() {
|
||||
return $this->blameConfiguration;
|
||||
}
|
||||
|
||||
public function shouldRenderAsync(PhabricatorDocumentRef $ref) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -50,7 +50,7 @@ final class PhabricatorSourceDocumentEngine
|
||||
}
|
||||
|
||||
$options = array();
|
||||
if ($ref->getBlameURI()) {
|
||||
if ($ref->getBlameURI() && $this->getBlameEnabled()) {
|
||||
$content = phutil_split_lines($content);
|
||||
$blame = range(1, count($content));
|
||||
$blame = array_fuse($blame);
|
||||
|
||||
@@ -69,6 +69,9 @@ abstract class PhabricatorDocumentRenderingEngine
|
||||
$engine->setHighlightingConfiguration($highlight_setting);
|
||||
}
|
||||
|
||||
$blame_setting = ($request->getStr('blame') !== 'off');
|
||||
$engine->setBlameConfiguration($blame_setting);
|
||||
|
||||
$views = array();
|
||||
foreach ($engines as $candidate_key => $candidate_engine) {
|
||||
$label = $candidate_engine->getViewAsLabel($ref);
|
||||
@@ -104,6 +107,8 @@ abstract class PhabricatorDocumentRenderingEngine
|
||||
'controlID' => $control_id,
|
||||
);
|
||||
|
||||
$this->willStageRef($ref);
|
||||
|
||||
if ($engine->shouldRenderAsync($ref)) {
|
||||
$content = $engine->newLoadingContent($ref);
|
||||
$config['next'] = 'render';
|
||||
@@ -142,7 +147,11 @@ abstract class PhabricatorDocumentRenderingEngine
|
||||
'value' => $highlight_setting,
|
||||
),
|
||||
'blame' => array(
|
||||
'icon' => 'fa-backward',
|
||||
'hide' => pht('Hide Blame'),
|
||||
'show' => pht('Show Blame'),
|
||||
'uri' => $ref->getBlameURI(),
|
||||
'enabled' => $blame_setting,
|
||||
'value' => null,
|
||||
),
|
||||
'coverage' => array(
|
||||
@@ -180,6 +189,7 @@ abstract class PhabricatorDocumentRenderingEngine
|
||||
}
|
||||
|
||||
final public function newRenderResponse(PhabricatorDocumentRef $ref) {
|
||||
$this->willStageRef($ref);
|
||||
$this->willRenderRef($ref);
|
||||
|
||||
$request = $this->getRequest();
|
||||
@@ -207,6 +217,9 @@ abstract class PhabricatorDocumentRenderingEngine
|
||||
$engine->setHighlightingConfiguration($highlight_setting);
|
||||
}
|
||||
|
||||
$blame_setting = ($request->getStr('blame') !== 'off');
|
||||
$engine->setBlameConfiguration($blame_setting);
|
||||
|
||||
try {
|
||||
$content = $engine->newDocument($ref);
|
||||
} catch (Exception $ex) {
|
||||
@@ -314,6 +327,10 @@ abstract class PhabricatorDocumentRenderingEngine
|
||||
return;
|
||||
}
|
||||
|
||||
protected function willStageRef(PhabricatorDocumentRef $ref) {
|
||||
return;
|
||||
}
|
||||
|
||||
protected function willRenderRef(PhabricatorDocumentRef $ref) {
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user