From 44c6109bf253b89a25389b73cd6df2444fa27dc5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 17 Oct 2012 08:36:33 -0700 Subject: [PATCH] Add cached summarization to PhamePost Summary: Restore summarization. Use the remarkup cache, and try to do it somewhat-intelligently (pick the first paragraph that looks like it's text). Test Plan: {F21323} {F21324} Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1373 Differential Revision: https://secure.phabricator.com/D3715 --- src/__phutil_library_map__.php | 2 - .../post/PhamePostPreviewController.php | 2 + .../phame/skins/PhameBasicBlogSkin.php | 9 +- src/applications/phame/storage/PhamePost.php | 9 +- .../phame/view/PhameBlogDetailView.php | 100 ------------------ src/applications/phame/view/PhamePostView.php | 32 ++++++ .../markup/PhabricatorMarkupEngine.php | 50 +++++++++ 7 files changed, 99 insertions(+), 105 deletions(-) delete mode 100644 src/applications/phame/view/PhameBlogDetailView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a69d7a4032..6ee3882191 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1152,7 +1152,6 @@ phutil_register_library_map(array( 'PhameBasicBlogSkin' => 'applications/phame/skins/PhameBasicBlogSkin.php', 'PhameBlog' => 'applications/phame/storage/PhameBlog.php', 'PhameBlogDeleteController' => 'applications/phame/controller/blog/PhameBlogDeleteController.php', - 'PhameBlogDetailView' => 'applications/phame/view/PhameBlogDetailView.php', 'PhameBlogEditController' => 'applications/phame/controller/blog/PhameBlogEditController.php', 'PhameBlogListController' => 'applications/phame/controller/blog/PhameBlogListController.php', 'PhameBlogLiveController' => 'applications/phame/controller/blog/PhameBlogLiveController.php', @@ -2286,7 +2285,6 @@ phutil_register_library_map(array( 2 => 'PhabricatorMarkupInterface', ), 'PhameBlogDeleteController' => 'PhameController', - 'PhameBlogDetailView' => 'AphrontView', 'PhameBlogEditController' => 'PhameController', 'PhameBlogListController' => 'PhameController', 'PhameBlogLiveController' => 'PhameController', diff --git a/src/applications/phame/controller/post/PhamePostPreviewController.php b/src/applications/phame/controller/post/PhamePostPreviewController.php index 8964c5a441..b69e0778e1 100644 --- a/src/applications/phame/controller/post/PhamePostPreviewController.php +++ b/src/applications/phame/controller/post/PhamePostPreviewController.php @@ -39,6 +39,8 @@ extends PhameController { PhamePost::MARKUP_FIELD_BODY, $user); + $content = '
'.$content.'
'; + return id(new AphrontAjaxResponse())->setContent($content); } } diff --git a/src/applications/phame/skins/PhameBasicBlogSkin.php b/src/applications/phame/skins/PhameBasicBlogSkin.php index 8c1a7109d9..b177420488 100644 --- a/src/applications/phame/skins/PhameBasicBlogSkin.php +++ b/src/applications/phame/skins/PhameBasicBlogSkin.php @@ -69,12 +69,17 @@ abstract class PhameBasicBlogSkin extends PhameBlogSkin { } protected function renderPostList(array $posts) { + $summaries = array(); + foreach ($posts as $post) { + $summaries[] = $post->renderWithSummary(); + } + $list = phutil_render_tag( 'div', array( 'class' => 'phame-post-list', ), - id(new AphrontNullView())->appendChild($posts)->render()); + id(new AphrontNullView())->appendChild($summaries)->render()); $pager = $this->renderOlderPageLink().$this->renderNewerPageLink(); if ($pager) { @@ -224,6 +229,7 @@ abstract class PhameBasicBlogSkin extends PhameBlogSkin { $phids = array(); foreach ($posts as $post) { $engine->addObject($post, PhamePost::MARKUP_FIELD_BODY); + $engine->addObject($post, PhamePost::MARKUP_FIELD_SUMMARY); $phids[] = $post->getBloggerPHID(); } @@ -240,6 +246,7 @@ abstract class PhameBasicBlogSkin extends PhameBlogSkin { ->setSkin($this) ->setPost($post) ->setBody($engine->getOutput($post, PhamePost::MARKUP_FIELD_BODY)) + ->setSummary($engine->getOutput($post, PhamePost::MARKUP_FIELD_SUMMARY)) ->setAuthor($handles[$post->getBloggerPHID()]); $post->makeEphemeral(); diff --git a/src/applications/phame/storage/PhamePost.php b/src/applications/phame/storage/PhamePost.php index c9143141d2..7a707789a3 100644 --- a/src/applications/phame/storage/PhamePost.php +++ b/src/applications/phame/storage/PhamePost.php @@ -23,6 +23,7 @@ final class PhamePost extends PhameDAO implements PhabricatorPolicyInterface, PhabricatorMarkupInterface { const MARKUP_FIELD_BODY = 'markup:body'; + const MARKUP_FIELD_SUMMARY = 'markup:summary'; const VISIBILITY_DRAFT = 0; const VISIBILITY_PUBLISHED = 1; @@ -193,10 +194,14 @@ final class PhamePost extends PhameDAO public function getMarkupText($field) { - return $this->getBody(); + switch ($field) { + case self::MARKUP_FIELD_BODY: + return $this->getBody(); + case self::MARKUP_FIELD_SUMMARY: + return PhabricatorMarkupEngine::summarize($this->getBody()); + } } - public function didMarkupText( $field, $output, diff --git a/src/applications/phame/view/PhameBlogDetailView.php b/src/applications/phame/view/PhameBlogDetailView.php deleted file mode 100644 index 330ff8b0e1..0000000000 --- a/src/applications/phame/view/PhameBlogDetailView.php +++ /dev/null @@ -1,100 +0,0 @@ -user = $user; - return $this; - } - private function getUser() { - return $this->user; - } - - public function setBloggers(array $bloggers) { - assert_instances_of($bloggers, 'PhabricatorObjectHandle'); - $this->bloggers = $bloggers; - return $this; - } - private function getBloggers() { - return $this->bloggers; - } - - public function setBlog(PhameBlog $blog) { - $this->blog = $blog; - return $this; - } - private function getBlog() { - return $this->blog; - } - - public function render() { - require_celerity_resource('phabricator-remarkup-css'); - - $user = $this->getUser(); - $blog = $this->getBlog(); - $bloggers = $this->getBloggers(); - $name = phutil_escape_html($blog->getName()); - $description = phutil_escape_html($blog->getDescription()); - - $detail = phutil_render_tag( - 'div', - array( - 'class' => 'blog-detail' - ), - phutil_render_tag( - 'div', - array( - 'class' => 'header', - ), - phutil_render_tag( - 'h1', - array(), - $name - ) - ). - phutil_render_tag( - 'div', - array( - 'class' => 'description' - ), - $description - ) - ); - - return $detail; - } - - private function getBloggersHTML(array $bloggers) { - assert_instances_of($bloggers, 'PhabricatorObjectHandle'); - - $arr = array(); - foreach ($bloggers as $blogger) { - $arr[] = ''.phutil_escape_html($blogger->getName()).''; - } - - return implode(' · ', $arr); - } -} diff --git a/src/applications/phame/view/PhamePostView.php b/src/applications/phame/view/PhamePostView.php index 235dd822de..b39af0d038 100644 --- a/src/applications/phame/view/PhamePostView.php +++ b/src/applications/phame/view/PhamePostView.php @@ -26,6 +26,8 @@ final class PhamePostView extends AphrontView { private $viewer; private $body; private $skin; + private $summary; + public function setSkin(PhameBlogSkin $skin) { $this->skin = $skin; @@ -72,6 +74,15 @@ final class PhamePostView extends AphrontView { return $this->body; } + public function setSummary($summary) { + $this->summary = $summary; + return $this; + } + + public function getSummary() { + return $this->summary; + } + public function renderTitle() { $href = $this->getSkin()->getURI('post/'.$this->getPost()->getPhameTitle()); return phutil_render_tag( @@ -111,6 +122,15 @@ final class PhamePostView extends AphrontView { $this->getBody()); } + public function renderSummary() { + return phutil_render_tag( + 'div', + array( + 'class' => 'phame-post-body', + ), + $this->getSummary()); + } + public function renderComments() { $post = $this->getPost(); @@ -141,6 +161,18 @@ final class PhamePostView extends AphrontView { $this->renderComments()); } + public function renderWithSummary() { + return phutil_render_tag( + 'div', + array( + 'class' => 'phame-post', + ), + $this->renderTitle(). + $this->renderDatePublished(). + $this->renderSummary(). + $this->renderComments()); + } + private function renderFacebookComments() { $fb_id = PhabricatorEnv::getEnvConfig('facebook.application-id'); if (!$fb_id) { diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index 3a3c297d1e..093fe16716 100644 --- a/src/infrastructure/markup/PhabricatorMarkupEngine.php +++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php @@ -495,4 +495,54 @@ final class PhabricatorMarkupEngine { return $mentions; } + + /** + * Produce a corpus summary, in a way that shortens the underlying text + * without truncating it somewhere awkward. + * + * TODO: We could do a better job of this. + * + * @param string Remarkup corpus to summarize. + * @return string Summarized corpus. + */ + public static function summarize($corpus) { + + // Major goals here are: + // - Don't split in the middle of a character (utf-8). + // - Don't split in the middle of, e.g., **bold** text, since + // we end up with hanging '**' in the summary. + // - Try not to pick an image macro, header, embedded file, etc. + // - Hopefully don't return too much text. We don't explicitly limit + // this right now. + + $blocks = preg_split("/\n *\n\s*/", trim($corpus)); + + $best = null; + foreach ($blocks as $block) { + // This is a test for normal spaces in the block, i.e. a heuristic to + // distinguish standard paragraphs from things like image macros. It may + // not work well for non-latin text. We prefer to summarize with a + // paragraph of normal words over an image macro, if possible. + $has_space = preg_match('/\w\s\w/', $block); + + // This is a test to find embedded images and headers. We prefer to + // summarize with a normal paragraph over a header or an embedded object, + // if possible. + $has_embed = preg_match('/^[{=]/', $block); + + if ($has_space && !$has_embed) { + // This seems like a good summary, so return it. + return $block; + } + + if (!$best) { + // This is the first block we found; if everything is garbage just + // use the first block. + $best = $block; + } + } + + return $best; + } + }