1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Improve Diviner handling of paths and remarkup

Summary:
  - Currently, the atomizers don't emit atoms with the right file in all cases. Make them always emit it correctly.
  - Currently, we use absolute paths in some cases and relative paths in other cases. Use them consistently: relative when storing/presenting, absolute when accessing data.
  - Don't preserve linebreaks when marking up documentation (documentation is generally wrapped at 80col, but should not be wrapped in this way when displayed).
  - Markup Diviner link rules (albeit uselesly).

Test Plan:
Before:

{F33044}

After:

{F33045}

Reviewers: chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T988

Differential Revision: https://secure.phabricator.com/D4992
This commit is contained in:
epriestley 2013-02-17 15:40:44 -08:00
parent dba42ec5c7
commit 57a9c3f07c
9 changed files with 67 additions and 13 deletions

View file

@ -474,6 +474,7 @@ phutil_register_library_map(array(
'DivinerListController' => 'applications/diviner/controller/DivinerListController.php', 'DivinerListController' => 'applications/diviner/controller/DivinerListController.php',
'DivinerPublishCache' => 'applications/diviner/cache/DivinerPublishCache.php', 'DivinerPublishCache' => 'applications/diviner/cache/DivinerPublishCache.php',
'DivinerPublisher' => 'applications/diviner/publisher/DivinerPublisher.php', 'DivinerPublisher' => 'applications/diviner/publisher/DivinerPublisher.php',
'DivinerRemarkupRuleSymbol' => 'applications/diviner/markup/DivinerRemarkupRuleSymbol.php',
'DivinerRenderer' => 'applications/diviner/renderer/DivinerRenderer.php', 'DivinerRenderer' => 'applications/diviner/renderer/DivinerRenderer.php',
'DivinerStaticPublisher' => 'applications/diviner/publisher/DivinerStaticPublisher.php', 'DivinerStaticPublisher' => 'applications/diviner/publisher/DivinerStaticPublisher.php',
'DivinerWorkflow' => 'applications/diviner/workflow/DivinerWorkflow.php', 'DivinerWorkflow' => 'applications/diviner/workflow/DivinerWorkflow.php',
@ -1974,6 +1975,7 @@ phutil_register_library_map(array(
'DivinerGenerateWorkflow' => 'DivinerWorkflow', 'DivinerGenerateWorkflow' => 'DivinerWorkflow',
'DivinerListController' => 'PhabricatorController', 'DivinerListController' => 'PhabricatorController',
'DivinerPublishCache' => 'DivinerDiskCache', 'DivinerPublishCache' => 'DivinerDiskCache',
'DivinerRemarkupRuleSymbol' => 'PhutilRemarkupRule',
'DivinerStaticPublisher' => 'DivinerPublisher', 'DivinerStaticPublisher' => 'DivinerPublisher',
'DivinerWorkflow' => 'PhutilArgumentWorkflow', 'DivinerWorkflow' => 'PhutilArgumentWorkflow',
'DrydockAllocatorWorker' => 'PhabricatorWorker', 'DrydockAllocatorWorker' => 'PhabricatorWorker',

View file

@ -2,7 +2,7 @@
final class DivinerArticleAtomizer extends DivinerAtomizer { final class DivinerArticleAtomizer extends DivinerAtomizer {
public function atomize($file_name, $file_data) { protected function executeAtomize($file_name, $file_data) {
$atom = $this->newAtom(DivinerAtom::TYPE_ARTICLE) $atom = $this->newAtom(DivinerAtom::TYPE_ARTICLE)
->setLine(1) ->setLine(1)
->setLength(count(explode("\n", $file_data))) ->setLength(count(explode("\n", $file_data)))

View file

@ -6,6 +6,7 @@
abstract class DivinerAtomizer { abstract class DivinerAtomizer {
private $book; private $book;
private $fileName;
/** /**
* If you make a significant change to an atomizer, you can bump this * If you make a significant change to an atomizer, you can bump this
@ -15,7 +16,12 @@ abstract class DivinerAtomizer {
return 1; return 1;
} }
abstract public function atomize($file_name, $file_data); final public function atomize($file_name, $file_data) {
$this->fileName = $file_name;
return $this->executeAtomize($file_name, $file_data);
}
abstract protected function executeAtomize($file_name, $file_data);
final public function setBook($book) { final public function setBook($book) {
$this->book = $book; $this->book = $book;
@ -29,6 +35,7 @@ abstract class DivinerAtomizer {
protected function newAtom($type) { protected function newAtom($type) {
return id(new DivinerAtom()) return id(new DivinerAtom())
->setBook($this->getBook()) ->setBook($this->getBook())
->setFile($this->fileName)
->setType($type); ->setType($type);
} }

View file

@ -2,7 +2,7 @@
final class DivinerFileAtomizer extends DivinerAtomizer { final class DivinerFileAtomizer extends DivinerAtomizer {
public function atomize($file_name, $file_data) { protected function executeAtomize($file_name, $file_data) {
$atom = $this->newAtom(DivinerAtom::TYPE_FILE) $atom = $this->newAtom(DivinerAtom::TYPE_FILE)
->setName($file_name) ->setName($file_name)
->setFile($file_name) ->setFile($file_name)

View file

@ -0,0 +1,36 @@
<?php
final class DivinerRemarkupRuleSymbol extends PhutilRemarkupRule {
public function apply($text) {
return $this->replaceHTML(
'/(?:^|\B)@{(?:(?P<type>[^:]+?):)?(?P<name>[^}]+?)}/',
array($this, 'markupSymbol'),
$text);
}
public function markupSymbol($matches) {
$type = $matches['type'];
$name = $matches['name'];
// Collapse sequences of whitespace into a single space.
$name = preg_replace('/\s+/', ' ', $name);
$book = null;
if (strpos($type, '@') !== false) {
list($type, $book) = explode('@', $type, 2);
}
// TODO: This doesn't actually do anything useful yet.
$link = phutil_tag(
'a',
array(
'href' => '#',
),
$name);
return $this->getEngine()->storeText($link);
}
}

View file

@ -172,7 +172,10 @@ final class DivinerDefaultRenderer extends DivinerRenderer {
} }
protected function getBlockMarkupEngine() { protected function getBlockMarkupEngine() {
return PhabricatorMarkupEngine::newMarkupEngine(array()); return PhabricatorMarkupEngine::newMarkupEngine(
array(
'preserve-linebreaks' => false,
));
} }
protected function getInlineMarkupEngine() { protected function getInlineMarkupEngine() {

View file

@ -59,9 +59,14 @@ final class DivinerAtomizeWorkflow extends DivinerWorkflow {
$file_atomizer = new DivinerFileAtomizer(); $file_atomizer = new DivinerFileAtomizer();
foreach (array($atomizer, $file_atomizer) as $configure) {
$configure->setBook($this->getConfig('name'));
}
$all_atoms = array(); $all_atoms = array();
foreach ($files as $file) { foreach ($files as $file) {
$data = Filesystem::readFile($file); $abs_path = Filesystem::resolvePath($file, $this->getConfig('root'));
$data = Filesystem::readFile($abs_path);
if (!$this->shouldAtomizeFile($file, $data)) { if (!$this->shouldAtomizeFile($file, $data)) {
$console->writeLog("Skipping %s...\n", $file); $console->writeLog("Skipping %s...\n", $file);
@ -89,10 +94,6 @@ final class DivinerAtomizeWorkflow extends DivinerWorkflow {
$all_atoms = array_mergev($all_atoms); $all_atoms = array_mergev($all_atoms);
foreach ($all_atoms as $atom) {
$atom->setBook($this->getConfig('name'));
}
$all_atoms = mpull($all_atoms, 'toDictionary'); $all_atoms = mpull($all_atoms, 'toDictionary');
$all_atoms = ipull($all_atoms, null, 'hash'); $all_atoms = ipull($all_atoms, null, 'hash');

View file

@ -198,7 +198,7 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow {
private function findFilesInProject() { private function findFilesInProject() {
$file_hashes = id(new FileFinder($this->getConfig('root'))) $raw_hashes = id(new FileFinder($this->getConfig('root')))
->excludePath('*/.*') ->excludePath('*/.*')
->withType('f') ->withType('f')
->setGenerateChecksums(true) ->setGenerateChecksums(true)
@ -206,11 +206,13 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow {
$version = $this->getDivinerAtomWorldVersion(); $version = $this->getDivinerAtomWorldVersion();
foreach ($file_hashes as $file => $md5_hash) { $file_hashes = array();
foreach ($raw_hashes as $file => $md5_hash) {
$rel_file = Filesystem::readablePath($file, $this->getConfig('root'));
// We want the hash to change if the file moves or Diviner gets updated, // We want the hash to change if the file moves or Diviner gets updated,
// not just if the file content changes. Derive a hash from everything // not just if the file content changes. Derive a hash from everything
// we care about. // we care about.
$file_hashes[$file] = md5("{$file}\0{$md5_hash}\0{$version}").'F'; $file_hashes[$rel_file] = md5("{$rel_file}\0{$md5_hash}\0{$version}").'F';
} }
return $file_hashes; return $file_hashes;

View file

@ -355,6 +355,7 @@ final class PhabricatorMarkupEngine {
'uri.allowed-protocols'), 'uri.allowed-protocols'),
'syntax-highlighter.engine' => PhabricatorEnv::getEnvConfig( 'syntax-highlighter.engine' => PhabricatorEnv::getEnvConfig(
'syntax-highlighter.engine'), 'syntax-highlighter.engine'),
'preserve-linebreaks' => true,
); );
} }
@ -368,7 +369,7 @@ final class PhabricatorMarkupEngine {
$engine = new PhutilRemarkupEngine(); $engine = new PhutilRemarkupEngine();
$engine->setConfig('preserve-linebreaks', true); $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',
@ -422,6 +423,8 @@ final class PhabricatorMarkupEngine {
$rules[] = new PhabricatorRemarkupRuleMeme(); $rules[] = new PhabricatorRemarkupRuleMeme();
} }
$rules[] = new DivinerRemarkupRuleSymbol();
$rules[] = new PhabricatorRemarkupRuleMention(); $rules[] = new PhabricatorRemarkupRuleMention();
$rules[] = new PhutilRemarkupRuleBold(); $rules[] = new PhutilRemarkupRuleBold();