Fix R= line handling when there is no value and improve presubmit TAG line regex

Make it more consistent across the tool. Using \s also includes \n, which
confuses the tool.

Add more tests.

R=iannucci@chromium.org
BUG=

Review URL: https://chromiumcodereview.appspot.com/14265007

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@195214 0039d316-1c4b-4281-b951-d872f2087c98

git_cl.py

@@ -798,7 +798,7 @@ def GetCodereviewSettingsInteractively():
 
 class ChangeDescription(object):
   """Contains a parsed form of the change description."""
-  R_LINE = r'^\s*(TBR|R)\s*=\s*(.+)\s*$'
+  R_LINE = r'^[ \t]*(TBR|R)[ \t]*=[ \t]*(.*?)[ \t]*$'
 
   def __init__(self, description):
     self._description = (description or '').strip()
@@ -820,7 +820,7 @@ class ChangeDescription(object):
       for i in xrange(len(matches) - 1, 0, -1):
         self._description = (
             self._description[:matches[i].start()] +
-            self._description[matches[i].end()+1:])
+            self._description[matches[i].end():])
 
     if is_tbr:
       new_r_line = 'TBR=' + ', '.join(reviewers)
@@ -830,7 +830,7 @@ class ChangeDescription(object):
     if matches:
       self._description = (
         self._description[:matches[0].start()] + new_r_line +
-        self._description[matches[0].end()+1:])
+        self._description[matches[0].end():]).strip()
     else:
       self.append_footer(new_r_line)
 
@@ -869,7 +869,7 @@ class ChangeDescription(object):
   def get_reviewers(self):
     """Retrieves the list of reviewers."""
     regexp = re.compile(self.R_LINE, re.MULTILINE)
-    reviewers = [i.group(2) for i in regexp.finditer(self._description)]
+    reviewers = [i.group(2).strip() for i in regexp.finditer(self._description)]
     return cleanup_list(reviewers)
 
 

presubmit_support.py

@@ -685,7 +685,7 @@ class Change(object):
 
   # Matches key/value (or "tag") lines in changelist descriptions.
   TAG_LINE_RE = re.compile(
-      '^\s*(?P<key>[A-Z][A-Z_0-9]*)\s*=\s*(?P<value>.*?)\s*$')
+      '^[ \t]*(?P<key>[A-Z][A-Z_0-9]*)[ \t]*=[ \t]*(?P<value>.*?)[ \t]*$')
   scm = ''
 
   def __init__(

tests/git_cl_test.py

@@ -612,11 +612,23 @@ class TestGitCl(TestCase):
       ('foo\nBUG=', ['a@c'], 'foo\nBUG=\nR=a@c'),
       ('foo\nR=xx\nTBR=yy\nR=bar', ['a@c'], 'foo\nTBR=a@c'),
       ('foo', ['a@c', 'b@c'], 'foo\n\nR=a@c, b@c'),
+      ('foo\nBar\n\nR=\nBUG=', ['c@c'], 'foo\nBar\n\nR=c@c\nBUG='),
+      ('foo\nBar\n\nR=\nBUG=\nR=', ['c@c'], 'foo\nBar\n\nR=c@c\nBUG='),
+      # Same as the line before, but full of whitespaces.
+      (
+        'foo\nBar\n\n R = \n BUG = \n R = ', ['c@c'],
+        'foo\nBar\n\nR=c@c\n BUG =',
+      ),
+      # Whitespaces aren't interpreted as new lines.
+      ('foo BUG=allo R=joe ', ['c@c'], 'foo BUG=allo R=joe\n\nR=c@c'),
     ]
-    for orig, reviewers, expected in data:
+    expected = [i[2] for i in data]
+    actual = []
+    for orig, reviewers, _expected in data:
       obj = git_cl.ChangeDescription(orig)
       obj.update_reviewers(reviewers)
-      self.assertEqual(expected, obj.description)
+      actual.append(obj.description)
+    self.assertEqual(expected, actual)
 
 
 if __name__ == '__main__':