check-commit: sort errors by line number

contrib/check-commit

@@ -1,91 +1,98 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 = [
     (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: '"),
     (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"),
     (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
+    hits = []
     for exp, msg in errors:
         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)
+            hits.append((end, exp, msg))
+    if hits:
+        hits.sort()
+        pos = 0
+        last = ''
+        for n, l in enumerate(commit.splitlines(True)):
+            pos += len(l)
+            while len(hits):
+                end, exp, msg = hits[0]
                 if pos < end:
-                    last = nonempty(l, last)
-                else:
-                    if not printed:
-                        printed = True
-                        print "node: %s" % node
-                    print "%d: %s" % (n, msg)
-                    print " %s" % nonempty(l, last)[:-1]
-                    if "BYPASS" not in os.environ:
-                        exitcode = 1
                     break
+                if not printed:
+                    printed = True
+                    print "node: %s" % node
+                print "%d: %s" % (n, msg)
+                print " %s" % nonempty(l, last)[:-1]
+                if "BYPASS" not in os.environ:
+                    exitcode = 1
+                del hits[0]
+            last = nonempty(l, last)
+
     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
+  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: no space allowed between issue and number
+  7: (BC) needs to be uppercase
    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
+  7: no space allowed between issue and number
    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: '
+  7: don't capitalize summary lines
    This has no topic and ends with a period.
-  7: don't capitalize summary lines
+  7: summary line doesn't start with 'topic: '
    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):
+  19: adds double empty line
+   +
   [1]