incubator-allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tvansteenbu...@apache.org
Subject git commit: [#6111][#6140] Improve commit message rendering
Date Tue, 29 Oct 2013 00:41:53 GMT
Updated Branches:
  refs/heads/tv/6140 [created] 52eaea741


[#6111][#6140] Improve commit message rendering

- Handle Trac-style artifact refs
- Only do minimal Markdown conversion (refs and linebreaks)

Signed-off-by: Tim Van Steenburgh <tvansteenburgh@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/52eaea74
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/52eaea74
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/52eaea74

Branch: refs/heads/tv/6140
Commit: 52eaea74185e6b594389f90dbf4341274a7cb0ca
Parents: 91cfa35
Author: Tim Van Steenburgh <tvansteenburgh@gmail.com>
Authored: Mon Oct 28 17:56:23 2013 +0000
Committer: Tim Van Steenburgh <tvansteenburgh@gmail.com>
Committed: Tue Oct 29 00:41:03 2013 +0000

----------------------------------------------------------------------
 Allura/allura/lib/app_globals.py                |  14 +-
 Allura/allura/lib/markdown_extensions.py        | 168 ++++++++++++++++++-
 Allura/allura/templates/widgets/repo/log.html   |   2 +-
 .../allura/templates/widgets/repo/revision.html |   4 +-
 Allura/allura/tests/test_markdown.py            | 130 ++++++++++++++
 5 files changed, 313 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/52eaea74/Allura/allura/lib/app_globals.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/app_globals.py b/Allura/allura/lib/app_globals.py
index 14fa5a1..332127b 100644
--- a/Allura/allura/lib/app_globals.py
+++ b/Allura/allura/lib/app_globals.py
@@ -51,7 +51,10 @@ from ming.utils import LazyProperty
 
 import allura.tasks.event_tasks
 from allura import model as M
-from allura.lib.markdown_extensions import ForgeExtension
+from allura.lib.markdown_extensions import (
+        ForgeExtension,
+        CommitMessageExtension,
+        )
 from allura.eventslistener import PostEvent
 
 from allura.lib import gravatar, plugin, utils
@@ -429,6 +432,15 @@ class Globals(object):
             return self.forge_markdown(wiki=True)
 
     @property
+    def markdown_commit(self):
+        """Return a Markdown parser configured for rendering commit messages.
+
+        """
+        app = getattr(c, 'app', None)
+        return ForgeMarkdown(extensions=[CommitMessageExtension(app), 'nl2br'],
+                output_format='html4')
+
+    @property
     def production_mode(self):
         return asbool(config.get('debug')) == False
 

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/52eaea74/Allura/allura/lib/markdown_extensions.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/markdown_extensions.py b/Allura/allura/lib/markdown_extensions.py
index ebd944b..0682e89 100644
--- a/Allura/allura/lib/markdown_extensions.py
+++ b/Allura/allura/lib/markdown_extensions.py
@@ -20,7 +20,6 @@ import logging
 from urlparse import urljoin
 
 from tg import config
-from pylons import request
 from BeautifulSoup import BeautifulSoup
 
 import markdown
@@ -40,6 +39,173 @@ PLAINTEXT_BLOCK_RE = re.compile( \
 MACRO_PATTERN = r'\[\[([^\]\[]+)\]\]'
 
 
+class CommitMessageExtension(markdown.Extension):
+    """Markdown extension for processing commit messages.
+
+    People don't expect their commit messages to be parsed as Markdown. This
+    extension is therefore intentionally minimal in what it does. It knows how
+    to handle Trac-style short refs, will replace short refs with links, and
+    will create paragraphs around double-line breaks. That is *all* it does.
+
+    To make it do more, re-add some inlinePatterns and/or blockprocessors.
+
+    Some examples of the Trac-style refs this extension can parse::
+
+        #100
+        r123
+        ticket:100
+        comment:13:ticket:100
+        source:path/to/file.c@123#L456 (rev 123, lineno 456)
+
+    Trac-style refs will be converted to links to the appropriate artifact by
+    the :class:`PatternReplacingProcessor` preprocessor.
+
+    """
+    def __init__(self, app):
+        markdown.Extension.__init__(self)
+        self.app = app
+        self._use_wiki = False
+
+    def extendMarkdown(self, md, md_globals):
+        md.registerExtension(self)
+        # remove default preprocessors and add our own
+        md.preprocessors.clear()
+        md.preprocessors['trac_refs'] = PatternReplacingProcessor(
+            TracRef1(), TracRef2(), TracRef3(self.app))
+        # remove all inlinepattern processors except short refs and links
+        md.inlinePatterns.clear()
+        md.inlinePatterns["link"] = markdown.inlinepatterns.LinkPattern(
+                markdown.inlinepatterns.LINK_RE, md)
+        md.inlinePatterns['short_reference'] = ForgeLinkPattern(
+                markdown.inlinepatterns.SHORT_REF_RE, md, ext=self)
+        # remove all default block processors except for paragraph
+        md.parser.blockprocessors.clear()
+        md.parser.blockprocessors['paragraph'] = \
+                markdown.blockprocessors.ParagraphProcessor(md.parser)
+        # wrap artifact link text in square brackets
+        self.forge_link_tree_processor = ForgeLinkTreeProcessor(md)
+        md.treeprocessors['links'] = self.forge_link_tree_processor
+        # Sanitize HTML
+        md.postprocessors['sanitize_html'] = HTMLSanitizer()
+        # Put a class around markdown content for custom css
+        md.postprocessors['add_custom_class'] = AddCustomClass()
+        md.postprocessors['mark_safe'] = MarkAsSafe()
+
+    def reset(self):
+        self.forge_link_tree_processor.reset()
+
+
+class Pattern(object):
+    """Base class for regex patterns used by the :class:`PatternReplacingProcessor`.
+
+    Subclasses must define :attr:`pattern` (a compiled regex), and
+    :meth:`repl`.
+
+    """
+    BEGIN, END = r'(^|\b|\s)', r'($|\b|\s)'
+
+    def sub(self, line):
+        return self.pattern.sub(self.repl, line)
+
+    def repl(self, match):
+        """Return a string to replace ``match`` in the source string (the
+        string in which the match was found).
+
+        """
+        return match.group()
+
+
+class TracRef1(Pattern):
+    """Replaces Trac-style short refs with links. Example patterns::
+
+        #100 (ticket 100)
+        r123 (revision 123)
+
+    """
+    pattern = re.compile(r'(?<!\[|\w)([#r]\d+)(?!\]|\w)')
+
+    def repl(self, match):
+        shortlink = M.Shortlink.lookup(match.group(1))
+        if shortlink and not getattr(shortlink.ref.artifact, 'deleted', False):
+            return '[{ref}]({url})'.format(
+                    ref=match.group(1),
+                    url=shortlink.url)
+        return match.group()
+
+
+class TracRef2(Pattern):
+    """Replaces Trac-style short refs with links. Example patterns::
+
+        ticket:100
+        comment:13:ticket:400
+
+    """
+    pattern = re.compile(
+            Pattern.BEGIN + r'((comment:\d+:)?(ticket:)(\d+))' + Pattern.END)
+
+    def repl(self, match):
+        shortlink = M.Shortlink.lookup('#' + match.group(5))
+        if shortlink and not getattr(shortlink.ref.artifact, 'deleted', False):
+            return '{front}[{ref}]({url}){back}'.format(
+                    front=match.group(1),
+                    ref=match.group(2),
+                    url=shortlink.url,
+                    back=match.group(6))
+        return match.group()
+
+
+class TracRef3(Pattern):
+    """Replaces Trac-style short refs with links. Example patterns::
+
+        source:trunk/server/file.c@123#L456 (rev 123, lineno 456)
+
+    Creates a link to a specific line of a source file at a specific revision.
+
+    """
+    pattern = re.compile(
+            Pattern.BEGIN + r'((source:)([^@#]+)(@(\d+))?(#L(\d+))?)' + Pattern.END)
+
+    def __init__(self, app):
+        super(Pattern, self).__init__()
+        self.app = app
+
+    def repl(self, match):
+        if not self.app:
+            return match.group()
+        file, rev, lineno = (
+                match.group(4),
+                match.group(6) or 'HEAD',
+                '#l' + match.group(8) if match.group(8) else '')
+        url = '{app_url}{rev}/tree/{file}{lineno}'.format(
+                app_url=self.app.url,
+                rev=rev,
+                file=file,
+                lineno=lineno)
+        return '{front}[{ref}]({url}){back}'.format(
+                front=match.group(1),
+                ref=match.group(2),
+                url=url,
+                back=match.group(9))
+
+
+class PatternReplacingProcessor(markdown.preprocessors.Preprocessor):
+    """A Markdown preprocessor that searches the source lines for patterns and
+    replaces matches with alternate text.
+
+    """
+
+    def __init__(self, *patterns):
+        self.patterns = patterns or []
+
+    def run(self, lines):
+        new_lines = []
+        for line in lines:
+            for pattern in self.patterns:
+                line = pattern.sub(line)
+            new_lines.append(line)
+        return new_lines
+
+
 class ForgeExtension(markdown.Extension):
 
     def __init__(self, wiki=False, email=False, macro_context=None):

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/52eaea74/Allura/allura/templates/widgets/repo/log.html
----------------------------------------------------------------------
diff --git a/Allura/allura/templates/widgets/repo/log.html b/Allura/allura/templates/widgets/repo/log.html
index f526a07..0359a8f 100644
--- a/Allura/allura/templates/widgets/repo/log.html
+++ b/Allura/allura/templates/widgets/repo/log.html
@@ -54,7 +54,7 @@
                 {%- if commit.committed.email != commit.authored.email %},
                 pushed by {{ user_link(commit.committed.email, commit.committed.name) }}
                 {% endif %}
-                {{g.markdown.convert(commit.message)}}
+                {{g.markdown_commit.convert(commit.message)}}
                 {% if commit.rename_details %}
                     <div>
                       <b>renamed from</b>

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/52eaea74/Allura/allura/templates/widgets/repo/revision.html
----------------------------------------------------------------------
diff --git a/Allura/allura/templates/widgets/repo/revision.html b/Allura/allura/templates/widgets/repo/revision.html
index 48df9c1..bfe5903 100644
--- a/Allura/allura/templates/widgets/repo/revision.html
+++ b/Allura/allura/templates/widgets/repo/revision.html
@@ -19,8 +19,8 @@
 {% from 'allura:templates/jinja_master/lib.html' import email_gravatar, abbr_date with context
%}
 <div class="commit-details">
     <div class="commit-message">
-        <div class="first-line">{{g.markdown.convert(h.really_unicode(value.message.split('\n')[0]))}}</div>
-        {{g.markdown.convert(h.really_unicode('\n'.join(value.message.split('\n')[1:])))}}
+        <div class="first-line">{{g.markdown_commit.convert(h.really_unicode(value.message.split('\n')[0]))}}</div>
+        {{g.markdown_commit.convert(h.really_unicode('\n'.join(value.message.split('\n')[1:])))}}
     </div>
     <div class="commit-details">
 

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/52eaea74/Allura/allura/tests/test_markdown.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_markdown.py b/Allura/allura/tests/test_markdown.py
new file mode 100644
index 0000000..c7fb148
--- /dev/null
+++ b/Allura/allura/tests/test_markdown.py
@@ -0,0 +1,130 @@
+# -*- coding: utf-8 -*-
+
+#       Licensed to the Apache Software Foundation (ASF) under one
+#       or more contributor license agreements.  See the NOTICE file
+#       distributed with this work for additional information
+#       regarding copyright ownership.  The ASF licenses this file
+#       to you under the Apache License, Version 2.0 (the
+#       "License"); you may not use this file except in compliance
+#       with the License.  You may obtain a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#       Unless required by applicable law or agreed to in writing,
+#       software distributed under the License is distributed on an
+#       "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+#       KIND, either express or implied.  See the License for the
+#       specific language governing permissions and limitations
+#       under the License.
+
+import unittest
+import mock
+
+from allura.lib import markdown_extensions as mde
+
+
+class TestTracRef1(unittest.TestCase):
+    @mock.patch('allura.lib.markdown_extensions.M.Shortlink.lookup')
+    def test_no_such_artifact(self, lookup):
+        lookup.return_value = None
+        self.assertEqual(mde.TracRef1().sub('#100'), '#100')
+
+    def test_skip_if_brackets(self):
+        self.assertEqual(mde.TracRef1().sub('[#100]'), '[#100]')
+        self.assertEqual(mde.TracRef1().sub('[r123]'), '[r123]')
+
+    def test_word_boundaries(self):
+        self.assertEqual(mde.TracRef1().sub('foo#100'), 'foo#100')
+        self.assertEqual(mde.TracRef1().sub('r123bar'), 'r123bar')
+
+    @mock.patch('allura.lib.markdown_extensions.M.Shortlink.lookup')
+    def test_legit_refs(self, lookup):
+        shortlink = mock.Mock(url='/p/project/tool/artifact')
+        shortlink.ref.artifact.deleted = False
+        lookup.return_value = shortlink
+        self.assertEqual(mde.TracRef1().sub('#100'), '[#100](/p/project/tool/artifact)')
+        self.assertEqual(mde.TracRef1().sub('r123'), '[r123](/p/project/tool/artifact)')
+
+
+class TestTracRef2(unittest.TestCase):
+    @mock.patch('allura.lib.markdown_extensions.M.Shortlink.lookup')
+    def test_no_such_artifact(self, lookup):
+        lookup.return_value = None
+        self.assertEqual(mde.TracRef2().sub('ticket:100'), 'ticket:100')
+
+    def test_word_boundaries(self):
+        self.assertEqual(mde.TracRef2().sub('myticket:100'), 'myticket:100')
+        self.assertEqual(mde.TracRef2().sub('ticket:100th'), 'ticket:100th')
+
+    @mock.patch('allura.lib.markdown_extensions.M.Shortlink.lookup')
+    def test_legit_refs(self, lookup):
+        shortlink = mock.Mock(url='/p/project/tool/artifact')
+        shortlink.ref.artifact.deleted = False
+        lookup.return_value = shortlink
+        self.assertEqual(mde.TracRef2().sub('ticket:100'), '[ticket:100](/p/project/tool/artifact)')
+        self.assertEqual(mde.TracRef2().sub('[ticket:100]'), '[[ticket:100](/p/project/tool/artifact)]')
+        self.assertEqual(mde.TracRef2().sub('comment:13:ticket:100'),
+                '[comment:13:ticket:100](/p/project/tool/artifact)')
+
+
+class TestTracRef3(unittest.TestCase):
+    def test_no_app_context(self):
+        self.assertEqual(mde.TracRef3(None).sub('source:file.py'), 'source:file.py')
+
+    def test_legit_refs(self):
+        app = mock.Mock(url='/p/project/tool/')
+        self.assertEqual(mde.TracRef3(app).sub('source:file.py'),
+                '[source:file.py](/p/project/tool/HEAD/tree/file.py)')
+        self.assertEqual(mde.TracRef3(app).sub('source:file.py@123'),
+                '[source:file.py@123](/p/project/tool/123/tree/file.py)')
+        self.assertEqual(mde.TracRef3(app).sub('source:file.py@123#L456'),
+                '[source:file.py@123#L456](/p/project/tool/123/tree/file.py#l456)')
+        self.assertEqual(mde.TracRef3(app).sub('source:file.py#L456'),
+                '[source:file.py#L456](/p/project/tool/HEAD/tree/file.py#l456)')
+
+
+class TestPatternReplacingProcessor(unittest.TestCase):
+    @mock.patch('allura.lib.markdown_extensions.M.Shortlink.lookup')
+    def test_run(self, lookup):
+        shortlink = mock.Mock(url='/p/project/tool/artifact')
+        shortlink.ref.artifact.deleted = False
+        lookup.return_value = shortlink
+        p = mde.PatternReplacingProcessor(mde.TracRef1(), mde.TracRef2())
+        res = p.run(['#100', 'ticket:100'])
+        self.assertEqual(res, [
+            '[#100](/p/project/tool/artifact)',
+            '[ticket:100](/p/project/tool/artifact)'])
+
+
+class TestCommitMessageExtension(unittest.TestCase):
+    @mock.patch('allura.lib.markdown_extensions.M.Shortlink.lookup')
+    def test_convert(self, lookup):
+        from allura.lib.app_globals import ForgeMarkdown
+
+        shortlink = mock.Mock(url='/p/project/tool/artifact')
+        shortlink.ref.artifact.deleted = False
+        lookup.return_value = shortlink
+        app = mock.Mock(url='/p/project/tool/')
+
+        text = """\
+# Not A Heading #
+---
+* #100, r2
+* ticket:100
+* comment:13:ticket:2
+* source:test.py@2#L3
+
+Not *strong* or _underlined_."""
+
+        expected_html = """\
+<div class="markdown_content"><p># Not A Heading #<br />
+---<br />
+* <a href="/p/project/tool/artifact">#100</a>, <a href="/p/project/tool/artifact">r2</a><br
/>
+* <a href="/p/project/tool/artifact">ticket:100</a><br />
+* <a href="/p/project/tool/artifact">comment:13:ticket:2</a><br />
+* <a href="/p/project/tool/2/tree/test.py#l3">source:test.py@2#L3</a></p>
+<p>Not *strong* or _underlined_.</p></div>"""
+
+        md = ForgeMarkdown(extensions=[mde.CommitMessageExtension(app), 'nl2br'],
+                output_format='html4')
+        self.assertEqual(md.convert(text), expected_html)


Mime
View raw message