[IMP] runbot_merge: split out content after separator when creating commit

Currently when creating *merges* the commit message is the
concatenation of the PR title and the PR body.

However it can be convenient to include description test which would
not be included in the commit message e.g. PR template
stuff. "Thematic breaks" seem like a good way to do this: the commit
message will only include the content preceding the first thematic
break, everything else is ignored (except headings which are not
ignored, double negations FTW).

Might be that that's a bit excessive and we should only ignore what
follows the *last* thematic break. Or that there needs to be a more
specific marker. We'll see.

Fixes #443.

runbot_merge/models/pull_requests.py

@@ -2041,19 +2041,62 @@ def parse_refs_smart(read):
         m = refline.match(line)
         yield m[1].decode(), m[2].decode()
 
+BREAK = re.compile(r'''
+    ^
+    [ ]{0,3} # 0-3 spaces of indentation
+    # followed by a sequence of three or more matching -, _, or * characters,
+    # each followed optionally by any number of spaces or tabs
+    # so needs to start with a _, - or *, then have at least 2 more such
+    # interspersed with any number of spaces or tabs
+    ([*_-])
+    ([ \t]*\1){2,}
+    [ \t]*
+    $
+''', flags=re.VERBOSE)
+SETEX_UNDERLINE = re.compile(r'''
+    ^
+    [ ]{0,3} # no more than 3 spaces indentation
+    [-=]+ # a sequence of = characters or a sequence of - characters
+    [ ]* # any number of trailing spaces
+    $
+    # we don't care about "a line containing a single -" because we want to
+    # disambiguate SETEX headings from thematic breaks, and thematic breaks have
+    # 3+ -. Doesn't look like GH interprets `- - -` as a line so yay...
+''', flags=re.VERBOSE)
 HEADER = re.compile('^([A-Za-z-]+): (.*)$')
 class Message:
     @classmethod
     def from_message(cls, msg):
         in_headers = True
+        maybe_setex = None
         headers = []
         body = []
         for line in reversed(msg.splitlines()):
+            if maybe_setex:
+                # NOTE: actually slightly more complicated: it's a SETEX heading
+                #       only if preceding line(s) can be interpreted as a
+                #       paragraph so e.g. a title followed by a line of dashes
+                #       would indeed be a break, but this should be good enough
+                #       for now, if we need more we'll need a full-blown
+                #       markdown parser probably
+                if line: # actually a SETEX title -> add underline to body then process current
+                    body.append(maybe_setex)
+                else: # actually break, remove body then process current
+                    body = []
+                maybe_setex = None
+
             if not line:
                 if not in_headers and body and body[-1]:
                     body.append(line)
                 continue
 
+            if BREAK.match(line):
+                if SETEX_UNDERLINE.match(line):
+                    maybe_setex = line
+                else:
+                    body = []
+                continue
+
             h = HEADER.match(line)
             if h:
                 # c-a-b = special case from an existing test, not sure if actually useful?

runbot_merge/tests/test_basic.py

@@ -2,6 +2,7 @@ import datetime
 import itertools
 import json
 import re
+import textwrap
 from unittest import mock
 
 import pytest
@@ -1632,6 +1633,89 @@ class TestMergeMethod:
                         '\n\nSigned-off-by: {}'.format(repo.name, prx.number, reviewer), m, c0)
         assert log_to_node(repo.log('heads/master')), expected
 
+    @pytest.mark.parametrize('separator', [
+        '***', '___', '\n---',
+        '*'*12, '\n----------------',
+        '- - -', '  **     **     **'
+    ])
+    def test_pr_message_break(self, repo, env, users, config, separator):
+        """ If the PR message contains a "thematic break", only the part before
+        should be included in the merge commit's message.
+        """
+        reviewer = get_partner(env, users["reviewer"]).formatted_email
+        with repo:
+            root = repo.make_commits(None, Commit("root", tree={'a': 'a'}), ref='heads/master')
+
+            repo.make_commits(root, Commit('C', tree={'a': 'b'}), ref=f'heads/change')
+            pr = repo.make_pr(title="title", body=f'first\n{separator}\nsecond',
+                              target='master', head=f'change')
+            repo.post_status(pr.head, 'success', 'legal/cla')
+            repo.post_status(pr.head, 'success', 'ci/runbot')
+            pr.post_comment('hansen r+ merge', config['role_reviewer']['token'])
+        env.run_crons()
+
+        with repo:
+            repo.post_status('heads/staging.master', 'success', 'ci/runbot')
+            repo.post_status('heads/staging.master', 'success', 'legal/cla')
+        env.run_crons()
+
+        head = repo.commit('heads/master')
+        assert head.message == textwrap.dedent(f"""\
+        title
+
+        first
+
+        closes {repo.name}#{pr.number}
+
+        Signed-off-by: {reviewer}
+        """).strip(), "should not contain the content which follows the thematic break"
+
+    def test_pr_message_setex_title(self, repo, env, users, config):
+        """ should not break on a proper SETEX-style title """
+        reviewer = get_partner(env, users["reviewer"]).formatted_email
+        with repo:
+            root = repo.make_commits(None, Commit("root", tree={'a': 'a'}), ref='heads/master')
+
+            repo.make_commits(root, Commit('C', tree={'a': 'b'}), ref=f'heads/change')
+            pr = repo.make_pr(title="title", body="""\
+Title
+---
+This is some text
+
+Title 2
+-------
+This is more text
+***
+removed
+""",
+                              target='master', head=f'change')
+            repo.post_status(pr.head, 'success', 'legal/cla')
+            repo.post_status(pr.head, 'success', 'ci/runbot')
+            pr.post_comment('hansen r+ merge', config['role_reviewer']['token'])
+        env.run_crons()
+
+        with repo:
+            repo.post_status('heads/staging.master', 'success', 'ci/runbot')
+            repo.post_status('heads/staging.master', 'success', 'legal/cla')
+        env.run_crons()
+
+        head = repo.commit('heads/master')
+        assert head.message == textwrap.dedent(f"""\
+        title
+
+        Title
+        ---
+        This is some text
+
+        Title 2
+        -------
+        This is more text
+
+        closes {repo.name}#{pr.number}
+
+        Signed-off-by: {reviewer}
+        """).strip(), "should not break the SETEX titles"
+
     def test_pr_mergehead(self, repo, env, config):
         """ if the head of the PR is a merge commit and one of the parents is
         in the target, replicate the merge commit instead of merging