check-commit: try to fix multiline handling

The old code did not understand the difference between the first line of the summary,
and a random line in the summary that happened to include a #, or a
random line in the changes that happened to include it.

965788d9ae09 is an example where it fails

contrib/check-commit

@@ -1,74 +1,91 b''
 #!/usr/bin/env python
 #
 # Copyright 2014 Matt Mackall <[e-mail]>
 #
 # A tool/hook to run basic sanity checks on commits/patches for
 # submission to Mercurial. Install by adding the following to your
 # .hg/hgrc:
 #
 # [hooks]
 # pretxncommit = contrib/check-commit
 #
 # The hook can be temporarily bypassed with:
 #
 # $ BYPASS= hg commit
 #
 # See also: https://mercurial-scm.org/wiki/ContributingChanges
 
 import re, sys, os
 
+commitheader = r"^(?:# [^\n]*\n)*"
+afterheader = commitheader + r"(?!#)"
+beforepatch = afterheader + r"(?!\n(?!@@))"
+
 errors = [
-    (r"[(]bc[)]", "(BC) needs to be uppercase"),
-    (r"[(]issue \d\d\d", "no space allowed between issue and number"),
-    (r"[(]bug(\d|\s)", "use (issueDDDD) instead of bug"),
-    (r"^# User [^@\n]+$", "username is not an email address"),
-    (r"^# .*\n(?!merge with )[^#]\S+[^:] ",
+    (beforepatch + r".*[(]bc[)]", "(BC) needs to be uppercase"),
+    (beforepatch + r".*[(]issue \d\d\d", "no space allowed between issue and number"),
+    (beforepatch + r".*[(]bug(\d|\s)", "use (issueDDDD) instead of bug"),
+    (commitheader + r"# User [^@\n]+\n", "username is not an email address"),
+    (commitheader + r"(?!merge with )[^#]\S+[^:] ",
      "summary line doesn't start with 'topic: '"),
-    (r"^# .*\n[A-Z][a-z]\S+", "don't capitalize summary lines"),
-    (r"^# .*\n[^\n]*: *[A-Z][a-z]\S+", "don't capitalize summary lines"),
-    (r"^# [^\n]*\n\S*[^A-Za-z0-9-]\S*: ",
+    (afterheader + r"[A-Z][a-z]\S+", "don't capitalize summary lines"),
+    (afterheader + r"[^\n]*: *[A-Z][a-z]\S+", "don't capitalize summary lines"),
+    (afterheader + r"\S*[^A-Za-z0-9-]\S*: ",
      "summary keyword should be most user-relevant one-word command or topic"),
-    (r"^# .*\n.*\.\s+$", "don't add trailing period on summary line"),
-    (r"^# .*\n[^#].{78,}", "summary line too long (limit is 78)"),
-    (r"^\+\n \n", "adds double empty line"),
-    (r"^ \n\+\n", "adds double empty line"),
-    (r"^\+[ \t]+def [a-z]+_[a-z]", "adds a function with foo_bar naming"),
+    (afterheader + r".*\.\s*\n", "don't add trailing period on summary line"),
+    (afterheader + r".{79,}", "summary line too long (limit is 78)"),
+    (r"\n\+\n \n", "adds double empty line"),
+    (r"\n \n\+\n", "adds double empty line"),
+    (r"\n\+[ \t]+def [a-z]+_[a-z]", "adds a function with foo_bar naming"),
 ]
 
+word = re.compile('\S')
+def nonempty(first, second):
+    if word.search(first):
+        return first
+    return second
+
 def checkcommit(commit, node = None):
     exitcode = 0
     printed = node is None
     for exp, msg in errors:
-        m = re.search(exp, commit, re.MULTILINE)
+        m = re.search(exp, commit)
         if m:
             pos = 0
+            end = m.end()
+            trailing = re.search(r'(\\n)+$', exp)
+            if trailing:
+                end -= len(trailing.group()) / 2
+            last = ''
             for n, l in enumerate(commit.splitlines(True)):
                 pos += len(l)
-                if pos >= m.end():
+                if pos < end:
+                    last = nonempty(l, last)
+                else:
                     if not printed:
                         printed = True
                         print "node: %s" % node
                     print "%d: %s" % (n, msg)
-                    print " %s" % l[:-1]
+                    print " %s" % nonempty(l, last)[:-1]
                     if "BYPASS" not in os.environ:
                         exitcode = 1
                     break
     return exitcode
 
 def readcommit(node):
     return os.popen("hg export %s" % node).read()
 
 if __name__ == "__main__":
     exitcode = 0
     node = os.environ.get("HG_NODE")
 
     if node:
         commit = readcommit(node)
         exitcode = checkcommit(commit)
     elif sys.argv[1:]:
         for node in sys.argv[1:]:
             exitcode |= checkcommit(readcommit(node), node)
     else:
         commit = sys.stdin.read()
         exitcode = checkcommit(commit)
     sys.exit(exitcode)

tests/test-contrib-check-commit.t

@@ -1,111 +1,111 b''
 Test the 'check-commit' script
 ==============================
 
 A fine patch:
 
   $ cat > patch-with-long-header.diff << EOF
   > # HG changeset patch
   > # User timeless <[e-mail]>
   > # Date 1448911706 0
   > #      Mon Nov 30 19:28:26 2015 +0000
   > # Node ID c41cb6d2b7dbd62b1033727f8606b8c09fc4aa88
   > # Parent  42aa0e570eaa364a622bc4443b0bcb79b1100a58
   > # ClownJoke This is a veryly long header that should not be warned about because its not the description
   > bundle2: use Oxford comma (issue123) (BC)
   > 
   > diff --git a/hgext/transplant.py b/hgext/transplant.py
   > --- a/hgext/transplant.py
   > +++ b/hgext/transplant.py
   > @@ -599,7 +599,7 @@
   >              return
   >          if not (opts.get('source') or revs or
   >                  opts.get('merge') or opts.get('branch')):
   > -            raise error.Abort(_('no source URL, branch revision or revision '
   > +            raise error.Abort(_('no source URL, branch revision, or revision '
   >                                 'list provided'))
   >          if opts.get('all'):
   > 
   > + def blahblah(x):
   > +     pass
   > EOF
   $ cat patch-with-long-header.diff | $TESTDIR/../contrib/check-commit
 
 A patch with lots of errors:
 
   $ cat > patch-with-long-header.diff << EOF
   > # HG changeset patch
   > # User timeless
   > # Date 1448911706 0
   > #      Mon Nov 30 19:28:26 2015 +0000
   > # Node ID c41cb6d2b7dbd62b1033727f8606b8c09fc4aa88
   > # Parent  42aa0e570eaa364a622bc4443b0bcb79b1100a58
   > # ClownJoke This is a veryly long header that should not be warned about because its not the description
   > transplant/foo: this summary is way too long use Oxford comma (bc) (bug123) (issue 244)
   > 
   > diff --git a/hgext/transplant.py b/hgext/transplant.py
   > --- a/hgext/transplant.py
   > +++ b/hgext/transplant.py
   > @@ -599,7 +599,7 @@
   >              return
   >          if not (opts.get('source') or revs or
   >                  opts.get('merge') or opts.get('branch')):
   > -            raise error.Abort(_('no source URL, branch revision or revision '
   > +            raise error.Abort(_('no source URL, branch revision, or revision '
   >                                 'list provided'))
   >          if opts.get('all'):
   > EOF
   $ cat patch-with-long-header.diff | $TESTDIR/../contrib/check-commit
   7: (BC) needs to be uppercase
    transplant/foo: this summary is way too long use Oxford comma (bc) (bug123) (issue 244)
   7: no space allowed between issue and number
    transplant/foo: this summary is way too long use Oxford comma (bc) (bug123) (issue 244)
   7: use (issueDDDD) instead of bug
    transplant/foo: this summary is way too long use Oxford comma (bc) (bug123) (issue 244)
   1: username is not an email address
    # User timeless
   7: summary keyword should be most user-relevant one-word command or topic
    transplant/foo: this summary is way too long use Oxford comma (bc) (bug123) (issue 244)
   7: summary line too long (limit is 78)
    transplant/foo: this summary is way too long use Oxford comma (bc) (bug123) (issue 244)
   [1]
 
 A patch with other errors:
 
   $ cat > patch-with-long-header.diff << EOF
   > # HG changeset patch
   > # User timeless
   > # Date 1448911706 0
   > #      Mon Nov 30 19:28:26 2015 +0000
   > # Node ID c41cb6d2b7dbd62b1033727f8606b8c09fc4aa88
   > # Parent  42aa0e570eaa364a622bc4443b0bcb79b1100a58
   > # ClownJoke This is a veryly long header that should not be warned about because its not the description
   > This has no topic and ends with a period.
   > 
   > diff --git a/hgext/transplant.py b/hgext/transplant.py
   > --- a/hgext/transplant.py
   > +++ b/hgext/transplant.py
   > @@ -599,7 +599,7 @@
   >          if opts.get('all'):
   >  
   > +
   > + def blah_blah(x):
   > +     pass
   > +
   >  
   > EOF
   $ cat patch-with-long-header.diff | $TESTDIR/../contrib/check-commit
   1: username is not an email address
    # User timeless
   7: summary line doesn't start with 'topic: '
    This has no topic and ends with a period.
   7: don't capitalize summary lines
    This has no topic and ends with a period.
   7: don't add trailing period on summary line
    This has no topic and ends with a period.
   19: adds double empty line
-    
+   +
   15: adds double empty line
    +
   16: adds a function with foo_bar naming
    + def blah_blah(x):
   [1]