Merge lp:~blr/launchpad/bug-1472045-demangle-inlinecomment-mail into lp:launchpad

[Kit Randel]

Ensure blank lines and git dirty headers (diff, index) are added to dirty_head.

[Kit Randel (blr)]

While I believe this does resolve the issue (the blankline before the dirty header is discarded by bzrlib.patches.parse), it does make the assumption that our diffs will continue to be rendered consistently in the form:

=== dirty header
--- patch header
+++ patch header
@ hunk header @
text
text
text

=== dirty header
etc.

I tested this branch against a very large real-world diff (elmo's) and it appeared to be behaving.

Colin, you suggested an approach which may work better for differing cases (git?), however I didn't entirely understand and would need to discuss it with you further. Shall we land this in the meantime to get things functional and look at refactoring? I can't say I love this code.

[Kit Randel (blr)]

Reverted the blank line handling in codereviewcomment, will attempt to handle in patches.

[Kit Randel (blr)]

Have cloned a local copy of bzrlib.patches with support for parsing dirty headers from git, and respecting blank lines preceding dirty headers.

Have added an additional test which ensures blank lines within hunks are still handled appropriately.

While this code could (and perhaps should) be refactored to handle all general cases of dirty headers, this does appear to work and will fix the broken behaviour in production.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/mail/codereviewcomment.py'
--- lib/lp/code/mail/codereviewcomment.py	2015-07-07 05:32:11 +0000
+++ lib/lp/code/mail/codereviewcomment.py	2015-07-09 05:40:58 +0000
@@ -10,7 +10,6 @@
     'CodeReviewCommentMailer',
     ]
 
-from bzrlib import patches
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -21,6 +20,7 @@
 from lp.code.interfaces.codereviewinlinecomment import (
     ICodeReviewInlineCommentSet,
     )
+from lp.code.mail import patches
 from lp.code.mail.branchmergeproposal import BMPMailer
 from lp.services.mail.sendmail import (
     append_footer,

=== added file 'lib/lp/code/mail/patches.py'
--- lib/lp/code/mail/patches.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/mail/patches.py	2015-07-09 05:40:58 +0000
@@ -0,0 +1,513 @@
+# This file was cloned from bzr-2.6.0-lp-3 (bzrlib.patches) and
+# customised for LP.
+#
+# Copyright (C) 2005-2010 Aaron Bentley, Canonical Ltd
+# <[e-mail]>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+from __future__ import absolute_import
+
+from bzrlib.errors import (
+    BinaryFiles,
+    MalformedHunkHeader,
+    MalformedLine,
+    MalformedPatchHeader,
+    PatchConflict,
+    PatchSyntax,
+    )
+
+import re
+
+
+binary_files_re = 'Binary files (.*) and (.*) differ\n'
+
+
+def get_patch_names(iter_lines):
+    line = iter_lines.next()
+    try:
+        match = re.match(binary_files_re, line)
+        if match is not None:
+            raise BinaryFiles(match.group(1), match.group(2))
+        if not line.startswith("--- "):
+            raise MalformedPatchHeader("No orig name", line)
+        else:
+            orig_name = line[4:].rstrip("\n")
+    except StopIteration:
+        raise MalformedPatchHeader("No orig line", "")
+    try:
+        line = iter_lines.next()
+        if not line.startswith("+++ "):
+            raise PatchSyntax("No mod name")
+        else:
+            mod_name = line[4:].rstrip("\n")
+    except StopIteration:
+        raise MalformedPatchHeader("No mod line", "")
+    return (orig_name, mod_name)
+
+
+def parse_range(textrange):
+    """Parse a patch range, handling the "1" special-case
+
+    :param textrange: The text to parse
+    :type textrange: str
+    :return: the position and range, as a tuple
+    :rtype: (int, int)
+    """
+    tmp = textrange.split(',')
+    if len(tmp) == 1:
+        pos = tmp[0]
+        range = "1"
+    else:
+        (pos, range) = tmp
+    pos = int(pos)
+    range = int(range)
+    return (pos, range)
+
+
+def hunk_from_header(line):
+    import re
+    matches = re.match(r'\@\@ ([^@]*) \@\@( (.*))?\n', line)
+    if matches is None:
+        raise MalformedHunkHeader("Does not match format.", line)
+    try:
+        (orig, mod) = matches.group(1).split(" ")
+    except (ValueError, IndexError), e:
+        raise MalformedHunkHeader(str(e), line)
+    if not orig.startswith('-') or not mod.startswith('+'):
+        raise MalformedHunkHeader("Positions don't start with + or -.", line)
+    try:
+        (orig_pos, orig_range) = parse_range(orig[1:])
+        (mod_pos, mod_range) = parse_range(mod[1:])
+    except (ValueError, IndexError), e:
+        raise MalformedHunkHeader(str(e), line)
+    if mod_range < 0 or orig_range < 0:
+        raise MalformedHunkHeader("Hunk range is negative", line)
+    tail = matches.group(3)
+    return Hunk(orig_pos, orig_range, mod_pos, mod_range, tail)
+
+
+class HunkLine:
+    def __init__(self, contents):
+        self.contents = contents
+
+    def get_str(self, leadchar):
+        if self.contents == "\n" and leadchar == " " and False:
+            return "\n"
+        if not self.contents.endswith('\n'):
+            terminator = '\n' + NO_NL
+        else:
+            terminator = ''
+        return leadchar + self.contents + terminator
+
+
+class ContextLine(HunkLine):
+    def __init__(self, contents):
+        HunkLine.__init__(self, contents)
+
+    def __str__(self):
+        return self.get_str(" ")
+
+
+class InsertLine(HunkLine):
+    def __init__(self, contents):
+        HunkLine.__init__(self, contents)
+
+    def __str__(self):
+        return self.get_str("+")
+
+
+class RemoveLine(HunkLine):
+    def __init__(self, contents):
+        HunkLine.__init__(self, contents)
+
+    def __str__(self):
+        return self.get_str("-")
+
+NO_NL = '\\ No newline at end of file\n'
+__pychecker__ = "no-returnvalues"
+
+
+def parse_line(line):
+    if line.startswith("\n"):
+        return ContextLine(line)
+    elif line.startswith(" "):
+        return ContextLine(line[1:])
+    elif line.startswith("+"):
+        return InsertLine(line[1:])
+    elif line.startswith("-"):
+        return RemoveLine(line[1:])
+    else:
+        raise MalformedLine("Unknown line type", line)
+__pychecker__ = ""
+
+
+class Hunk:
+    def __init__(self, orig_pos, orig_range, mod_pos, mod_range, tail=None):
+        self.orig_pos = orig_pos
+        self.orig_range = orig_range
+        self.mod_pos = mod_pos
+        self.mod_range = mod_range
+        self.tail = tail
+        self.lines = []
+
+    def get_header(self):
+        if self.tail is None:
+            tail_str = ''
+        else:
+            tail_str = ' ' + self.tail
+        return "@@ -%s +%s @@%s\n" % (self.range_str(self.orig_pos,
+                                                     self.orig_range),
+                                      self.range_str(self.mod_pos,
+                                                     self.mod_range),
+                                      tail_str)
+
+    def range_str(self, pos, range):
+        """Return a file range, special-casing for 1-line files.
+
+        :param pos: The position in the file
+        :type pos: int
+        :range: The range in the file
+        :type range: int
+        :return: a string in the format 1,4 except when range == pos == 1
+        """
+        if range == 1:
+            return "%i" % pos
+        else:
+            return "%i,%i" % (pos, range)
+
+    def __str__(self):
+        lines = [self.get_header()]
+        for line in self.lines:
+            lines.append(str(line))
+        return "".join(lines)
+
+    def shift_to_mod(self, pos):
+        if pos < self.orig_pos - 1:
+            return 0
+        elif pos > self.orig_pos + self.orig_range:
+            return self.mod_range - self.orig_range
+        else:
+            return self.shift_to_mod_lines(pos)
+
+    def shift_to_mod_lines(self, pos):
+        position = self.orig_pos - 1
+        shift = 0
+        for line in self.lines:
+            if isinstance(line, InsertLine):
+                shift += 1
+            elif isinstance(line, RemoveLine):
+                if position == pos:
+                    return None
+                shift -= 1
+                position += 1
+            elif isinstance(line, ContextLine):
+                position += 1
+            if position > pos:
+                break
+        return shift
+
+
+def iter_hunks(iter_lines, allow_dirty=False):
+    '''
+    :arg iter_lines: iterable of lines to parse for hunks
+    :kwarg allow_dirty: If True, when we encounter something that is not
+        a hunk header when we're looking for one, assume the rest of the lines
+        are not part of the patch (comments or other junk).  Default False
+    '''
+    hunk = None
+    for line in iter_lines:
+        if line == "\n":
+            if hunk is not None:
+                yield hunk
+                hunk = None
+            continue
+        if hunk is not None:
+            yield hunk
+        try:
+            hunk = hunk_from_header(line)
+        except MalformedHunkHeader:
+            if allow_dirty:
+                # If the line isn't a hunk header, then we've reached the end
+                # of this patch and there's "junk" at the end.  Ignore the
+                # rest of this patch.
+                return
+            raise
+        orig_size = 0
+        mod_size = 0
+        while orig_size < hunk.orig_range or mod_size < hunk.mod_range:
+            hunk_line = parse_line(iter_lines.next())
+            hunk.lines.append(hunk_line)
+            if isinstance(hunk_line, (RemoveLine, ContextLine)):
+                orig_size += 1
+            if isinstance(hunk_line, (InsertLine, ContextLine)):
+                mod_size += 1
+    if hunk is not None:
+        yield hunk
+
+
+class BinaryPatch(object):
+    def __init__(self, oldname, newname):
+        self.oldname = oldname
+        self.newname = newname
+
+    def __str__(self):
+        return 'Binary files %s and %s differ\n' % (self.oldname, self.newname)
+
+
+class Patch(BinaryPatch):
+
+    def __init__(self, oldname, newname):
+        BinaryPatch.__init__(self, oldname, newname)
+        self.hunks = []
+
+    def __str__(self):
+        ret = self.get_header()
+        ret += "".join([str(h) for h in self.hunks])
+        return ret
+
+    def get_header(self):
+        return "--- %s\n+++ %s\n" % (self.oldname, self.newname)
+
+    def stats_values(self):
+        """Calculate the number of inserts and removes."""
+        removes = 0
+        inserts = 0
+        for hunk in self.hunks:
+            for line in hunk.lines:
+                if isinstance(line, InsertLine):
+                    inserts += 1
+                elif isinstance(line, RemoveLine):
+                    removes += 1
+        return (inserts, removes, len(self.hunks))
+
+    def stats_str(self):
+        """Return a string of patch statistics"""
+        return "%i inserts, %i removes in %i hunks" % \
+            self.stats_values()
+
+    def pos_in_mod(self, position):
+        newpos = position
+        for hunk in self.hunks:
+            shift = hunk.shift_to_mod(position)
+            if shift is None:
+                return None
+            newpos += shift
+        return newpos
+
+    def iter_inserted(self):
+        """Iteraties through inserted lines
+
+        :return: Pair of line number, line
+        :rtype: iterator of (int, InsertLine)
+        """
+        for hunk in self.hunks:
+            pos = hunk.mod_pos - 1
+            for line in hunk.lines:
+                if isinstance(line, InsertLine):
+                    yield (pos, line)
+                    pos += 1
+                if isinstance(line, ContextLine):
+                    pos += 1
+
+
+def parse_patch(iter_lines, allow_dirty=False):
+    '''
+    :arg iter_lines: iterable of lines to parse
+    :kwarg allow_dirty: If True, allow the patch to have trailing junk.
+        Default False
+    '''
+    iter_lines = iter_lines_handle_nl(iter_lines)
+    try:
+        (orig_name, mod_name) = get_patch_names(iter_lines)
+    except BinaryFiles, e:
+        return BinaryPatch(e.orig_name, e.mod_name)
+    else:
+        patch = Patch(orig_name, mod_name)
+        for hunk in iter_hunks(iter_lines, allow_dirty):
+            patch.hunks.append(hunk)
+        return patch
+
+
+def iter_file_patch(iter_lines, allow_dirty=False, keep_dirty=False):
+    '''
+    :arg iter_lines: iterable of lines to parse for patches
+    :kwarg allow_dirty: If True, allow comments and other non-patch text
+        before the first patch.  Note that the algorithm here can only find
+        such text before any patches have been found.  Comments after the
+        first patch are stripped away in iter_hunks() if it is also passed
+        allow_dirty=True.  Default False.
+    '''
+    # FIXME: Docstring is not quite true.  We allow certain comments no
+    # matter what, If they startwith '===', '***', or '#' Someone should
+    # reexamine this logic and decide if we should include those in
+    # allow_dirty or restrict those to only being before the patch is found
+    # (as allow_dirty does).
+    regex = re.compile(binary_files_re)
+    saved_lines = []
+    dirty_head = []
+    orig_range = 0
+    beginning = True
+
+    dirty_headers = ('=== ', 'diff ', 'index ')
+    for line in iter_lines:
+        # preserve bzr modified/added headers and blank lines
+        if line.startswith(dirty_headers) or not line.strip('\n'):
+            if len(saved_lines) > 0:
+                if keep_dirty and len(dirty_head) > 0:
+                    yield {'saved_lines': saved_lines,
+                           'dirty_head': dirty_head}
+                    dirty_head = []
+                else:
+                    yield saved_lines
+                saved_lines = []
+            dirty_head.append(line)
+            continue
+        if line.startswith('*** '):
+            continue
+        if line.startswith('#'):
+            continue
+        elif orig_range > 0:
+            if line.startswith('-') or line.startswith(' '):
+                orig_range -= 1
+        elif line.startswith('--- ') or regex.match(line):
+            if allow_dirty and beginning:
+                # Patches can have "junk" at the beginning
+                # Stripping junk from the end of patches is handled when we
+                # parse the patch
+                beginning = False
+            elif len(saved_lines) > 0:
+                if keep_dirty and len(dirty_head) > 0:
+                    yield {'saved_lines': saved_lines,
+                           'dirty_head': dirty_head}
+                    dirty_head = []
+                else:
+                    yield saved_lines
+            saved_lines = []
+        elif line.startswith('@@'):
+            hunk = hunk_from_header(line)
+            orig_range = hunk.orig_range
+        saved_lines.append(line)
+    if len(saved_lines) > 0:
+        if keep_dirty and len(dirty_head) > 0:
+            yield {'saved_lines': saved_lines,
+                   'dirty_head': dirty_head}
+        else:
+            yield saved_lines
+
+
+def iter_lines_handle_nl(iter_lines):
+    """
+    Iterates through lines, ensuring that lines that originally had no
+    terminating \n are produced without one.  This transformation may be
+    applied at any point up until hunk line parsing, and is safe to apply
+    repeatedly.
+    """
+    last_line = None
+    for line in iter_lines:
+        if line == NO_NL:
+            if not last_line.endswith('\n'):
+                raise AssertionError()
+            last_line = last_line[:-1]
+            line = None
+        if last_line is not None:
+            yield last_line
+        last_line = line
+    if last_line is not None:
+        yield last_line
+
+
+def parse_patches(iter_lines, allow_dirty=False, keep_dirty=False):
+    '''
+    :arg iter_lines: iterable of lines to parse for patches
+    :kwarg allow_dirty: If True, allow text that's not part of the patch at
+        selected places.  This includes comments before and after a patch
+        for instance.  Default False.
+    :kwarg keep_dirty: If True, returns a dict of patches with dirty headers.
+        Default False.
+    '''
+    patches = []
+    for patch_lines in iter_file_patch(iter_lines, allow_dirty, keep_dirty):
+        if 'dirty_head' in patch_lines:
+            patches.append({'patch': parse_patch(
+                patch_lines['saved_lines'], allow_dirty),
+                'dirty_head': patch_lines['dirty_head']})
+        else:
+            patches.append(parse_patch(patch_lines, allow_dirty))
+    return patches
+
+
+def difference_index(atext, btext):
+    """Find the indext of the first character that differs between two texts
+
+    :param atext: The first text
+    :type atext: str
+    :param btext: The second text
+    :type str: str
+    :return: The index, or None if there are no differences within the range
+    :rtype: int or NoneType
+    """
+    length = len(atext)
+    if len(btext) < length:
+        length = len(btext)
+    for i in range(length):
+        if atext[i] != btext[i]:
+            return i
+    return None
+
+
+def iter_patched(orig_lines, patch_lines):
+    """Iterate through a series of lines with a patch applied.
+    This handles a single file, and does exact, not fuzzy patching.
+    """
+    patch_lines = iter_lines_handle_nl(iter(patch_lines))
+    get_patch_names(patch_lines)
+    return iter_patched_from_hunks(orig_lines, iter_hunks(patch_lines))
+
+
+def iter_patched_from_hunks(orig_lines, hunks):
+    """Iterate through a series of lines with a patch applied.
+    This handles a single file, and does exact, not fuzzy patching.
+
+    :param orig_lines: The unpatched lines.
+    :param hunks: An iterable of Hunk instances.
+    """
+    seen_patch = []
+    line_no = 1
+    if orig_lines is not None:
+        orig_lines = iter(orig_lines)
+    for hunk in hunks:
+        while line_no < hunk.orig_pos:
+            orig_line = orig_lines.next()
+            yield orig_line
+            line_no += 1
+        for hunk_line in hunk.lines:
+            seen_patch.append(str(hunk_line))
+            if isinstance(hunk_line, InsertLine):
+                yield hunk_line.contents
+            elif isinstance(hunk_line, (ContextLine, RemoveLine)):
+                orig_line = orig_lines.next()
+                if orig_line != hunk_line.contents:
+                    raise PatchConflict(line_no, orig_line,
+                                        "".join(seen_patch))
+                if isinstance(hunk_line, ContextLine):
+                    yield orig_line
+                else:
+                    if not isinstance(hunk_line, RemoveLine):
+                        raise AssertionError(hunk_line)
+                line_no += 1
+    if orig_lines is not None:
+        for line in orig_lines:
+            yield line

=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
--- lib/lp/code/mail/tests/test_codereviewcomment.py	2015-07-07 05:32:11 +0000
+++ lib/lp/code/mail/tests/test_codereviewcomment.py	2015-07-09 05:40:58 +0000
@@ -3,7 +3,6 @@
 
 """Test CodeReviewComment emailing functionality."""
 
-
 import testtools
 import transaction
 from zope.component import getUtility
@@ -382,9 +381,9 @@
 
     diff_text = (
         "=== added directory 'foo/bar'\n"
-        "=== modified file 'foo/bar/baz.py'\n"
-        "--- bar\t2009-08-26 15:53:34.000000000 -0400\n"
-        "+++ bar\t1969-12-31 19:00:00.000000000 -0500\n"
+        "=== modified file 'foo/bar/bar.py'\n"
+        "--- bar.py\t2009-08-26 15:53:34.000000000 -0400\n"
+        "+++ bar.py\t1969-12-31 19:00:00.000000000 -0500\n"
         "@@ -1,3 +0,0 @@\n"
         "-\xc3\xa5\n"
         "-b\n"
@@ -404,7 +403,35 @@
         "-b\n"
         " c\n"
         "+d\n"
-        "+e\n")
+        "+e\n"
+        "\n"
+        "=== modified file 'fulango.py'\n"
+        "--- fulano.py\t2014-08-26 15:53:34.000000000 -0400\n"
+        "+++ fulano.py\t2015-12-31 19:00:00.000000000 -0500\n"
+        "@@ -1,3 +1,4 @@\n"
+        " a\n"
+        "-fulano\n"
+        " c\n"
+        "+mengano\n"
+        "+zutano\n")
+
+    git_diff_text = (
+        "diff --git a/foo b/foo\n"
+        "index 5716ca5..7601807 100644\n"
+        "--- a/foo\n"
+        "+++ b/foo\n"
+        "@@ -1 +1 @@\n"
+        "-bar\n"
+        "+baz\n"
+        "diff --git a/fulano b/fulano\n"
+        "index 5716ca5..7601807 100644\n"
+        "--- a/fulano\n"
+        "+++ b/fulano\n"
+        "@@ -1,3 +1,3 @@\n"
+        " fulano\n"
+        " \n"
+        "-mengano\n"
+        "+zutano\n")
 
     binary_diff_text = (
         "=== added file 'lib/canonical/launchpad/images/foo.png'\n"
@@ -412,9 +439,10 @@
         "1970-01-01 00:00:00 +0000 and "
         "lib/canonical/launchpad/images/foo.png\t"
         "2015-06-21 22:07:50 +0000 differ\n"
-        "=== modified file 'foo/bar/baz.py'\n"
-        "--- bar\t2009-08-26 15:53:34.000000000 -0400\n"
-        "+++ bar\t1969-12-31 19:00:00.000000000 -0500\n"
+        "\n"
+        "=== modified file 'foo/bar/bar.py'\n"
+        "--- bar.py\t2009-08-26 15:53:34.000000000 -0400\n"
+        "+++ bar.py\t1969-12-31 19:00:00.000000000 -0500\n"
         "@@ -1,3 +0,0 @@\n"
         "-a\n"
         "-b\n"
@@ -443,7 +471,7 @@
 
     def test_binary_patch_in_diff(self):
         # Binary patches with comments are handled appropriately.
-        comments = {'1': 'Updated the png', '2': 'foo', '8': 'bar'}
+        comments = {'1': 'Updated the png', '2': 'foo', '9': 'bar'}
         section = self.getSection(comments, diff_text=self.binary_diff_text)
         self.assertEqual(
             map(unicode, [
@@ -458,9 +486,10 @@
                 "",
                 "foo",
                 "",
-                "> === modified file 'foo/bar/baz.py'",
-                "> --- bar\t2009-08-26 15:53:34.000000000 -0400",
-                "> +++ bar\t1969-12-31 19:00:00.000000000 -0500",
+                "> ",
+                "> === modified file 'foo/bar/bar.py'",
+                "> --- bar.py\t2009-08-26 15:53:34.000000000 -0400",
+                "> +++ bar.py\t1969-12-31 19:00:00.000000000 -0500",
                 "> @@ -1,3 +0,0 @@",
                 "> -a",
                 "> -b",
@@ -468,7 +497,7 @@
                 "bar",
                 "",
                 "> -c"]),
-            section.splitlines()[4:22])
+            section.splitlines()[4:23])
 
     def test_single_line_comment(self):
         # The inline comments are correctly contextualized in the diff.
@@ -476,12 +505,44 @@
         comments = {'4': '\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae'}
         self.assertEqual(
             map(unicode, [
-                '> +++ bar\t1969-12-31 19:00:00.000000000 -0500',
+                '> +++ bar.py\t1969-12-31 19:00:00.000000000 -0500',
                 '',
                 '\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae',
                 '']),
             self.getSection(comments).splitlines()[7:11])
 
+    def test_comments_in_git_diff(self):
+        comments = {'1': 'foo', '5': 'bar', '15': 'baz'}
+        section = self.getSection(comments, diff_text=self.git_diff_text)
+        self.assertEqual(
+            map(unicode, [
+                "> diff --git a/foo b/foo",
+                "",
+                "foo",
+                "",
+                "> index 5716ca5..7601807 100644",
+                "> --- a/foo",
+                "> +++ b/foo",
+                "> @@ -1 +1 @@",
+                "",
+                "bar",
+                "",
+                "> -bar",
+                "> +baz",
+                "> diff --git a/fulano b/fulano",
+                "> index 5716ca5..7601807 100644",
+                "> --- a/fulano",
+                "> +++ b/fulano",
+                "> @@ -1,3 +1,3 @@",
+                ">  fulano",
+                ">  ",
+                "> -mengano",
+                "",
+                "baz",
+                "",
+                "> +zutano"]),
+            section.splitlines()[4:29])
+
     def test_commentless_hunks_ignored(self):
         # Hunks without inline comments are not returned in the diff text.
         comments = {'16': 'A comment', '21': 'Another comment'}
@@ -556,13 +617,32 @@
                 '> +b']),
             self.getSection(comments).splitlines()[4:12])
 
+    def test_comment_in_patch_after_linebreak(self):
+        comments = {'31': 'que?'}
+        self.assertEqual(
+            map(unicode, [
+                "> ",
+                "> === modified file 'fulango.py'",
+                "> --- fulano.py\t2014-08-26 15:53:34.000000000 -0400",
+                "> +++ fulano.py\t2015-12-31 19:00:00.000000000 -0500",
+                "> @@ -1,3 +1,4 @@",
+                ">  a",
+                "> -fulano",
+                "",
+                "que?",
+                "",
+                ">  c",
+                "> +mengano",
+                "> +zutano"]),
+            self.getSection(comments).splitlines()[4:17])
+
     def test_multi_line_comment(self):
         # Inline comments with multiple lines are rendered appropriately.
         comments = {'4': 'Foo\nBar'}
         self.assertEqual(
             map(unicode, [
-                '> --- bar\t2009-08-26 15:53:34.000000000 -0400',
-                '> +++ bar\t1969-12-31 19:00:00.000000000 -0500',
+                '> --- bar.py\t2009-08-26 15:53:34.000000000 -0400',
+                '> +++ bar.py\t1969-12-31 19:00:00.000000000 -0500',
                 '',
                 'Foo',
                 'Bar',
@@ -573,7 +653,7 @@
         # Multiple inline comments are redered appropriately.
         comments = {'4': 'Foo', '5': 'Bar'}
         self.assertEqual(
-            ['> +++ bar\t1969-12-31 19:00:00.000000000 -0500',
+            ['> +++ bar.py\t1969-12-31 19:00:00.000000000 -0500',
              '',
              'Foo',
              '',