From 3d3fff4991ab921853fd0d1fc96f8b1e65b962d7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 22 May 2016 09:54:41 -0700 Subject: [PATCH] Fix weird remarkup linewrapping on a few instructions forms, plus move toward fixing Phame/CORGI remarkup issues Summary: Fixes T10381. When we converted to `PHUIRemarkupView`, some instructional text got linebreaks added when it shouldn't have them (the source is written in PHP and wrapped at 80 characters, but the output should flow naturally). Fix this so we don't preserve linebreaks. This also makes `PHUIRemarkupView` a little more powerful and inches us toward fixing Phame/CORGI remarkup issues, getting rid of `PhabricatorMarkupInterface` / `PhabricatorMarkupOneOff`, and dropping all the application hard-coding in `PhabricatorMarkupEngine`. Test Plan: - Grepped for all callsites, looking for callsites which accept remarkup written in `<<setRemarkupOptions( + array( + PHUIRemarkupView::OPTION_PRESERVE_LINEBREAKS => false, + )); + return id(new PHUIBoxView()) ->appendChild($view) ->addPadding(PHUI::PADDING_LARGE); diff --git a/src/applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php b/src/applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php index b0abe516de..d21e2105fb 100644 --- a/src/applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php +++ b/src/applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php @@ -313,7 +313,14 @@ EOTEXT protected function renderInstructions($corpus) { $viewer = $this->getUser(); - return new PHUIRemarkupView($viewer, $corpus); + $view = new PHUIRemarkupView($viewer, $corpus); + + $view->setRemarkupOptions( + array( + PHUIRemarkupView::OPTION_PRESERVE_LINEBREAKS => false, + )); + + return $view; } } diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index 92e027f86d..dc3809f873 100644 --- a/src/infrastructure/markup/PhabricatorMarkupEngine.php +++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php @@ -470,6 +470,7 @@ final class PhabricatorMarkupEngine extends Phobject { $engine = new PhutilRemarkupEngine(); $engine->setConfig('preserve-linebreaks', $options['preserve-linebreaks']); + $engine->setConfig('pygments.enabled', $options['pygments']); $engine->setConfig( 'uri.allowed-protocols', diff --git a/src/infrastructure/markup/PhabricatorMarkupOneOff.php b/src/infrastructure/markup/PhabricatorMarkupOneOff.php index 0bfba1722f..6bffcf7e2b 100644 --- a/src/infrastructure/markup/PhabricatorMarkupOneOff.php +++ b/src/infrastructure/markup/PhabricatorMarkupOneOff.php @@ -10,6 +10,7 @@ final class PhabricatorMarkupOneOff private $content; private $preserveLinebreaks; private $engineRuleset; + private $engine; private $disableCache; public function setEngineRuleset($engine_ruleset) { @@ -35,6 +36,15 @@ final class PhabricatorMarkupOneOff return $this->content; } + public function setEngine(PhutilMarkupEngine $engine) { + $this->engine = $engine; + return $this; + } + + public function getEngine() { + return $this->engine; + } + public function setDisableCache($disable_cache) { $this->disableCache = $disable_cache; return $this; @@ -49,6 +59,10 @@ final class PhabricatorMarkupOneOff } public function newMarkupEngine($field) { + if ($this->engine) { + return $this->engine; + } + if ($this->engineRuleset) { return PhabricatorMarkupEngine::getEngine($this->engineRuleset); } else if ($this->preserveLinebreaks) { diff --git a/src/infrastructure/markup/view/PHUIRemarkupView.php b/src/infrastructure/markup/view/PHUIRemarkupView.php index 0a07e64a87..e30c09ce7c 100644 --- a/src/infrastructure/markup/view/PHUIRemarkupView.php +++ b/src/infrastructure/markup/view/PHUIRemarkupView.php @@ -12,21 +12,19 @@ final class PHUIRemarkupView extends AphrontView { private $corpus; - private $markupType; private $contextObject; + private $options; - const DOCUMENT = 'document'; + // TODO: In the long run, rules themselves should define available options. + // For now, just define constants here so we can more easily replace things + // later once this is cleaned up. + const OPTION_PRESERVE_LINEBREAKS = 'preserve-linebreaks'; public function __construct(PhabricatorUser $viewer, $corpus) { $this->setUser($viewer); $this->corpus = $corpus; } - private function setMarkupType($type) { - $this->markupType($type); - return $this; - } - public function setContextObject($context_object) { $this->contextObject = $context_object; return $this; @@ -36,29 +34,63 @@ final class PHUIRemarkupView extends AphrontView { return $this->contextObject; } + public function setRemarkupOption($key, $value) { + $this->options[$key] = $value; + return $this; + } + + public function setRemarkupOptions(array $options) { + foreach ($options as $key => $value) { + $this->setRemarkupOption($key, $value); + } + return $this; + } + public function render() { - $viewer = $this->getUser(); + $viewer = $this->getViewer(); $corpus = $this->corpus; $context = $this->getContextObject(); + $options = $this->options; + + $oneoff = id(new PhabricatorMarkupOneOff()) + ->setContent($corpus); + + if ($options) { + $oneoff->setEngine($this->getEngine()); + } else { + $oneoff->setPreserveLinebreaks(true); + } + $content = PhabricatorMarkupEngine::renderOneObject( - id(new PhabricatorMarkupOneOff()) - ->setPreserveLinebreaks(true) - ->setContent($corpus), + $oneoff, 'default', $viewer, $context); - if ($this->markupType == self::DOCUMENT) { - return phutil_tag( - 'div', - array( - 'class' => 'phabricator-remarkup phui-document-view', - ), - $content); - } - return $content; } + private function getEngine() { + $options = $this->options; + $viewer = $this->getViewer(); + + $viewer_key = $viewer->getCacheFragment(); + + ksort($options); + $engine_key = serialize($options); + $engine_key = PhabricatorHash::digestForIndex($engine_key); + + $cache = PhabricatorCaches::getRequestCache(); + $cache_key = "remarkup.engine({$viewer}, {$engine_key})"; + + $engine = $cache->getKey($cache_key); + if (!$engine) { + $engine = PhabricatorMarkupEngine::newMarkupEngine($options); + $cache->setKey($cache_key, $engine); + } + + return $engine; + } + } diff --git a/src/view/form/AphrontFormView.php b/src/view/form/AphrontFormView.php index ecd4c1206e..3c92f72c88 100644 --- a/src/view/form/AphrontFormView.php +++ b/src/view/form/AphrontFormView.php @@ -84,9 +84,20 @@ final class AphrontFormView extends AphrontView { } public function appendRemarkupInstructions($remarkup) { - return $this->appendInstructions( - new PHUIRemarkupView($this->getViewer(), $remarkup)); + $view = $this->newInstructionsRemarkupView($remarkup); + return $this->appendInstructions($view); + } + public function newInstructionsRemarkupView($remarkup) { + $viewer = $this->getViewer(); + $view = new PHUIRemarkupView($viewer, $remarkup); + + $view->setRemarkupOptions( + array( + PHUIRemarkupView::OPTION_PRESERVE_LINEBREAKS => false, + )); + + return $view; } public function buildLayoutView() { diff --git a/src/view/form/PHUIFormLayoutView.php b/src/view/form/PHUIFormLayoutView.php index ffc0eb31d5..682c6c62cf 100644 --- a/src/view/form/PHUIFormLayoutView.php +++ b/src/view/form/PHUIFormLayoutView.php @@ -31,14 +31,11 @@ final class PHUIFormLayoutView extends AphrontView { } public function appendRemarkupInstructions($remarkup) { - if ($this->getUser() === null) { - throw new PhutilInvalidStateException('setUser'); - } + $view = id(new AphrontFormView()) + ->setViewer($this->getViewer()) + ->newInstructionsRemarkupView($remarkup); - $viewer = $this->getUser(); - $instructions = new PHUIRemarkupView($viewer, $remarkup); - - return $this->appendInstructions($instructions); + return $this->appendInstructions($view); } public function render() {