1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-21 04:50:55 +01:00

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 `<<<HEREDOC` format.
  - Viewed form instructions, Conduit API methods, HTTP parameter edit instructions.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10381

Differential Revision: https://secure.phabricator.com/D15963
This commit is contained in:
epriestley 2016-05-22 09:54:41 -07:00
parent 2a00f185eb
commit 3d3fff4991
7 changed files with 97 additions and 30 deletions

View file

@ -598,6 +598,11 @@ EOTEXT
$view = new PHUIRemarkupView($viewer, $remarkup); $view = new PHUIRemarkupView($viewer, $remarkup);
$view->setRemarkupOptions(
array(
PHUIRemarkupView::OPTION_PRESERVE_LINEBREAKS => false,
));
return id(new PHUIBoxView()) return id(new PHUIBoxView())
->appendChild($view) ->appendChild($view)
->addPadding(PHUI::PADDING_LARGE); ->addPadding(PHUI::PADDING_LARGE);

View file

@ -313,7 +313,14 @@ EOTEXT
protected function renderInstructions($corpus) { protected function renderInstructions($corpus) {
$viewer = $this->getUser(); $viewer = $this->getUser();
return new PHUIRemarkupView($viewer, $corpus); $view = new PHUIRemarkupView($viewer, $corpus);
$view->setRemarkupOptions(
array(
PHUIRemarkupView::OPTION_PRESERVE_LINEBREAKS => false,
));
return $view;
} }
} }

View file

@ -470,6 +470,7 @@ final class PhabricatorMarkupEngine extends Phobject {
$engine = new PhutilRemarkupEngine(); $engine = new PhutilRemarkupEngine();
$engine->setConfig('preserve-linebreaks', $options['preserve-linebreaks']); $engine->setConfig('preserve-linebreaks', $options['preserve-linebreaks']);
$engine->setConfig('pygments.enabled', $options['pygments']); $engine->setConfig('pygments.enabled', $options['pygments']);
$engine->setConfig( $engine->setConfig(
'uri.allowed-protocols', 'uri.allowed-protocols',

View file

@ -10,6 +10,7 @@ final class PhabricatorMarkupOneOff
private $content; private $content;
private $preserveLinebreaks; private $preserveLinebreaks;
private $engineRuleset; private $engineRuleset;
private $engine;
private $disableCache; private $disableCache;
public function setEngineRuleset($engine_ruleset) { public function setEngineRuleset($engine_ruleset) {
@ -35,6 +36,15 @@ final class PhabricatorMarkupOneOff
return $this->content; return $this->content;
} }
public function setEngine(PhutilMarkupEngine $engine) {
$this->engine = $engine;
return $this;
}
public function getEngine() {
return $this->engine;
}
public function setDisableCache($disable_cache) { public function setDisableCache($disable_cache) {
$this->disableCache = $disable_cache; $this->disableCache = $disable_cache;
return $this; return $this;
@ -49,6 +59,10 @@ final class PhabricatorMarkupOneOff
} }
public function newMarkupEngine($field) { public function newMarkupEngine($field) {
if ($this->engine) {
return $this->engine;
}
if ($this->engineRuleset) { if ($this->engineRuleset) {
return PhabricatorMarkupEngine::getEngine($this->engineRuleset); return PhabricatorMarkupEngine::getEngine($this->engineRuleset);
} else if ($this->preserveLinebreaks) { } else if ($this->preserveLinebreaks) {

View file

@ -12,21 +12,19 @@
final class PHUIRemarkupView extends AphrontView { final class PHUIRemarkupView extends AphrontView {
private $corpus; private $corpus;
private $markupType;
private $contextObject; 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) { public function __construct(PhabricatorUser $viewer, $corpus) {
$this->setUser($viewer); $this->setUser($viewer);
$this->corpus = $corpus; $this->corpus = $corpus;
} }
private function setMarkupType($type) {
$this->markupType($type);
return $this;
}
public function setContextObject($context_object) { public function setContextObject($context_object) {
$this->contextObject = $context_object; $this->contextObject = $context_object;
return $this; return $this;
@ -36,29 +34,63 @@ final class PHUIRemarkupView extends AphrontView {
return $this->contextObject; 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() { public function render() {
$viewer = $this->getUser(); $viewer = $this->getViewer();
$corpus = $this->corpus; $corpus = $this->corpus;
$context = $this->getContextObject(); $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( $content = PhabricatorMarkupEngine::renderOneObject(
id(new PhabricatorMarkupOneOff()) $oneoff,
->setPreserveLinebreaks(true)
->setContent($corpus),
'default', 'default',
$viewer, $viewer,
$context); $context);
if ($this->markupType == self::DOCUMENT) {
return phutil_tag(
'div',
array(
'class' => 'phabricator-remarkup phui-document-view',
),
$content);
}
return $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;
}
} }

View file

@ -84,9 +84,20 @@ final class AphrontFormView extends AphrontView {
} }
public function appendRemarkupInstructions($remarkup) { public function appendRemarkupInstructions($remarkup) {
return $this->appendInstructions( $view = $this->newInstructionsRemarkupView($remarkup);
new PHUIRemarkupView($this->getViewer(), $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() { public function buildLayoutView() {

View file

@ -31,14 +31,11 @@ final class PHUIFormLayoutView extends AphrontView {
} }
public function appendRemarkupInstructions($remarkup) { public function appendRemarkupInstructions($remarkup) {
if ($this->getUser() === null) { $view = id(new AphrontFormView())
throw new PhutilInvalidStateException('setUser'); ->setViewer($this->getViewer())
} ->newInstructionsRemarkupView($remarkup);
$viewer = $this->getUser(); return $this->appendInstructions($view);
$instructions = new PHUIRemarkupView($viewer, $remarkup);
return $this->appendInstructions($instructions);
} }
public function render() { public function render() {