From cd41b834f71fc94aa36339babc2f2bbbea4255e6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 18 Feb 2013 09:44:43 -0800 Subject: [PATCH] Improve Diviner linking Summary: Do this somewhat reasonably: - For links to the same documentation book (the common case), go look up that the thing you're linking to actualy exists. If it doesn't, render a which we can make have a red background and warn about later. - For links to some other book, just generate a link and hope it hits something. We can improve and augment this later. - For non-documentation links (links in comments, e.g.) just generate a query link into the Diviner app. We'll do a query and figure out where to send the user after they click the link. We could pre-resolve these later. Test Plan: Generated documentation, saw it build mostly-correct links when objects were referenced correctly. Used preview to generate various `@{x:y|z}` things and made sure they ended up reasonable-looking. Reviewers: chad Reviewed By: chad CC: aran Maniphest Tasks: T988 Differential Revision: https://secure.phabricator.com/D5001 --- src/applications/diviner/atom/DivinerAtom.php | 3 + .../diviner/atom/DivinerAtomRef.php | 33 +++-- .../markup/DivinerRemarkupRuleSymbol.php | 121 ++++++++++++++++-- .../diviner/publisher/DivinerPublisher.php | 2 + .../publisher/DivinerStaticPublisher.php | 43 +++++++ .../renderer/DivinerDefaultRenderer.php | 67 +++++++++- .../diviner/renderer/DivinerRenderer.php | 31 +++++ 7 files changed, 276 insertions(+), 24 deletions(-) diff --git a/src/applications/diviner/atom/DivinerAtom.php b/src/applications/diviner/atom/DivinerAtom.php index 3aa56105f8..31a067fed5 100644 --- a/src/applications/diviner/atom/DivinerAtom.php +++ b/src/applications/diviner/atom/DivinerAtom.php @@ -285,8 +285,10 @@ final class DivinerAtom { public function getRef() { $group = null; + $title = null; if ($this->docblockMeta) { $group = $this->getDocblockMetaValue('group'); + $title = $this->getDocblockMetaValue('title'); } return id(new DivinerAtomRef()) @@ -294,6 +296,7 @@ final class DivinerAtom { ->setContext($this->getContext()) ->setType($this->getType()) ->setName($this->getName()) + ->setTitle($title) ->setGroup($group); } diff --git a/src/applications/diviner/atom/DivinerAtomRef.php b/src/applications/diviner/atom/DivinerAtomRef.php index b0eb3a87b0..f1bd344b75 100644 --- a/src/applications/diviner/atom/DivinerAtomRef.php +++ b/src/applications/diviner/atom/DivinerAtomRef.php @@ -9,6 +9,7 @@ final class DivinerAtomRef { private $group; private $summary; private $index; + private $title; public function getSortKey() { return implode( @@ -47,7 +48,7 @@ final class DivinerAtomRef { "Atom names must not be in the form '/@\d+/'. This pattern is ". "reserved for disambiguating atoms with similar names."); } - $this->name = $name; + $this->name = $normal_name; return $this; } @@ -78,7 +79,11 @@ final class DivinerAtomRef { } public function setBook($book) { - $this->book = self::normalizeString($book); + if ($book === null) { + $this->book = $book; + } else { + $this->book = self::normalizeString($book); + } return $this; } @@ -95,6 +100,15 @@ final class DivinerAtomRef { return $this->group; } + public function setTitle($title) { + $this->title = $title; + return $this; + } + + public function getTitle() { + return $this->title; + } + public function toDictionary() { return array( 'book' => $this->getBook(), @@ -104,6 +118,7 @@ final class DivinerAtomRef { 'group' => $this->getGroup(), 'index' => $this->getIndex(), 'summary' => $this->getSummary(), + 'title' => $this->getTitle(), ); } @@ -113,6 +128,7 @@ final class DivinerAtomRef { unset($dict['group']); unset($dict['index']); unset($dict['summary']); + unset($dict['title']); ksort($dict); return md5(serialize($dict)).'S'; @@ -120,13 +136,14 @@ final class DivinerAtomRef { public static function newFromDictionary(array $dict) { $obj = new DivinerAtomRef(); - $obj->book = idx($dict, 'book'); - $obj->context = idx($dict, 'context'); - $obj->type = idx($dict, 'type'); - $obj->name = idx($dict, 'name'); + $obj->setBook(idx($dict, 'book')); + $obj->setContext(idx($dict, 'context')); + $obj->setType(idx($dict, 'type')); + $obj->setName(idx($dict, 'name')); $obj->group = idx($dict, 'group'); $obj->index = idx($dict, 'index'); $obj->summary = idx($dict, 'summary'); + $obj->title = idx($dict, 'title'); return $obj; } @@ -163,8 +180,8 @@ final class DivinerAtomRef { // Replace all spaces with underscores. $str = preg_replace('/ +/', '_', $str); - // Replace control characters with "@". - $str = preg_replace('/[\x00-\x19]/', '@', $str); + // Replace control characters with "X". + $str = preg_replace('/[\x00-\x19]/', 'X', $str); // Replace specific problematic names with alternative names. $alternates = array( diff --git a/src/applications/diviner/markup/DivinerRemarkupRuleSymbol.php b/src/applications/diviner/markup/DivinerRemarkupRuleSymbol.php index bf2273b9ea..9b988c7e8f 100644 --- a/src/applications/diviner/markup/DivinerRemarkupRuleSymbol.php +++ b/src/applications/diviner/markup/DivinerRemarkupRuleSymbol.php @@ -2,35 +2,130 @@ final class DivinerRemarkupRuleSymbol extends PhutilRemarkupRule { + const KEY_RULE_ATOM_REF = 'rule.diviner.atomref'; + public function apply($text) { + // Grammar here is: + // + // rule = '@{' maybe_type name maybe_title '}' + // maybe_type = null | type ':' | type '@' book ':' + // name = name | name '@' context + // maybe_title = null | '|' title + // + // So these are all valid: + // + // @{name} + // @{type : name} + // @{name | title} + // @{type @ book : name @ context | title} + return $this->replaceHTML( - '/(?:^|\B)@{(?:(?P[^:]+?):)?(?P[^}]+?)}/', + '/(?:^|\B)@{'. + '(?:(?P[^:]+?):)?'. + '(?P[^}|]+?)'. + '(?:[|](?P[^}]+))?'. + '}/', array($this, 'markupSymbol'), $text); } public function markupSymbol($matches) { - $type = $matches['type']; - $name = $matches['name']; + $type = (string)idx($matches, 'type'); + $name = (string)$matches['name']; + $title = (string)idx($matches, 'title'); // Collapse sequences of whitespace into a single space. - $name = preg_replace('/\s+/', ' ', $name); + $type = preg_replace('/\s+/', ' ', trim($type)); + $name = preg_replace('/\s+/', ' ', trim($name)); + $title = preg_replace('/\s+/', ' ', trim($title)); + + $ref = array(); - $book = null; if (strpos($type, '@') !== false) { list($type, $book) = explode('@', $type, 2); + $ref['type'] = trim($type); + $ref['book'] = trim($book); + } else { + $ref['type'] = $type; } - // TODO: This doesn't actually do anything useful yet. + if (strpos($name, '@') !== false) { + list($name, $context) = explode('@', $name, 2); + $ref['name'] = trim($name); + $ref['context'] = trim($context); + } else { + $ref['name'] = $name; + } - $link = phutil_tag( - 'a', - array( - 'href' => '#', - ), - $name); + $ref['title'] = $title; - return $this->getEngine()->storeText($link); + foreach ($ref as $key => $value) { + if ($value === '') { + unset($ref[$key]); + } + } + + $engine = $this->getEngine(); + $token = $engine->storeText(''); + + $key = self::KEY_RULE_ATOM_REF; + $data = $engine->getTextMetadata($key, array()); + $data[$token] = $ref; + $engine->setTextMetadata($key, $data); + + return $token; + } + + public function didMarkupText() { + $engine = $this->getEngine(); + + $key = self::KEY_RULE_ATOM_REF; + $data = $engine->getTextMetadata($key, array()); + + $renderer = $engine->getConfig('diviner.renderer'); + + foreach ($data as $token => $ref_dict) { + $ref = DivinerAtomRef::newFromDictionary($ref_dict); + $title = nonempty($ref->getTitle(), $ref->getName()); + + $href = null; + if ($renderer) { + // Here, we're generating documentation. If possible, we want to find + // the real atom ref so we can render the correct default title and + // render invalid links in an alternate style. + + $ref = $renderer->normalizeAtomRef($ref); + if ($ref) { + $title = nonempty($ref->getTitle(), $ref->getName()); + $href = $renderer->getHrefForAtomRef($ref); + } + } else { + // Here, we're generating commment text or something like that. Just + // link to Diviner and let it sort things out. + + $href = id(new PhutilURI('/diviner/find/')) + ->setQueryParams($ref_dict + array('jump' => true)); + } + + if ($href) { + $link = phutil_tag( + 'a', + array( + 'class' => 'atom-ref', + 'href' => $href, + ), + $title); + } else { + $link = phutil_tag( + 'span', + array( + 'class' => 'atom-ref-invalid', + ), + $title); + } + + $engine->overwriteStoredText($token, $link); + } } } diff --git a/src/applications/diviner/publisher/DivinerPublisher.php b/src/applications/diviner/publisher/DivinerPublisher.php index 98837a2422..1a63704186 100644 --- a/src/applications/diviner/publisher/DivinerPublisher.php +++ b/src/applications/diviner/publisher/DivinerPublisher.php @@ -10,6 +10,7 @@ abstract class DivinerPublisher { private $symbolReverseMap; public function setRenderer(DivinerRenderer $renderer) { + $renderer->setPublisher($this); $this->renderer = $renderer; return $this; } @@ -103,6 +104,7 @@ abstract class DivinerPublisher { abstract protected function loadAllPublishedHashes(); abstract protected function deleteDocumentsByHash(array $hashes); abstract protected function createDocumentsByHash(array $hashes); + abstract public function findAtomByRef(DivinerAtomRef $ref); final public function publishAtoms(array $hashes) { $existing = $this->loadAllPublishedHashes(); diff --git a/src/applications/diviner/publisher/DivinerStaticPublisher.php b/src/applications/diviner/publisher/DivinerStaticPublisher.php index 4c03da30c0..fcfe859b4a 100644 --- a/src/applications/diviner/publisher/DivinerStaticPublisher.php +++ b/src/applications/diviner/publisher/DivinerStaticPublisher.php @@ -3,6 +3,7 @@ final class DivinerStaticPublisher extends DivinerPublisher { private $publishCache; + private $atomNameMap; private function getPublishCache() { if (!$this->publishCache) { @@ -115,6 +116,48 @@ final class DivinerStaticPublisher extends DivinerPublisher { Filesystem::writeFile($path, $content); } + public function findAtomByRef(DivinerAtomRef $ref) { + if ($ref->getBook() != $this->getConfig('name')) { + return null; + } + + if ($this->atomNameMap === null) { + $name_map = array(); + foreach ($this->getPublishCache()->getIndex() as $hash => $dict) { + $name_map[$dict['name']][$hash] = $dict; + } + $this->atomNameMap = $name_map; + } + + $name = $ref->getName(); + if (empty($this->atomNameMap[$name])) { + return null; + } + + $candidates = $this->atomNameMap[$name]; + foreach ($candidates as $key => $dict) { + $candidates[$key] = DivinerAtomRef::newFromDict($dict); + if ($ref->getType()) { + if ($candidates[$key]->getType() != $ref->getType()) { + unset($candidates[$key]); + } + } + + if ($ref->getContext()) { + if ($candidates[$key]->getContext() != $ref->getContext()) { + unset($candidates[$key]); + } + } + } + + // If we have exactly one uniquely identifiable atom, return it. + if (count($candidates) == 1) { + return $this->getAtomFromNodeHash(last_key($candidates)); + } + + return null; + } + private function addAtomToIndex($hash, DivinerAtom $atom) { $ref = $atom->getRef(); $ref->setIndex($this->getAtomSimilarIndex($atom)); diff --git a/src/applications/diviner/renderer/DivinerDefaultRenderer.php b/src/applications/diviner/renderer/DivinerDefaultRenderer.php index e5ca5958e5..f8380156c6 100644 --- a/src/applications/diviner/renderer/DivinerDefaultRenderer.php +++ b/src/applications/diviner/renderer/DivinerDefaultRenderer.php @@ -91,12 +91,17 @@ final class DivinerDefaultRenderer extends DivinerRenderer { protected function renderAtomDescription(DivinerAtom $atom) { $text = $this->getAtomDescription($atom); $engine = $this->getBlockMarkupEngine(); + + $this->pushAtomStack($atom); + $description = $engine->markupText($text); + $this->popAtomStack($atom); + return phutil_tag( 'div', array( 'class' => 'atom-description', ), - $engine->markupText($text)); + $description); } protected function getAtomDescription(DivinerAtom $atom) { @@ -106,12 +111,17 @@ final class DivinerDefaultRenderer extends DivinerRenderer { public function renderAtomSummary(DivinerAtom $atom) { $text = $this->getAtomSummary($atom); $engine = $this->getInlineMarkupEngine(); + + $this->pushAtomStack($atom); + $summary = $engine->markupText($text); + $this->popAtomStack(); + return phutil_tag( 'span', array( 'class' => 'atom-summary', ), - $engine->markupText($text)); + $summary); } protected function getAtomSummary(DivinerAtom $atom) { @@ -172,15 +182,66 @@ final class DivinerDefaultRenderer extends DivinerRenderer { } protected function getBlockMarkupEngine() { - return PhabricatorMarkupEngine::newMarkupEngine( + $engine = PhabricatorMarkupEngine::newMarkupEngine( array( 'preserve-linebreaks' => false, )); + $engine->setConfig('diviner.renderer', $this); + return $engine; } protected function getInlineMarkupEngine() { return $this->getBlockMarkupEngine(); } + public function normalizeAtomRef(DivinerAtomRef $ref) { + if (!strlen($ref->getBook())) { + $ref->setBook($this->getConfig('name')); + } + + if ($ref->getBook() != $this->getConfig('name')) { + // If the ref is from a different book, we can't normalize it. Just return + // it as-is if it has enough information to resolve. + if ($ref->getName() && $ref->getType()) { + return $ref; + } else { + return null; + } + } + + $atom = $this->getPublisher()->findAtomByRef($ref); + if ($atom) { + return $atom->getRef(); + } + + return null; + } + + protected function getAtomHrefDepth(DivinerAtom $atom) { + if ($atom->getContext()) { + return 4; + } else { + return 3; + } + } + + public function getHrefForAtomRef(DivinerAtomRef $ref) { + $atom = $this->peekAtomStack(); + $depth = $this->getAtomHrefDepth($atom); + $href = str_repeat('../', $depth); + + $book = $ref->getBook(); + $type = $ref->getType(); + $name = $ref->getName(); + $context = $ref->getContext(); + + $href .= $book.'/'.$type.'/'; + if ($context !== null) { + $href .= $context.'/'; + } + $href .= $name.'/'; + + return $href; + } } diff --git a/src/applications/diviner/renderer/DivinerRenderer.php b/src/applications/diviner/renderer/DivinerRenderer.php index 333ef07047..dc86e0fe32 100644 --- a/src/applications/diviner/renderer/DivinerRenderer.php +++ b/src/applications/diviner/renderer/DivinerRenderer.php @@ -2,8 +2,39 @@ abstract class DivinerRenderer { + private $publisher; + private $atomStack; + + public function setPublisher($publisher) { + $this->publisher = $publisher; + return $this; + } + + public function getPublisher() { + return $this->publisher; + } + + public function getConfig($key, $default = null) { + return $this->getPublisher()->getConfig($key, $default); + } + + protected function pushAtomStack(DivinerAtom $atom) { + $this->atomStack[] = $atom; + return $this; + } + + protected function peekAtomStack() { + return end($this->atomStack); + } + + protected function popAtomStack() { + array_pop($this->atomStack); + return $this; + } + abstract public function renderAtom(DivinerAtom $atom); abstract public function renderAtomSummary(DivinerAtom $atom); abstract public function renderAtomIndex(array $refs); + abstract public function getHrefForAtomRef(DivinerAtomRef $ref); }