Let git_footers split final paragraphs in specific cases

Suppose the final paragraph of a commit message looks like this:
    """
    And here's the final paragraph.
    Bug: 1234
    TBR=soandso
    Change-Id: deadbeef
    """
In this case, we don't want to lose the Bug and Change-Id footers,
so we process the whole final paragraph. *But* we'd also like to
help the user get things formatted correctly. This change lets
git_footers notice this situation, and insert a newline before the
first well-formed footer (Bug: in this case), so that the set of
well- and mal-formed footers are separated from the rest of the
malformed body text.

In the rare case where the last line of the last non-trailer paragraph
is a url, this will also visibly push the url into the block of
trailers (where it doesn't belong), prompting the user to fix it.
A more comprehensive fix for that particular case is coming later.

Bug: 766234
Change-Id: I6ae0072fff68ddf06e6f43b70f9a82a7f247f4ab
Reviewed-on: https://chromium-review.googlesource.com/849481
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>

git_footers.py

@@ -57,17 +57,27 @@ def split_footers(message):
   """Returns (non_footer_lines, footer_lines, parsed footers).
 
   Guarantees that:
-    (non_footer_lines + footer_lines) == message.splitlines().
+    (non_footer_lines + footer_lines) ~= message.splitlines(), with at
+      most one new newline, if the last paragraph is text followed by footers.
     parsed_footers is parse_footer applied on each line of footer_lines.
       There could be fewer parsed_footers than footer lines if some lines in
       last paragraph are malformed.
   """
   message_lines = list(message.splitlines())
   footer_lines = []
+  maybe_footer_lines = []
   for line in reversed(message_lines):
     if line == '' or line.isspace():
       break
-    footer_lines.append(line)
+    elif parse_footer(line):
+      footer_lines.extend(maybe_footer_lines)
+      maybe_footer_lines = []
+      footer_lines.append(line)
+    else:
+      # We only want to include malformed lines if they are preceeded by
+      # well-formed lines. So keep them in holding until we see a well-formed
+      # line (case above).
+      maybe_footer_lines.append(line)
   else:
     # The whole description was consisting of footers,
     # which means those aren't footers.
@@ -77,6 +87,10 @@ def split_footers(message):
   footers = filter(None, map(parse_footer, footer_lines))
   if not footers:
     return message_lines, [], []
+  if maybe_footer_lines:
+    # If some malformed lines were left over, add a newline to split them
+    # from the well-formed ones.
+    return message_lines[:-len(footer_lines)] + [''], footer_lines, footers
   return message_lines[:-len(footer_lines)], footer_lines, footers
 
 

tests/git_footers_test.py

@@ -107,6 +107,27 @@ My commit message is my best friend. It is my life. I must master it.
                      {'Followed': ['by'],
                       'Some': ['footers']})
 
+  def testSplittingLastParagraph(self):
+    message = ('Title.\n'
+               '\n'
+               'The final paragraph has some normal text first.\n'
+               'Followed: by\n'
+               'nonsense trailers and\n'
+               'Some: footers')
+    self.assertEqual(git_footers.split_footers(message),
+                     (['Title.',
+                       '',
+                       'The final paragraph has some normal text first.',
+                       ''],
+                      ['Followed: by',
+                       'nonsense trailers and',
+                       'Some: footers'],
+                      [('Followed', 'by'),
+                       ('Some', 'footers')]))
+    self.assertEqual(git_footers.parse_footers(message),
+                     {'Followed': ['by'],
+                      'Some': ['footers']})
+
   def testGetFooterChangeId(self):
     msg = '\n'.join(['whatever',
                      '',