launchpad-reviewers team mailing list archive

[Martin Pool <mbp@xxxxxxxxxxxxx>]

Martin Pool has proposed merging lp:~mbp/launchpad/391780-markdown into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~mbp/launchpad/391780-markdown/+merge/82832

Support Markdown markup in project and user home pages, per bug 391780.
-- 
https://code.launchpad.net/~mbp/launchpad/391780-markdown/+merge/82832
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mbp/launchpad/391780-markdown into lp:launchpad.

=== modified file 'lib/lp/app/browser/stringformatter.py'
--- lib/lp/app/browser/stringformatter.py	2011-11-03 03:24:57 +0000
+++ lib/lp/app/browser/stringformatter.py	2011-11-20 23:02:25 +0000
@@ -22,6 +22,7 @@
 import re
 from lxml import html
 from xml.sax.saxutils import unescape as xml_unescape
+import markdown
 
 from zope.component import getUtility
 from zope.interface import implements
@@ -39,6 +40,9 @@
     re_email_address,
     obfuscate_email,
     )
+from lp.services.features import (
+    getFeatureFlag,
+    )
 
 
 def escape(text, quote=True):
@@ -972,6 +976,12 @@
         url = root_url + self._stringtoformat
         return '<a href="%s">%s</a>' % (url, self._stringtoformat)
 
+    def markdown(self):
+        if getFeatureFlag('markdown.enabled'):
+            return format_markdown(self._stringtoformat)
+        else:
+            return self.text_to_html()
+
     def traverse(self, name, furtherPath):
         if name == 'nl_to_br':
             return self.nl_to_br()
@@ -981,6 +991,8 @@
             return self.lower()
         elif name == 'break-long-words':
             return self.break_long_words()
+        elif name == 'markdown':
+            return self.markdown()
         elif name == 'text-to-html':
             return self.text_to_html()
         elif name == 'text-to-html-with-target':
@@ -1021,3 +1033,14 @@
             return self.oops_id()
         else:
             raise TraversalError(name)
+
+
+def format_markdown(text):
+    """Return html form of marked-up text."""
+    # This returns whole paragraphs (in p tags), similarly to text_to_html.
+    md = markdown.Markdown(
+        safe_mode='escape',
+        extensions=[
+            'tables',
+            ])
+    return md.convert(text)  # How easy was that?

=== modified file 'lib/lp/app/browser/tests/test_stringformatter.py'
--- lib/lp/app/browser/tests/test_stringformatter.py	2011-08-28 07:29:11 +0000
+++ lib/lp/app/browser/tests/test_stringformatter.py	2011-11-20 23:02:25 +0000
@@ -9,6 +9,11 @@
 from textwrap import dedent
 import unittest
 
+from testtools.matchers import (
+    Equals,
+    Matcher,
+    )
+
 from zope.component import getUtility
 
 from canonical.config import config
@@ -20,6 +25,7 @@
     linkify_bug_numbers,
     )
 from lp.testing import TestCase
+from lp.services.features.testing import FeatureFixture
 
 
 def test_split_paragraphs():
@@ -401,6 +407,50 @@
                 expected_string, formatted_string))
 
 
+class MarksDownAs(Matcher):
+
+    def __init__(self, expected_html):
+        self.expected_html = expected_html
+
+    def match(self, input_string):
+        return Equals(self.expected_html).match(
+            FormattersAPI(input_string).markdown())
+
+
+class TestMarkdownDisabled(TestCase):
+    """Feature flag can turn Markdown stuff off.
+    """
+
+    layer = DatabaseFunctionalLayer  # Fixtures need the database for now
+
+    def setUp(self):
+        super(TestMarkdownDisabled, self).setUp()
+        self.useFixture(FeatureFixture({'markdown.enabled': None}))
+
+    def test_plain_text(self):
+        self.assertThat(
+            'hello **simple** world',
+            MarksDownAs('<p>hello **simple** world</p>'))
+
+
+class TestMarkdown(TestCase):
+    """Test for Markdown integration within Launchpad.
+
+    Not an exhaustive test, more of a check for our integration and configuration.
+    """
+
+    layer = DatabaseFunctionalLayer  # Fixtures need the database for now
+
+    def setUp(self):
+        super(TestMarkdown, self).setUp()
+        self.useFixture(FeatureFixture({'markdown.enabled': 'on'}))
+
+    def test_plain_text(self):
+        self.assertThat(
+            'hello world',
+            MarksDownAs('<p>hello world</p>'))
+
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTests(DocTestSuite())

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2011-10-19 12:44:55 +0000
+++ lib/lp/registry/browser/person.py	2011-11-20 23:02:25 +0000
@@ -2381,10 +2381,12 @@
         if content is None:
             return None
         elif self.is_probationary_or_invalid_user:
+            # XXX: Is this really useful?  They can post links in many other
+            # places. -- mbp 2011-11-20.
             return cgi.escape(content)
         else:
             formatter = FormattersAPI
-            return formatter(content).text_to_html()
+            return formatter(content).markdown()
 
     @cachedproperty
     def recently_approved_members(self):

=== modified file 'lib/lp/registry/templates/product-index.pt'
--- lib/lp/registry/templates/product-index.pt	2011-06-16 13:50:58 +0000
+++ lib/lp/registry/templates/product-index.pt	2011-11-20 23:02:25 +0000
@@ -55,14 +55,15 @@
           <a tal:replace="structure overview_menu/review_license/fmt:icon"/>
         </p>
 
-        <div class="summary" tal:content="context/summary">
+	<div class="summary" 
+	  tal:content="structure context/summary/fmt:markdown">
           $Product.summary goes here. This should be quite short,
           just a single paragraph of text really, giving the project
           highlights.
         </div>
 
         <div class="description"
-          tal:content="structure context/description/fmt:text-to-html"
+          tal:content="structure context/description/fmt:markdown"
           tal:condition="context/description">
           $Product.description goes here. This should be a longer piece of
           text, up to three paragraphs in length, which gives much more

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2011-11-16 23:46:01 +0000
+++ lib/lp/services/features/flags.py	2011-11-20 23:02:25 +0000
@@ -107,6 +107,12 @@
      '',
      '',
      ''),
+    ('markdown.enabled',
+     'boolean',
+     'Interpret selected user content as Markdown.',
+     'disabled',
+     'Markdown',
+     'https://launchpad.net/bugs/391780'),
     ('memcache',
      'boolean',
      'Enables use of memcached where it is supported.',

=== modified file 'setup.py'
--- setup.py	2011-11-07 12:47:36 +0000
+++ setup.py	2011-11-20 23:02:25 +0000
@@ -52,6 +52,7 @@
         # Required for launchpadlib
         'keyring',
         'manuel',
+        'Markdown',
         'mechanize',
         'meliae',
         'mercurial',

=== modified file 'versions.cfg'
--- versions.cfg	2011-11-17 23:52:26 +0000
+++ versions.cfg	2011-11-20 23:02:25 +0000
@@ -43,6 +43,7 @@
 lazr.testing = 0.1.1
 lazr.uri = 1.0.2
 manuel = 1.1.1
+Markdown = 2.0.3
 martian = 0.11
 mechanize = 0.1.11
 meliae = 0.2.0.final.0